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

Commit

Permalink
Revert of [crankshaft] Fixing ES6 tail call elimination. (patchset #7 …
Browse files Browse the repository at this point in the history 
…id:200001 of https://codereview.chromium.org/1780043004/ )

Reason for revert:
[Sheriff] Leads to mac gc stress crashes:
https://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/4975

Original issue's description:
> [crankshaft] Fixing ES6 tail call elimination.
>
> 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
>
> Committed: https://crrev.com/689980f7d4dfd4c29492f616d7b616b86ec9af91
> Cr-Commit-Position: refs/heads/master@{#34830}

[email protected],[email protected]
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4698

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

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

LInstruction* LChunkBuilder::AssignEnvironment(LInstruction* instr) {
HEnvironment* hydrogen_env = current_block_->last_environment();
return LChunkBuilderBase::AssignEnvironment(instr, hydrogen_env);
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;
}


Expand Down Expand Up @@ -872,28 +877,15 @@ 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;
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_);
if (hydrogen_val->HasObservableSideEffects()) {
HSimulate* sim = HSimulate::cast(hydrogen_val->next());
sim->ReplayEnvironment(current_block_->last_environment());
hydrogen_value_for_lazy_bailout = sim;
}
LInstruction* bailout = AssignEnvironment(new(zone()) LLazyBailout());
bailout->set_hydrogen_value(hydrogen_value_for_lazy_bailout);
chunk_->AddInstruction(bailout, current_block_);
}
}

Expand Down Expand Up @@ -1073,9 +1065,6 @@ 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 @@ -1084,9 +1073,6 @@ 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: 0 additions & 6 deletions src/crankshaft/arm/lithium-arm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,6 @@ 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 @@ -263,7 +258,6 @@ 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: 13 additions & 27 deletions src/crankshaft/arm64/lithium-arm64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,35 +715,27 @@ 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;
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_);
if (hydrogen_val->HasObservableSideEffects()) {
HSimulate* sim = HSimulate::cast(hydrogen_val->next());
sim->ReplayEnvironment(current_block_->last_environment());
hydrogen_value_for_lazy_bailout = sim;
}
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();
return LChunkBuilderBase::AssignEnvironment(instr, hydrogen_env);
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;
}


Expand Down Expand Up @@ -1026,9 +1018,6 @@ 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 @@ -1521,9 +1510,6 @@ 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: 0 additions & 6 deletions src/crankshaft/arm64/lithium-arm64.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,6 @@ 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 @@ -267,7 +262,6 @@ 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: 1 addition & 17 deletions src/crankshaft/hydrogen-instructions.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -911,18 +911,6 @@ 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 @@ -1045,11 +1033,7 @@ std::ostream& HCallWithDescriptor::PrintDataTo(
for (int i = 0; i < OperandCount(); i++) {
os << NameOf(OperandAt(i)) << " ";
}
os << "#" << argument_count();
if (syntactic_tail_call_mode() == TailCallMode::kAllow) {
os << ", JSTailCall";
}
return os;
return os << "#" << argument_count();
}


Expand Down
71 changes: 20 additions & 51 deletions src/crankshaft/hydrogen-instructions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2214,17 +2214,19 @@ 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,
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);
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());
return res;
}

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

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

// 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; }
bool IsTailCall() const { return call_mode_ == TAIL_CALL; }

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


class HInvokeFunction final : public HBinaryCall {
public:
DECLARE_INSTRUCTION_WITH_CONTEXT_FACTORY_P5(HInvokeFunction, HValue*,
DECLARE_INSTRUCTION_WITH_CONTEXT_FACTORY_P4(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) |
SyntacticTailCallModeField::encode(syntactic_tail_call_mode)) {
DCHECK(tail_call_mode != TailCallMode::kAllow ||
syntactic_tail_call_mode == TailCallMode::kAllow);
bit_field_(TailCallModeField::encode(tail_call_mode)) {
formal_parameter_count_ =
known_function.is_null()
? 0
Expand All @@ -2378,8 +2349,6 @@ 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 d64b41d

Please sign in to comment.