From 531af2f4c1433a3b8b0a42b6ff9c8dd6cf9afdf6 Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Mon, 30 Oct 2017 11:24:48 +0000 Subject: [PATCH] [parser] Use n-ary addition for template strings When closing untagged template string literals, create a single n-ary addition operation, instead of a tree of binary operations. As a clean-up, this also entirely removes the "second" field from n-ary operations. This was proving to be too confusing an API when building an n-ary operation incrementally from a single expression (rather than converting a binary operation). Bug: v8:6964 Change-Id: I8f2a395d413cf345bab0a1a347b47f412cde83b1 Reviewed-on: https://chromium-review.googlesource.com/739821 Reviewed-by: Adam Klein Commit-Queue: Leszek Swirski Cr-Commit-Position: refs/heads/master@{#49054} --- src/ast/ast.h | 38 +++++++++++------------- src/parsing/parser.cc | 25 +++++++++------- test/js-perf-test/ExpressionDepth/run.js | 5 ++-- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/src/ast/ast.h b/src/ast/ast.h index e49cee3a9e..e834dc7578 100644 --- a/src/ast/ast.h +++ b/src/ast/ast.h @@ -1822,18 +1822,15 @@ class NaryOperation final : public Expression { Expression* first() const { return first_; } void set_first(Expression* e) { first_ = e; } Expression* subsequent(size_t index) const { - if (index == 0) return second_; - return subsequent_[index - 1].expression; + return subsequent_[index].expression; } Expression* set_subsequent(size_t index, Expression* e) { - if (index == 0) return second_ = e; - return subsequent_[index - 1].expression = e; + return subsequent_[index].expression = e; } - size_t subsequent_length() const { return 1 + subsequent_.size(); } + size_t subsequent_length() const { return subsequent_.size(); } int subsequent_op_position(size_t index) const { - if (index == 0) return position(); - return subsequent_[index - 1].op_position; + return subsequent_[index].op_position; } void AddSubsequent(Expression* expr, int pos) { @@ -1844,33 +1841,32 @@ class NaryOperation final : public Expression { friend class AstNodeFactory; NaryOperation(Zone* zone, Token::Value op, Expression* first, - Expression* second, int pos) - : Expression(pos, kNaryOperation), + size_t initial_subsequent_size) + : Expression(kNoSourcePosition, kNaryOperation), first_(first), - second_(second), subsequent_(zone) { bit_field_ |= OperatorField::encode(op); DCHECK(Token::IsBinaryOp(op)); DCHECK_NE(op, Token::EXP); + subsequent_.reserve(initial_subsequent_size); } - // Nary operations store the first operation (and so first two child - // expressions) inline, where the position of the first operation is the - // position of this expression. Subsequent child expressions are stored - // out-of-line, along with with their operation's position and feedback slot. + // Nary operations store the first (lhs) child expression inline, and the + // child expressions (rhs of each op) are stored out-of-line, along with + // their operation's position. Note that the Nary operation expression's + // position has no meaning. // // So an nary add: // - // expr + expr + expr + expr + ... + // expr + expr + expr + ... // // is stored as: // - // (expr + expr) [(+ expr), (+ expr), ...] - // '-----.-----' '-----------.-----------' - // this subsequent entry list + // (expr) [(+ expr), (+ expr), ...] + // '-.--' '-----------.-----------' + // first subsequent entry list Expression* first_; - Expression* second_; struct NaryOperationEntry { Expression* expression; @@ -3148,8 +3144,8 @@ class AstNodeFactory final BASE_EMBEDDED { } NaryOperation* NewNaryOperation(Token::Value op, Expression* first, - Expression* second, int pos) { - return new (zone_) NaryOperation(zone_, op, first, second, pos); + size_t initial_subsequent_size) { + return new (zone_) NaryOperation(zone_, op, first, initial_subsequent_size); } CountOperation* NewCountOperation(Token::Value op, diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc index 6789d0af5f..5c47f54291 100644 --- a/src/parsing/parser.cc +++ b/src/parsing/parser.cc @@ -319,8 +319,8 @@ bool Parser::CollapseNaryExpression(Expression** x, Expression* y, BinaryOperation* binop = (*x)->AsBinaryOperation(); if (binop->op() != op) return false; - nary = factory()->NewNaryOperation(op, binop->left(), binop->right(), - binop->position()); + nary = factory()->NewNaryOperation(op, binop->left(), 2); + nary->AddSubsequent(binop->right(), binop->position()); *x = nary; } else if ((*x)->IsNaryOperation()) { nary = (*x)->AsNaryOperation(); @@ -3488,9 +3488,16 @@ Expression* Parser::CloseTemplateLiteral(TemplateLiteralState* state, int start, DCHECK_EQ(cooked_strings->length(), expressions->length() + 1); if (!tag) { - // Build tree of BinaryOps to simplify code-generation - Expression* expr = + Expression* first_string = factory()->NewStringLiteral(cooked_strings->at(0), kNoSourcePosition); + if (expressions->length() == 0) return first_string; + + // Build N-ary addition op to simplify code-generation. + // TODO(leszeks): Could we just store this expression in the + // TemplateLiteralState and build it as we go? + NaryOperation* expr = factory()->NewNaryOperation( + Token::ADD, first_string, 2 * expressions->length()); + int i = 0; while (i < expressions->length()) { Expression* sub = expressions->at(i++); @@ -3501,13 +3508,11 @@ Expression* Parser::CloseTemplateLiteral(TemplateLiteralState* state, int start, ZoneList* args = new (zone()) ZoneList(1, zone()); args->Add(sub, zone()); - Expression* middle = factory()->NewCallRuntime(Runtime::kInlineToString, - args, sub->position()); + Expression* sub_to_string = factory()->NewCallRuntime( + Runtime::kInlineToString, args, sub->position()); - expr = factory()->NewBinaryOperation( - Token::ADD, - factory()->NewBinaryOperation(Token::ADD, expr, middle, - expr->position()), + expr->AddSubsequent(sub_to_string, sub->position()); + expr->AddSubsequent( factory()->NewStringLiteral(cooked_str, kNoSourcePosition), sub->position()); } diff --git a/test/js-perf-test/ExpressionDepth/run.js b/test/js-perf-test/ExpressionDepth/run.js index 92bcb56f84..1b3bd83998 100644 --- a/test/js-perf-test/ExpressionDepth/run.js +++ b/test/js-perf-test/ExpressionDepth/run.js @@ -24,6 +24,7 @@ AddTest('Add', '+'); AddTest('Sub', '-'); AddTest('BitwiseOr', '|'); AddTestCustomPrologue('StringConcat', '+', '"string" +'); +AddTestCustomPrologue('TemplateString', '} ${', '`${', '}`'); function TestExpressionDepth(depth, expression, prologue, epilogue) { var func = '(function f(a) {\n' + prologue; @@ -84,6 +85,6 @@ function AddTest(name, expression, in_test) { RunTest(name, expression, prologue, epilogue); } -function AddTestCustomPrologue(name, expression, prologue) { - RunTest(name, expression, prologue, ''); +function AddTestCustomPrologue(name, expression, prologue, epilogue='') { + RunTest(name, expression, prologue, epilogue); }