Commit d12dbab4 authored by ishell's avatar ishell Committed by Commit bot

[es6] More efficient way of marking AST call expressions in tail positions.

Instead of doing a full function body traversal we collect return expressions and mark them after function parsing.

And since we rewrite do-expressions so that the result is explicitly assigned to a result variable the statements marking will never hit so I removed it from the AST.

BUG=v8:4698
LOG=N

Review URL: https://codereview.chromium.org/1693523002

Cr-Commit-Position: refs/heads/master@{#33911}
parent 879d254d
......@@ -244,7 +244,6 @@ class Statement : public AstNode {
bool IsEmpty() { return AsEmptyStatement() != NULL; }
virtual bool IsJump() const { return false; }
virtual void MarkTail() {}
};
......@@ -472,13 +471,6 @@ class Block final : public BreakableStatement {
&& labels() == NULL; // Good enough as an approximation...
}
void MarkTail() override {
for (int i = 0; i < statements_.length(); i++) {
Statement* stmt = statements_.at(i);
stmt->MarkTail();
}
}
Scope* scope() const { return scope_; }
void set_scope(Scope* scope) { scope_ = scope; }
......@@ -509,8 +501,6 @@ class DoExpression final : public Expression {
VariableProxy* result() { return result_; }
void set_result(VariableProxy* v) { result_ = v; }
void MarkTail() override { block_->MarkTail(); }
protected:
DoExpression(Zone* zone, Block* block, VariableProxy* result, int pos)
: Expression(zone, pos), block_(block), result_(result) {
......@@ -1022,8 +1012,6 @@ class ReturnStatement final : public JumpStatement {
void set_expression(Expression* e) { expression_ = e; }
void MarkTail() override { expression_->MarkTail(); }
protected:
explicit ReturnStatement(Zone* zone, Expression* expression, int pos)
: JumpStatement(zone, pos), expression_(expression) { }
......@@ -1048,8 +1036,6 @@ class WithStatement final : public Statement {
BailoutId ToObjectId() const { return BailoutId(local_id(0)); }
BailoutId EntryId() const { return BailoutId(local_id(1)); }
void MarkTail() override { statement_->MarkTail(); }
protected:
WithStatement(Zone* zone, Scope* scope, Expression* expression,
Statement* statement, int pos)
......@@ -1092,13 +1078,6 @@ class CaseClause final : public Expression {
BailoutId EntryId() const { return BailoutId(local_id(0)); }
TypeFeedbackId CompareId() { return TypeFeedbackId(local_id(1)); }
void MarkTail() override {
for (int i = 0; i < statements_->length(); i++) {
Statement* stmt = statements_->at(i);
stmt->MarkTail();
}
}
Type* compare_type() { return compare_type_; }
void set_compare_type(Type* type) { compare_type_ = type; }
......@@ -1131,13 +1110,6 @@ class SwitchStatement final : public BreakableStatement {
void set_tag(Expression* t) { tag_ = t; }
void MarkTail() override {
for (int i = 0; i < cases_->length(); i++) {
CaseClause* clause = cases_->at(i);
clause->MarkTail();
}
}
protected:
SwitchStatement(Zone* zone, ZoneList<const AstRawString*>* labels, int pos)
: BreakableStatement(zone, labels, TARGET_FOR_ANONYMOUS, pos),
......@@ -1175,11 +1147,6 @@ class IfStatement final : public Statement {
&& HasElseStatement() && else_statement()->IsJump();
}
void MarkTail() override {
then_statement_->MarkTail();
else_statement_->MarkTail();
}
void set_base_id(int id) { base_id_ = id; }
static int num_ids() { return parent_num_ids() + 3; }
BailoutId IfId() const { return BailoutId(local_id(0)); }
......@@ -1249,8 +1216,6 @@ class TryCatchStatement final : public TryStatement {
Block* catch_block() const { return catch_block_; }
void set_catch_block(Block* b) { catch_block_ = b; }
void MarkTail() override { catch_block_->MarkTail(); }
protected:
TryCatchStatement(Zone* zone, Block* try_block, Scope* scope,
Variable* variable, Block* catch_block, int pos)
......@@ -1273,8 +1238,6 @@ class TryFinallyStatement final : public TryStatement {
Block* finally_block() const { return finally_block_; }
void set_finally_block(Block* b) { finally_block_ = b; }
void MarkTail() override { finally_block_->MarkTail(); }
protected:
TryFinallyStatement(Zone* zone, Block* try_block, Block* finally_block,
int pos)
......
......@@ -246,6 +246,22 @@ class ParserBase : public Traits {
destructuring_assignments_to_rewrite_.Add(pair);
}
List<ExpressionT>& expressions_in_tail_position() {
return expressions_in_tail_position_;
}
void AddExpressionInTailPosition(ExpressionT expression) {
if (collect_expressions_in_tail_position_) {
expressions_in_tail_position_.Add(expression);
}
}
bool collect_expressions_in_tail_position() const {
return collect_expressions_in_tail_position_;
}
void set_collect_expressions_in_tail_position(bool collect) {
collect_expressions_in_tail_position_ = collect;
}
private:
// Used to assign an index to each literal that needs materialization in
// the function. Includes regexp literals, and boilerplate for object and
......@@ -276,6 +292,8 @@ class ParserBase : public Traits {
Scope* outer_scope_;
List<DestructuringAssignment> destructuring_assignments_to_rewrite_;
List<ExpressionT> expressions_in_tail_position_;
bool collect_expressions_in_tail_position_;
void RewriteDestructuringAssignments();
......@@ -933,7 +951,6 @@ class ParserBase : public Traits {
bool allow_harmony_function_name_;
};
template <class Traits>
ParserBase<Traits>::FunctionState::FunctionState(
FunctionState** function_state_stack, Scope** scope_stack, Scope* scope,
......@@ -949,6 +966,7 @@ ParserBase<Traits>::FunctionState::FunctionState(
outer_function_state_(*function_state_stack),
scope_stack_(scope_stack),
outer_scope_(*scope_stack),
collect_expressions_in_tail_position_(true),
factory_(factory) {
*scope_stack_ = scope;
*function_state_stack = this;
......
......@@ -2792,6 +2792,11 @@ Statement* Parser::ParseReturnStatement(bool* ok) {
is_undefined, ThisExpression(scope_, factory(), pos),
is_object_conditional, pos);
}
// ES6 14.6.1 Static Semantics: IsInTailPosition
if (FLAG_harmony_tailcalls && !is_sloppy(language_mode())) {
function_state_->AddExpressionInTailPosition(return_value);
}
}
ExpectSemicolon(CHECK_OK);
......@@ -2997,6 +3002,40 @@ Statement* Parser::ParseThrowStatement(bool* ok) {
factory()->NewThrow(exception, pos), pos);
}
class Parser::DontCollectExpressionsInTailPositionScope {
public:
DontCollectExpressionsInTailPositionScope(
Parser::FunctionState* function_state)
: function_state_(function_state),
old_value_(function_state->collect_expressions_in_tail_position()) {
function_state->set_collect_expressions_in_tail_position(false);
}
~DontCollectExpressionsInTailPositionScope() {
function_state_->set_collect_expressions_in_tail_position(old_value_);
}
private:
Parser::FunctionState* function_state_;
bool old_value_;
};
// Collects all return expressions at tail call position in this scope
// to a separate list.
class Parser::CollectExpressionsInTailPositionToListScope {
public:
CollectExpressionsInTailPositionToListScope(
Parser::FunctionState* function_state, List<Expression*>* list)
: function_state_(function_state), list_(list) {
function_state->expressions_in_tail_position().Swap(list_);
}
~CollectExpressionsInTailPositionToListScope() {
function_state_->expressions_in_tail_position().Swap(list_);
}
private:
Parser::FunctionState* function_state_;
List<Expression*>* list_;
};
TryStatement* Parser::ParseTryStatement(bool* ok) {
// TryStatement ::
......@@ -3013,7 +3052,11 @@ TryStatement* Parser::ParseTryStatement(bool* ok) {
Expect(Token::TRY, CHECK_OK);
int pos = position();
Block* try_block = ParseBlock(NULL, CHECK_OK);
Block* try_block;
{
DontCollectExpressionsInTailPositionScope no_tail_calls(function_state_);
try_block = ParseBlock(NULL, CHECK_OK);
}
Token::Value tok = peek();
if (tok != Token::CATCH && tok != Token::FINALLY) {
......@@ -3025,6 +3068,7 @@ TryStatement* Parser::ParseTryStatement(bool* ok) {
Scope* catch_scope = NULL;
Variable* catch_variable = NULL;
Block* catch_block = NULL;
List<Expression*> expressions_in_tail_position_in_catch_block;
if (tok == Token::CATCH) {
Consume(Token::CATCH);
......@@ -3050,6 +3094,9 @@ TryStatement* Parser::ParseTryStatement(bool* ok) {
Expect(Token::RPAREN, CHECK_OK);
{
CollectExpressionsInTailPositionToListScope
collect_expressions_in_tail_position_scope(
function_state_, &expressions_in_tail_position_in_catch_block);
BlockState block_state(&scope_, catch_scope);
// TODO(adamk): Make a version of ParseBlock that takes a scope and
......@@ -3124,6 +3171,11 @@ TryStatement* Parser::ParseTryStatement(bool* ok) {
TryStatement* result = NULL;
if (catch_block != NULL) {
// For a try-catch construct append return expressions from the catch block
// to the list of return expressions.
function_state_->expressions_in_tail_position().AddAll(
expressions_in_tail_position_in_catch_block);
DCHECK(finally_block == NULL);
DCHECK(catch_scope != NULL && catch_variable != NULL);
result = factory()->NewTryCatchStatement(try_block, catch_scope,
......@@ -4751,11 +4803,11 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
}
// ES6 14.6.1 Static Semantics: IsInTailPosition
if (FLAG_harmony_tailcalls && !is_sloppy(language_mode())) {
for (int i = 0; i < body->length(); i++) {
Statement* stmt = body->at(i);
stmt->MarkTail();
}
// Mark collected return expressions that are in tail call position.
const List<Expression*>& expressions_in_tail_position =
function_state_->expressions_in_tail_position();
for (int i = 0; i < expressions_in_tail_position.length(); ++i) {
expressions_in_tail_position[i]->MarkTail();
}
return result;
}
......
......@@ -902,6 +902,8 @@ class Parser : public ParserBase<ParserTraits> {
Statement* ParseForStatement(ZoneList<const AstRawString*>* labels, bool* ok);
Statement* ParseThrowStatement(bool* ok);
Expression* MakeCatchContext(Handle<String> id, VariableProxy* value);
class DontCollectExpressionsInTailPositionScope;
class CollectExpressionsInTailPositionToListScope;
TryStatement* ParseTryStatement(bool* ok);
DebuggerStatement* ParseDebuggerStatement(bool* ok);
......
......@@ -15,6 +15,16 @@ function CheckStackTrace(expected) {
}
}
function f(expected_call_stack, a, b) {
CheckStackTrace(expected_call_stack);
return a;
}
function f_153(expected_call_stack, a) {
CheckStackTrace(expected_call_stack);
return 153;
}
// Tail call when caller does not have an arguments adaptor frame.
(function test() {
......@@ -175,3 +185,149 @@ function CheckStackTrace(expected) {
function g4(a) { return b4(2); }
assertEquals(12, g4());
})();
// Tail calling via various expressions.
(function test() {
function g1(a) {
return f([f, g1, test], false) || f([f, test], true);
}
assertEquals(true, g1());
function g2(a) {
return f([f, g2, test], true) && f([f, test], true);
}
assertEquals(true, g2());
function g3(a) {
return f([f, g3, test], 13), f([f, test], 153);
}
assertEquals(153, g3());
})();
// Test tail calls from try-catch-finally constructs.
(function test() {
//
// try-catch
//
function tc1(a) {
try {
f_153([f_153, tc1, test]);
return f_153([f_153, tc1, test]);
} catch(e) {
f_153([f_153, tc1, test]);
}
}
assertEquals(153, tc1());
function tc2(a) {
try {
f_153([f_153, tc2, test]);
throw new Error("boom");
} catch(e) {
f_153([f_153, tc2, test]);
return f_153([f_153, test]);
}
}
assertEquals(153, tc2());
function tc3(a) {
try {
f_153([f_153, tc3, test]);
throw new Error("boom");
} catch(e) {
f_153([f_153, tc3, test]);
}
f_153([f_153, tc3, test]);
return f_153([f_153, test]);
}
assertEquals(153, tc3());
//
// try-finally
//
function tf1(a) {
try {
f_153([f_153, tf1, test]);
return f_153([f_153, tf1, test]);
} finally {
f_153([f_153, tf1, test]);
}
}
assertEquals(153, tf1());
function tf2(a) {
try {
f_153([f_153, tf2, test]);
throw new Error("boom");
} finally {
f_153([f_153, tf2, test]);
return f_153([f_153, test]);
}
}
assertEquals(153, tf2());
function tf3(a) {
try {
f_153([f_153, tf3, test]);
} finally {
f_153([f_153, tf3, test]);
}
return f_153([f_153, test]);
}
assertEquals(153, tf3());
//
// try-catch-finally
//
function tcf1(a) {
try {
f_153([f_153, tcf1, test]);
return f_153([f_153, tcf1, test]);
} catch(e) {
} finally {
f_153([f_153, tcf1, test]);
}
}
assertEquals(153, tcf1());
function tcf2(a) {
try {
f_153([f_153, tcf2, test]);
throw new Error("boom");
} catch(e) {
f_153([f_153, tcf2, test]);
return f_153([f_153, tcf2, test]);
} finally {
f_153([f_153, tcf2, test]);
}
}
assertEquals(153, tcf2());
function tcf3(a) {
try {
f_153([f_153, tcf3, test]);
throw new Error("boom");
} catch(e) {
f_153([f_153, tcf3, test]);
} finally {
f_153([f_153, tcf3, test]);
return f_153([f_153, test]);
}
}
assertEquals(153, tcf3());
function tcf4(a) {
try {
f_153([f_153, tcf4, test]);
throw new Error("boom");
} catch(e) {
f_153([f_153, tcf4, test]);
} finally {
f_153([f_153, tcf4, test]);
}
return f_153([f_153, test]);
}
assertEquals(153, tcf4());
})();
......@@ -45,6 +45,7 @@
# Issue 4698: not fully supported by Turbofan yet
'es6/tail-call-simple': [SKIP],
'es6/tail-call': [PASS, NO_VARIANTS],
# Issue 3389: deopt_every_n_garbage_collections is unsafe
'regress/regress-2653': [SKIP],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment