Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
[crankshaft] Fixing ES6 tail call elimination.
Browse files Browse the repository at this point in the history 
In case when F inlined normal call to G which tail calls H we should not write translation for G for the tail call site.
Otherwise we will see G in a stack trace inside H.

This CL also enables all existing tests related to ES6 tail call elimination.

[email protected]
BUG=v8:4698
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#34830}
  • Loading branch information
@isheludko
isheludko authored and Commit bot committed Mar 16, 2016
1 parent b64bcb4 commit 689980f
Show file tree
Hide file tree
Showing 32 changed files with 330 additions and 165 deletions.
40 changes: 27 additions & 13 deletions src/crankshaft/arm/lithium-arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -546,12 +546,7 @@ LInstruction* LChunkBuilder::DefineFixedDouble(

LInstruction* LChunkBuilder::AssignEnvironment(LInstruction* instr) {
HEnvironment* hydrogen_env = current_block_->last_environment();
int argument_index_accumulator = 0;
ZoneList<HValue*> objects_to_materialize(0, zone());
instr->set_environment(CreateEnvironment(hydrogen_env,
&argument_index_accumulator,
&objects_to_materialize));
return instr;
return LChunkBuilderBase::AssignEnvironment(instr, hydrogen_env);
}


Expand Down Expand Up @@ -877,15 +872,28 @@ void LChunkBuilder::AddInstruction(LInstruction* instr,
chunk_->AddInstruction(instr, current_block_);

if (instr->IsCall()) {
HEnvironment* hydrogen_env = current_block_->last_environment();
HValue* hydrogen_value_for_lazy_bailout = hydrogen_val;
if (hydrogen_val->HasObservableSideEffects()) {
HSimulate* sim = HSimulate::cast(hydrogen_val->next());
sim->ReplayEnvironment(current_block_->last_environment());
hydrogen_value_for_lazy_bailout = sim;
DCHECK_NOT_NULL(hydrogen_env);
if (instr->IsTailCall()) {
hydrogen_env = hydrogen_env->outer();
if (hydrogen_env != nullptr &&
hydrogen_env->frame_type() == ARGUMENTS_ADAPTOR) {
hydrogen_env = hydrogen_env->outer();
}
} else {
if (hydrogen_val->HasObservableSideEffects()) {
HSimulate* sim = HSimulate::cast(hydrogen_val->next());
sim->ReplayEnvironment(hydrogen_env);
hydrogen_value_for_lazy_bailout = sim;
}
}
if (hydrogen_env != nullptr) {
LInstruction* bailout = LChunkBuilderBase::AssignEnvironment(
new (zone()) LLazyBailout(), hydrogen_env);
bailout->set_hydrogen_value(hydrogen_value_for_lazy_bailout);
chunk_->AddInstruction(bailout, current_block_);
}
LInstruction* bailout = AssignEnvironment(new(zone()) LLazyBailout());
bailout->set_hydrogen_value(hydrogen_value_for_lazy_bailout);
chunk_->AddInstruction(bailout, current_block_);
}
}

Expand Down Expand Up @@ -1065,6 +1073,9 @@ LInstruction* LChunkBuilder::DoCallWithDescriptor(

LCallWithDescriptor* result = new(zone()) LCallWithDescriptor(
descriptor, ops, zone());
if (instr->syntactic_tail_call_mode() == TailCallMode::kAllow) {
result->MarkAsTailCall();
}
return MarkAsCall(DefineFixed(result, r0), instr);
}

Expand All @@ -1073,6 +1084,9 @@ LInstruction* LChunkBuilder::DoInvokeFunction(HInvokeFunction* instr) {
LOperand* context = UseFixed(instr->context(), cp);
LOperand* function = UseFixed(instr->function(), r1);
LInvokeFunction* result = new(zone()) LInvokeFunction(context, function);
if (instr->syntactic_tail_call_mode() == TailCallMode::kAllow) {
result->MarkAsTailCall();
}
return MarkAsCall(DefineFixed(result, r0), instr, CANNOT_DEOPTIMIZE_EAGERLY);
}

Expand Down
6 changes: 6 additions & 0 deletions src/crankshaft/arm/lithium-arm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ class LInstruction : public ZoneObject {
void MarkAsCall() { bit_field_ = IsCallBits::update(bit_field_, true); }
bool IsCall() const { return IsCallBits::decode(bit_field_); }

void MarkAsTailCall() {
bit_field_ = IsTailCallBits::update(bit_field_, true);
}
bool IsTailCall() const { return IsTailCallBits::decode(bit_field_); }

// Interface to the register allocator and iterators.
bool ClobbersTemps() const { return IsCall(); }
bool ClobbersRegisters() const { return IsCall(); }
Expand Down Expand Up @@ -258,6 +263,7 @@ class LInstruction : public ZoneObject {
virtual LOperand* TempAt(int i) = 0;

class IsCallBits: public BitField<bool, 0, 1> {};
class IsTailCallBits : public BitField<bool, IsCallBits::kNext, 1> {};

LEnvironment* environment_;
SetOncePointer<LPointerMap> pointer_map_;
Expand Down
40 changes: 27 additions & 13 deletions src/crankshaft/arm64/lithium-arm64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,27 +715,35 @@ void LChunkBuilder::AddInstruction(LInstruction* instr,
chunk_->AddInstruction(instr, current_block_);

if (instr->IsCall()) {
HEnvironment* hydrogen_env = current_block_->last_environment();
HValue* hydrogen_value_for_lazy_bailout = hydrogen_val;
if (hydrogen_val->HasObservableSideEffects()) {
HSimulate* sim = HSimulate::cast(hydrogen_val->next());
sim->ReplayEnvironment(current_block_->last_environment());
hydrogen_value_for_lazy_bailout = sim;
DCHECK_NOT_NULL(hydrogen_env);
if (instr->IsTailCall()) {
hydrogen_env = hydrogen_env->outer();
if (hydrogen_env != nullptr &&
hydrogen_env->frame_type() == ARGUMENTS_ADAPTOR) {
hydrogen_env = hydrogen_env->outer();
}
} else {
if (hydrogen_val->HasObservableSideEffects()) {
HSimulate* sim = HSimulate::cast(hydrogen_val->next());
sim->ReplayEnvironment(hydrogen_env);
hydrogen_value_for_lazy_bailout = sim;
}
}
if (hydrogen_env != nullptr) {
LInstruction* bailout = LChunkBuilderBase::AssignEnvironment(
new (zone()) LLazyBailout(), hydrogen_env);
bailout->set_hydrogen_value(hydrogen_value_for_lazy_bailout);
chunk_->AddInstruction(bailout, current_block_);
}
LInstruction* bailout = AssignEnvironment(new(zone()) LLazyBailout());
bailout->set_hydrogen_value(hydrogen_value_for_lazy_bailout);
chunk_->AddInstruction(bailout, current_block_);
}
}


LInstruction* LChunkBuilder::AssignEnvironment(LInstruction* instr) {
HEnvironment* hydrogen_env = current_block_->last_environment();
int argument_index_accumulator = 0;
ZoneList<HValue*> objects_to_materialize(0, zone());
instr->set_environment(CreateEnvironment(hydrogen_env,
&argument_index_accumulator,
&objects_to_materialize));
return instr;
return LChunkBuilderBase::AssignEnvironment(instr, hydrogen_env);
}


Expand Down Expand Up @@ -1018,6 +1026,9 @@ LInstruction* LChunkBuilder::DoCallWithDescriptor(
LCallWithDescriptor* result = new(zone()) LCallWithDescriptor(descriptor,
ops,
zone());
if (instr->syntactic_tail_call_mode() == TailCallMode::kAllow) {
result->MarkAsTailCall();
}
return MarkAsCall(DefineFixed(result, x0), instr);
}

Expand Down Expand Up @@ -1510,6 +1521,9 @@ LInstruction* LChunkBuilder::DoInvokeFunction(HInvokeFunction* instr) {
// The function is required (by MacroAssembler::InvokeFunction) to be in x1.
LOperand* function = UseFixed(instr->function(), x1);
LInvokeFunction* result = new(zone()) LInvokeFunction(context, function);
if (instr->syntactic_tail_call_mode() == TailCallMode::kAllow) {
result->MarkAsTailCall();
}
return MarkAsCall(DefineFixed(result, x0), instr, CANNOT_DEOPTIMIZE_EAGERLY);
}

Expand Down
6 changes: 6 additions & 0 deletions src/crankshaft/arm64/lithium-arm64.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,11 @@ class LInstruction : public ZoneObject {
void MarkAsCall() { bit_field_ = IsCallBits::update(bit_field_, true); }
bool IsCall() const { return IsCallBits::decode(bit_field_); }

void MarkAsTailCall() {
bit_field_ = IsTailCallBits::update(bit_field_, true);
}
bool IsTailCall() const { return IsTailCallBits::decode(bit_field_); }

// Interface to the register allocator and iterators.
bool ClobbersTemps() const { return IsCall(); }
bool ClobbersRegisters() const { return IsCall(); }
Expand Down Expand Up @@ -262,6 +267,7 @@ class LInstruction : public ZoneObject {

private:
class IsCallBits: public BitField<bool, 0, 1> {};
class IsTailCallBits : public BitField<bool, IsCallBits::kNext, 1> {};

LEnvironment* environment_;
SetOncePointer<LPointerMap> pointer_map_;
Expand Down
18 changes: 17 additions & 1 deletion src/crankshaft/hydrogen-instructions.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,18 @@ std::ostream& HBinaryCall::PrintDataTo(std::ostream& os) const { // NOLINT
<< argument_count();
}

std::ostream& HInvokeFunction::PrintTo(std::ostream& os) const { // NOLINT
if (tail_call_mode() == TailCallMode::kAllow) os << "Tail";
return HBinaryCall::PrintTo(os);
}

std::ostream& HInvokeFunction::PrintDataTo(std::ostream& os) const { // NOLINT
HBinaryCall::PrintDataTo(os);
if (syntactic_tail_call_mode() == TailCallMode::kAllow) {
os << ", JSTailCall";
}
return os;
}

void HBoundsCheck::ApplyIndexChange() {
if (skip_check()) return;
Expand Down Expand Up @@ -1033,7 +1045,11 @@ std::ostream& HCallWithDescriptor::PrintDataTo(
for (int i = 0; i < OperandCount(); i++) {
os << NameOf(OperandAt(i)) << " ";
}
return os << "#" << argument_count();
os << "#" << argument_count();
if (syntactic_tail_call_mode() == TailCallMode::kAllow) {
os << ", JSTailCall";
}
return os;
}


Expand Down
71 changes: 51 additions & 20 deletions src/crankshaft/hydrogen-instructions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2214,19 +2214,17 @@ class HBinaryCall : public HCall<2> {
};


enum CallMode { NORMAL_CALL, TAIL_CALL };


class HCallWithDescriptor final : public HInstruction {
public:
static HCallWithDescriptor* New(Isolate* isolate, Zone* zone, HValue* context,
HValue* target, int argument_count,
CallInterfaceDescriptor descriptor,
const Vector<HValue*>& operands,
CallMode call_mode = NORMAL_CALL) {
HCallWithDescriptor* res = new (zone) HCallWithDescriptor(
target, argument_count, descriptor, operands, call_mode, zone);
DCHECK_EQ(operands.length(), res->GetParameterCount());
static HCallWithDescriptor* New(
Isolate* isolate, Zone* zone, HValue* context, HValue* target,
int argument_count, CallInterfaceDescriptor descriptor,
const Vector<HValue*>& operands,
TailCallMode syntactic_tail_call_mode = TailCallMode::kDisallow,
TailCallMode tail_call_mode = TailCallMode::kDisallow) {
HCallWithDescriptor* res = new (zone)
HCallWithDescriptor(target, argument_count, descriptor, operands,
syntactic_tail_call_mode, tail_call_mode, zone);
return res;
}

Expand All @@ -2248,7 +2246,16 @@ class HCallWithDescriptor final : public HInstruction {

HType CalculateInferredType() final { return HType::Tagged(); }

bool IsTailCall() const { return call_mode_ == TAIL_CALL; }
// Defines whether this instruction corresponds to a JS call at tail position.
TailCallMode syntactic_tail_call_mode() const {
return SyntacticTailCallModeField::decode(bit_field_);
}

// Defines whether this call should be generated as a tail call.
TailCallMode tail_call_mode() const {
return TailCallModeField::decode(bit_field_);
}
bool IsTailCall() const { return tail_call_mode() == TailCallMode::kAllow; }

virtual int argument_count() const {
return argument_count_;
Expand All @@ -2268,14 +2275,18 @@ class HCallWithDescriptor final : public HInstruction {
// The argument count includes the receiver.
HCallWithDescriptor(HValue* target, int argument_count,
CallInterfaceDescriptor descriptor,
const Vector<HValue*>& operands, CallMode call_mode,
Zone* zone)
const Vector<HValue*>& operands,
TailCallMode syntactic_tail_call_mode,
TailCallMode tail_call_mode, Zone* zone)
: descriptor_(descriptor),
values_(GetParameterCount() + 1, zone),
argument_count_(argument_count),
call_mode_(call_mode) {
bit_field_(
TailCallModeField::encode(tail_call_mode) |
SyntacticTailCallModeField::encode(syntactic_tail_call_mode)) {
DCHECK_EQ(operands.length(), GetParameterCount());
// We can only tail call without any stack arguments.
DCHECK(call_mode != TAIL_CALL || argument_count == 0);
DCHECK(tail_call_mode != TailCallMode::kAllow || argument_count == 0);
AddOperand(target, zone);
for (int i = 0; i < operands.length(); i++) {
AddOperand(operands[i], zone);
Expand All @@ -2300,39 +2311,57 @@ class HCallWithDescriptor final : public HInstruction {
CallInterfaceDescriptor descriptor_;
ZoneList<HValue*> values_;
int argument_count_;
CallMode call_mode_;
class TailCallModeField : public BitField<TailCallMode, 0, 1> {};
class SyntacticTailCallModeField
: public BitField<TailCallMode, TailCallModeField::kNext, 1> {};
uint32_t bit_field_;
};


class HInvokeFunction final : public HBinaryCall {
public:
DECLARE_INSTRUCTION_WITH_CONTEXT_FACTORY_P4(HInvokeFunction, HValue*,
DECLARE_INSTRUCTION_WITH_CONTEXT_FACTORY_P5(HInvokeFunction, HValue*,
Handle<JSFunction>, int,
TailCallMode);
TailCallMode, TailCallMode);

HValue* context() { return first(); }
HValue* function() { return second(); }
Handle<JSFunction> known_function() { return known_function_; }
int formal_parameter_count() const { return formal_parameter_count_; }

bool HasStackCheck() final { return HasStackCheckField::decode(bit_field_); }

// Defines whether this instruction corresponds to a JS call at tail position.
TailCallMode syntactic_tail_call_mode() const {
return SyntacticTailCallModeField::decode(bit_field_);
}

// Defines whether this call should be generated as a tail call.
TailCallMode tail_call_mode() const {
return TailCallModeField::decode(bit_field_);
}

DECLARE_CONCRETE_INSTRUCTION(InvokeFunction)

std::ostream& PrintTo(std::ostream& os) const override; // NOLINT
std::ostream& PrintDataTo(std::ostream& os) const override; // NOLINT

private:
void set_has_stack_check(bool has_stack_check) {
bit_field_ = HasStackCheckField::update(bit_field_, has_stack_check);
}

HInvokeFunction(HValue* context, HValue* function,
Handle<JSFunction> known_function, int argument_count,
TailCallMode syntactic_tail_call_mode,
TailCallMode tail_call_mode)
: HBinaryCall(context, function, argument_count),
known_function_(known_function),
bit_field_(TailCallModeField::encode(tail_call_mode)) {
bit_field_(
TailCallModeField::encode(tail_call_mode) |
SyntacticTailCallModeField::encode(syntactic_tail_call_mode)) {
DCHECK(tail_call_mode != TailCallMode::kAllow ||
syntactic_tail_call_mode == TailCallMode::kAllow);
formal_parameter_count_ =
known_function.is_null()
? 0
Expand All @@ -2349,6 +2378,8 @@ class HInvokeFunction final : public HBinaryCall {
class HasStackCheckField : public BitField<bool, 0, 1> {};
class TailCallModeField
: public BitField<TailCallMode, HasStackCheckField::kNext, 1> {};
class SyntacticTailCallModeField
: public BitField<TailCallMode, TailCallModeField::kNext, 1> {};
uint32_t bit_field_;
};

Expand Down
Loading

0 comments on commit 689980f

Please sign in to comment.