[parser] Reduce AST overhead for parsing SwitchStatements

This makes several changes to SwitchStatement handling:

  - Store the CaseClause list inline (as it's always allocated)
  - Only rewrite with additional blocks if the Block Scope for
    the switch statement isn't empty
  - Use Parser::IgnoreCompletion() instead of inserting an additional
    `undefined` ExpressionStatement

Bug: v8:6092
Change-Id: Ib08d0ba851dd8e78b3dc74782b8e554541e79182
Reviewed-on: https://chromium-review.googlesource.com/644176Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47751}

src/ast/ast.h

@@ -888,29 +888,26 @@ class CaseClause final : public ZoneObject {
 
 class SwitchStatement final : public BreakableStatement {
  public:
-  void Initialize(Expression* tag, ZoneList<CaseClause*>* cases) {
-    tag_ = tag;
-    cases_ = cases;
-  }
-
   ZoneList<const AstRawString*>* labels() const { return labels_; }
-  Expression* tag() const { return tag_; }
-  ZoneList<CaseClause*>* cases() const { return cases_; }
 
+  Expression* tag() const { return tag_; }
   void set_tag(Expression* t) { tag_ = t; }
 
+  ZoneList<CaseClause*>* cases() { return &cases_; }
+
  private:
   friend class AstNodeFactory;
 
-  SwitchStatement(ZoneList<const AstRawString*>* labels, int pos)
+  SwitchStatement(Zone* zone, ZoneList<const AstRawString*>* labels,
+                  Expression* tag, int pos)
       : BreakableStatement(TARGET_FOR_ANONYMOUS, pos, kSwitchStatement),
         labels_(labels),
-        tag_(NULL),
-        cases_(NULL) {}
+        tag_(tag),
+        cases_(4, zone) {}
 
   ZoneList<const AstRawString*>* labels_;
   Expression* tag_;
-  ZoneList<CaseClause*>* cases_;
+  ZoneList<CaseClause*> cases_;
 };
 
 
@@ -3181,9 +3178,13 @@ class AstNodeFactory final BASE_EMBEDDED {
   STATEMENT_WITH_LABELS(DoWhileStatement)
   STATEMENT_WITH_LABELS(WhileStatement)
   STATEMENT_WITH_LABELS(ForStatement)
-  STATEMENT_WITH_LABELS(SwitchStatement)
 #undef STATEMENT_WITH_LABELS
 
+  SwitchStatement* NewSwitchStatement(ZoneList<const AstRawString*>* labels,
+                                      Expression* tag, int pos) {
+    return new (zone_) SwitchStatement(zone_, labels, tag, pos);
+  }
+
   ForEachStatement* NewForEachStatement(ForEachStatement::VisitMode visit_mode,
                                         ZoneList<const AstRawString*>* labels,
                                         int pos) {

src/parsing/parser-base.h

@@ -5391,7 +5391,8 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseSwitchStatement(
   ExpressionT tag = ParseExpression(true, CHECK_OK);
   Expect(Token::RPAREN, CHECK_OK);
 
-  auto switch_statement = factory()->NewSwitchStatement(labels, switch_pos);
+  auto switch_statement =
+      factory()->NewSwitchStatement(labels, tag, switch_pos);
 
   {
     BlockState cases_block_state(zone(), &scope_);
@@ -5400,7 +5401,6 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseSwitchStatement(
     typename Types::Target target(this, switch_statement);
 
     bool default_seen = false;
-    auto cases = impl()->NewCaseClauseList(4);
     Expect(Token::LBRACE, CHECK_OK);
     while (peek() != Token::RBRACE) {
       // An empty label indicates the default case.
@@ -5427,15 +5427,18 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseSwitchStatement(
       }
       auto clause = factory()->NewCaseClause(label, statements);
       impl()->RecordCaseClauseSourceRange(clause, range_scope.Finalize());
-      cases->Add(clause, zone());
+      switch_statement->cases()->Add(clause, zone());
     }
     Expect(Token::RBRACE, CHECK_OK);
 
     int end_position = scanner()->location().end_pos;
     scope()->set_end_position(end_position);
     impl()->RecordSwitchStatementSourceRange(switch_statement, end_position);
-    return impl()->RewriteSwitchStatement(tag, switch_statement, cases,
-                                          scope()->FinalizeBlockScope());
+    Scope* switch_scope = scope()->FinalizeBlockScope();
+    if (switch_scope != nullptr) {
+      return impl()->RewriteSwitchStatement(switch_statement, switch_scope);
+    }
+    return switch_statement;
   }
 }
 

src/parsing/parser.cc

@@ -1584,9 +1584,7 @@ Expression* Parser::RewriteDoExpression(Block* body, int pos, bool* ok) {
   return expr;
 }
 
-Statement* Parser::RewriteSwitchStatement(Expression* tag,
-                                          SwitchStatement* switch_statement,
-                                          ZoneList<CaseClause*>* cases,
+Statement* Parser::RewriteSwitchStatement(SwitchStatement* switch_statement,
                                           Scope* scope) {
   // In order to get the CaseClauses to execute in their own lexical scope,
   // but without requiring downstream code to have special scope handling
@@ -1597,35 +1595,29 @@ Statement* Parser::RewriteSwitchStatement(Expression* tag,
   //     switch (.tag_variable) { CaseClause* }
   //   }
   // }
+  DCHECK_NOT_NULL(scope);
+  DCHECK(scope->is_block_scope());
+  DCHECK_GE(switch_statement->position(), scope->start_position());
+  DCHECK_LT(switch_statement->position(), scope->end_position());
 
   Block* switch_block = factory()->NewBlock(2, false);
 
+  Expression* tag = switch_statement->tag();
   Variable* tag_variable =
       NewTemporary(ast_value_factory()->dot_switch_tag_string());
   Assignment* tag_assign = factory()->NewAssignment(
       Token::ASSIGN, factory()->NewVariableProxy(tag_variable), tag,
       tag->position());
-  Statement* tag_statement =
-      factory()->NewExpressionStatement(tag_assign, kNoSourcePosition);
+  // Wrap with IgnoreCompletion so the tag isn't returned as the completion
+  // value, in case the switch statements don't have a value.
+  Statement* tag_statement = IgnoreCompletion(
+      factory()->NewExpressionStatement(tag_assign, kNoSourcePosition));
   switch_block->statements()->Add(tag_statement, zone());
 
-  // make statement: undefined;
-  // This is needed so the tag isn't returned as the value, in case the switch
-  // statements don't have a value.
-  switch_block->statements()->Add(
-      factory()->NewExpressionStatement(
-          factory()->NewUndefinedLiteral(kNoSourcePosition), kNoSourcePosition),
-      zone());
-
-  Expression* tag_read = factory()->NewVariableProxy(tag_variable);
-  switch_statement->Initialize(tag_read, cases);
+  switch_statement->set_tag(factory()->NewVariableProxy(tag_variable));
   Block* cases_block = factory()->NewBlock(1, false);
   cases_block->statements()->Add(switch_statement, zone());
   cases_block->set_scope(scope);
-  DCHECK_IMPLIES(scope != nullptr,
-                 switch_statement->position() >= scope->start_position());
-  DCHECK_IMPLIES(scope != nullptr,
-                 switch_statement->position() < scope->end_position());
   switch_block->statements()->Add(cases_block, zone());
   return switch_block;
 }

src/parsing/parser.h

@@ -332,9 +332,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   bool ContainsLabel(ZoneList<const AstRawString*>* labels,
                      const AstRawString* label);
   Expression* RewriteReturn(Expression* return_value, int pos);
-  Statement* RewriteSwitchStatement(Expression* tag,
-                                    SwitchStatement* switch_statement,
-                                    ZoneList<CaseClause*>* cases, Scope* scope);
+  Statement* RewriteSwitchStatement(SwitchStatement* switch_statement,
+                                    Scope* scope);
   void RewriteCatchPattern(CatchInfo* catch_info, bool* ok);
   void ValidateCatchBlock(const CatchInfo& catch_info, bool* ok);
   Statement* RewriteTryStatement(Block* try_block, Block* catch_block,
@@ -916,9 +915,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   V8_INLINE ZoneList<Statement*>* NewStatementList(int size) const {
     return new (zone()) ZoneList<Statement*>(size, zone());
   }
-  V8_INLINE ZoneList<CaseClause*>* NewCaseClauseList(int size) const {
-    return new (zone()) ZoneList<CaseClause*>(size, zone());
-  }
 
   V8_INLINE Expression* NewV8Intrinsic(const AstRawString* name,
                                        ZoneList<Expression*>* args, int pos,

src/parsing/preparser.h

@@ -464,7 +464,11 @@ class PreParserStatement {
   // and PreParser.
   PreParserStatement* operator->() { return this; }
 
+  // TODO(adamk): These should return something even lighter-weight than
+  // PreParserStatementList.
   PreParserStatementList statements() { return PreParserStatementList(); }
+  PreParserStatementList cases() { return PreParserStatementList(); }
+
   void set_scope(Scope* scope) {}
   void Initialize(const PreParserExpression& cond, PreParserStatement body,
                   const SourceRange& body_range = {}) {}
@@ -713,6 +717,7 @@ class PreParserFactory {
   }
 
   PreParserStatement NewSwitchStatement(ZoneList<const AstRawString*>* labels,
+                                        const PreParserExpression& tag,
                                         int pos) {
     return PreParserStatement::Default();
   }
@@ -996,9 +1001,8 @@ class PreParser : public ParserBase<PreParser> {
   RewriteReturn(const PreParserExpression& return_value, int pos) {
     return return_value;
   }
-  V8_INLINE PreParserStatement RewriteSwitchStatement(
-      const PreParserExpression& tag, PreParserStatement switch_statement,
-      PreParserStatementList cases, Scope* scope) {
+  V8_INLINE PreParserStatement
+  RewriteSwitchStatement(PreParserStatement switch_statement, Scope* scope) {
     return PreParserStatement::Default();
   }
 
@@ -1512,10 +1516,6 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserStatementList();
   }
 
-  PreParserStatementList NewCaseClauseList(int size) {
-    return PreParserStatementList();
-  }
-
   V8_INLINE PreParserExpression
   NewV8Intrinsic(const PreParserIdentifier& name,
                  const PreParserExpressionList& arguments, int pos, bool* ok) {

test/cctest/interpreter/bytecode_expectations/Switch.golden

@@ -13,20 +13,19 @@ snippet: "
    case 2: return 3;
   }
 "
-frame size: 3
+frame size: 2
 parameter count: 1
-bytecode array length: 34
+bytecode array length: 32
 bytecodes: [
   /*   30 E> */ B(StackCheck),
   /*   42 S> */ B(LdaSmi), I8(1),
                 B(Star), R(0),
-                B(Star), R(1),
   /*   45 S> */ B(LdaSmi), I8(1),
-                B(TestEqualStrict), R(1), U8(0),
-                B(Mov), R(0), R(2),
+                B(TestEqualStrict), R(0), U8(0),
+                B(Mov), R(0), R(1),
                 B(JumpIfTrue), U8(11),
                 B(LdaSmi), I8(2),
-                B(TestEqualStrict), R(2), U8(1),
+                B(TestEqualStrict), R(1), U8(1),
                 B(JumpIfTrue), U8(7),
                 B(Jump), U8(8),
   /*   66 S> */ B(LdaSmi), I8(2),
@@ -49,20 +48,19 @@ snippet: "
    case 2: a = 3; break;
   }
 "
-frame size: 3
+frame size: 2
 parameter count: 1
-bytecode array length: 40
+bytecode array length: 38
 bytecodes: [
   /*   30 E> */ B(StackCheck),
   /*   42 S> */ B(LdaSmi), I8(1),
                 B(Star), R(0),
-                B(Star), R(1),
   /*   45 S> */ B(LdaSmi), I8(1),
-                B(TestEqualStrict), R(1), U8(0),
-                B(Mov), R(0), R(2),
+                B(TestEqualStrict), R(0), U8(0),
+                B(Mov), R(0), R(1),
                 B(JumpIfTrue), U8(11),
                 B(LdaSmi), I8(2),
-                B(TestEqualStrict), R(2), U8(1),
+                B(TestEqualStrict), R(1), U8(1),
                 B(JumpIfTrue), U8(10),
                 B(Jump), U8(14),
   /*   66 S> */ B(LdaSmi), I8(2),
@@ -87,20 +85,19 @@ snippet: "
    case 2: a = 3; break;
   }
 "
-frame size: 3
+frame size: 2
 parameter count: 1
-bytecode array length: 38
+bytecode array length: 36
 bytecodes: [
   /*   30 E> */ B(StackCheck),
   /*   42 S> */ B(LdaSmi), I8(1),
                 B(Star), R(0),
-                B(Star), R(1),
   /*   45 S> */ B(LdaSmi), I8(1),
-                B(TestEqualStrict), R(1), U8(0),
-                B(Mov), R(0), R(2),
+                B(TestEqualStrict), R(0), U8(0),
+                B(Mov), R(0), R(1),
                 B(JumpIfTrue), U8(11),
                 B(LdaSmi), I8(2),
-                B(TestEqualStrict), R(2), U8(1),
+                B(TestEqualStrict), R(1), U8(1),
                 B(JumpIfTrue), U8(8),
                 B(Jump), U8(12),
   /*   66 S> */ B(LdaSmi), I8(2),
@@ -125,20 +122,19 @@ snippet: "
    default: a = 1; break;
   }
 "
-frame size: 3
+frame size: 2
 parameter count: 1
-bytecode array length: 38
+bytecode array length: 36
 bytecodes: [
   /*   30 E> */ B(StackCheck),
   /*   42 S> */ B(LdaSmi), I8(1),
                 B(Star), R(0),
-                B(Star), R(1),
   /*   45 S> */ B(LdaSmi), I8(2),
-                B(TestEqualStrict), R(1), U8(0),
-                B(Mov), R(0), R(2),
+                B(TestEqualStrict), R(0), U8(0),
+                B(Mov), R(0), R(1),
                 B(JumpIfTrue), U8(11),
                 B(LdaSmi), I8(3),
-                B(TestEqualStrict), R(2), U8(1),
+                B(TestEqualStrict), R(1), U8(1),
                 B(JumpIfTrue), U8(6),
                 B(Jump), U8(6),
   /*   66 S> */ B(Jump), U8(10),
@@ -163,21 +159,20 @@ snippet: "
    default: a = 3; break;
   }
 "
-frame size: 3
+frame size: 2
 parameter count: 1
-bytecode array length: 47
+bytecode array length: 44
 bytecodes: [
   /*   30 E> */ B(StackCheck),
   /*   42 S> */ B(LdaSmi), I8(1),
                 B(Star), R(0),
-  /*   42 E> */ B(TypeOf),
+  /*   45 S> */ B(TypeOf),
                 B(Star), R(1),
-  /*   45 S> */ B(LdaSmi), I8(2),
+                B(LdaSmi), I8(2),
                 B(TestEqualStrict), R(1), U8(0),
-                B(Mov), R(1), R(2),
                 B(JumpIfTrue), U8(11),
                 B(LdaSmi), I8(3),
-                B(TestEqualStrict), R(2), U8(1),
+                B(TestEqualStrict), R(1), U8(1),
                 B(JumpIfTrue), U8(10),
                 B(Jump), U8(14),
   /*   74 S> */ B(LdaSmi), I8(1),
@@ -205,17 +200,16 @@ snippet: "
    default: a = 2; break;
   }
 "
-frame size: 3
+frame size: 2
 parameter count: 1
-bytecode array length: 32
+bytecode array length: 30
 bytecodes: [
   /*   30 E> */ B(StackCheck),
   /*   42 S> */ B(LdaSmi), I8(1),
                 B(Star), R(0),
-                B(Star), R(1),
   /*   45 S> */ B(TypeOf),
-                B(TestEqualStrict), R(1), U8(0),
-                B(Mov), R(0), R(2),
+                B(TestEqualStrict), R(0), U8(0),
+                B(Mov), R(0), R(1),
                 B(JumpIfTrue), U8(4),
                 B(Jump), U8(8),
   /*   74 S> */ B(LdaSmi), I8(1),
@@ -307,20 +301,19 @@ snippet: "
     break;
   }
 "
-frame size: 3
+frame size: 2
 parameter count: 1
-bytecode array length: 292
+bytecode array length: 290
 bytecodes: [
   /*   30 E> */ B(StackCheck),
   /*   42 S> */ B(LdaSmi), I8(1),
                 B(Star), R(0),
-                B(Star), R(1),
   /*   45 S> */ B(LdaSmi), I8(1),
-                B(TestEqualStrict), R(1), U8(0),
-                B(Mov), R(0), R(2),
+                B(TestEqualStrict), R(0), U8(0),
+                B(Mov), R(0), R(1),
                 B(JumpIfTrue), U8(11),
                 B(LdaSmi), I8(2),
-                B(TestEqualStrict), R(2), U8(1),
+                B(TestEqualStrict), R(1), U8(1),
                 B(JumpIfTrueConstant), U8(0),
                 B(JumpConstant), U8(1),
   /*   68 S> */ B(LdaSmi), I8(2),
@@ -477,28 +470,26 @@ snippet: "
    case 2: a = 3;
   }
 "
-frame size: 5
+frame size: 3
 parameter count: 1
-bytecode array length: 63
+bytecode array length: 58
 bytecodes: [
   /*   30 E> */ B(StackCheck),
   /*   42 S> */ B(LdaSmi), I8(1),
                 B(Star), R(0),
-                B(Star), R(2),
   /*   45 S> */ B(LdaSmi), I8(1),
-                B(TestEqualStrict), R(2), U8(3),
-                B(Mov), R(0), R(3),
+                B(TestEqualStrict), R(0), U8(3),
+                B(Mov), R(0), R(1),
                 B(JumpIfTrue), U8(11),
                 B(LdaSmi), I8(2),
-                B(TestEqualStrict), R(3), U8(4),
-                B(JumpIfTrue), U8(35),
-                B(Jump), U8(37),
-                B(Ldar), R(0),
+                B(TestEqualStrict), R(1), U8(4),
+                B(JumpIfTrue), U8(32),
+                B(Jump), U8(34),
+  /*   70 S> */ B(Ldar), R(0),
   /*   79 E> */ B(AddSmi), I8(1), U8(0),
-                B(Star), R(1),
-  /*   70 S> */ B(LdaSmi), I8(2),
-                B(TestEqualStrict), R(1), U8(1),
-                B(Mov), R(1), R(4),
+                B(Star), R(2),
+                B(LdaSmi), I8(2),
+                B(TestEqualStrict), R(2), U8(1),
                 B(JumpIfTrue), U8(4),
                 B(Jump), U8(8),
   /*  101 S> */ B(LdaSmi), I8(1),