From 626454a61a64c2c2c74791e4e592ecd8c7196cf4 Mon Sep 17 00:00:00 2001 From: "fschneider@chromium.org" Date: Tue, 13 Dec 2011 17:10:34 +0000 Subject: [PATCH] [hydrogen] don't bailout assignments to consts If constant variable is allocated in CONTEXT Patch by Fedor Indutny . BUG= TEST= R=vegorov@chromium.org Review URL: http://codereview.chromium.org/8857001 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10244 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-codegen-arm.cc | 22 +++++++++-- src/hydrogen-instructions.h | 39 ++++++++++++++++---- src/hydrogen.cc | 79 +++++++++++++++++++++++++++------------- src/ia32/lithium-codegen-ia32.cc | 22 ++++++++++- src/mips/lithium-codegen-mips.cc | 26 +++++++++++-- src/x64/lithium-codegen-x64.cc | 21 ++++++++++- 6 files changed, 165 insertions(+), 44 deletions(-) diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 25532a2..cdc464c 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -2306,7 +2306,11 @@ void LCodeGen::DoLoadContextSlot(LLoadContextSlot* instr) { if (instr->hydrogen()->RequiresHoleCheck()) { __ LoadRoot(ip, Heap::kTheHoleValueRootIndex); __ cmp(result, ip); - DeoptimizeIf(eq, instr->environment()); + if (instr->hydrogen()->DeoptimizesOnHole()) { + DeoptimizeIf(eq, instr->environment()); + } else { + __ mov(result, Operand(factory()->undefined_value()), LeaveCC, eq); + } } } @@ -2314,14 +2318,22 @@ void LCodeGen::DoLoadContextSlot(LLoadContextSlot* instr) { void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) { Register context = ToRegister(instr->context()); Register value = ToRegister(instr->value()); + Register scratch = scratch0(); MemOperand target = ContextOperand(context, instr->slot_index()); + + Label skip_assignment; + if (instr->hydrogen()->RequiresHoleCheck()) { - Register scratch = scratch0(); __ ldr(scratch, target); __ LoadRoot(ip, Heap::kTheHoleValueRootIndex); __ cmp(scratch, ip); - DeoptimizeIf(eq, instr->environment()); + if (instr->hydrogen()->DeoptimizesOnHole()) { + DeoptimizeIf(eq, instr->environment()); + } else { + __ b(ne, &skip_assignment); + } } + __ str(value, target); if (instr->hydrogen()->NeedsWriteBarrier()) { HType type = instr->hydrogen()->value()->type(); @@ -2330,12 +2342,14 @@ void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) { __ RecordWriteContextSlot(context, target.offset(), value, - scratch0(), + scratch, kLRHasBeenSaved, kSaveFPRegs, EMIT_REMEMBERED_SET, check_needed); } + + __ bind(&skip_assignment); } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index d7c0eb0..75305ec 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -3449,28 +3449,45 @@ class HLoadContextSlot: public HUnaryOperation { public: enum Mode { // Perform a normal load of the context slot without checking its value. - kLoad, + kNoCheck, // Load and check the value of the context slot. Deoptimize if it's the // hole value. This is used for checking for loading of uninitialized // harmony bindings where we deoptimize into full-codegen generated code // which will subsequently throw a reference error. - kLoadCheck + kCheckDeoptimize, + // Load and check the value of the context slot. Return undefined if it's + // the hole value. This is used for non-harmony const assignments + kCheckReturnUndefined }; HLoadContextSlot(HValue* context, Variable* var) : HUnaryOperation(context), slot_index_(var->index()) { ASSERT(var->IsContextSlot()); - mode_ = (var->mode() == LET || var->mode() == CONST_HARMONY) - ? kLoadCheck : kLoad; + switch (var->mode()) { + case LET: + case CONST_HARMONY: + mode_ = kCheckDeoptimize; + break; + case CONST: + mode_ = kCheckReturnUndefined; + break; + default: + mode_ = kNoCheck; + } set_representation(Representation::Tagged()); SetFlag(kUseGVN); SetFlag(kDependsOnContextSlots); } int slot_index() const { return slot_index_; } + Mode mode() const { return mode_; } + + bool DeoptimizesOnHole() { + return mode_ == kCheckDeoptimize; + } bool RequiresHoleCheck() { - return mode_ == kLoadCheck; + return mode_ != kNoCheck; } virtual Representation RequiredInputRepresentation(int index) { @@ -3498,12 +3515,14 @@ class HStoreContextSlot: public HTemplateInstruction<2> { enum Mode { // Perform a normal store to the context slot without checking its previous // value. - kAssign, + kNoCheck, // Check the previous value of the context slot and deoptimize if it's the // hole value. This is used for checking for assignments to uninitialized // harmony bindings where we deoptimize into full-codegen generated code // which will subsequently throw a reference error. - kAssignCheck + kCheckDeoptimize, + // Check the previous value and ignore assignment if it isn't a hole value + kCheckIgnoreAssignment }; HStoreContextSlot(HValue* context, int slot_index, Mode mode, HValue* value) @@ -3522,8 +3541,12 @@ class HStoreContextSlot: public HTemplateInstruction<2> { return StoringValueNeedsWriteBarrier(value()); } + bool DeoptimizesOnHole() { + return mode_ == kCheckDeoptimize; + } + bool RequiresHoleCheck() { - return mode_ == kAssignCheck; + return mode_ != kNoCheck; } virtual Representation RequiredInputRepresentation(int index) { diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 3663237..5349514 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -3285,9 +3285,6 @@ void HGraphBuilder::VisitVariableProxy(VariableProxy* expr) { } case Variable::CONTEXT: { - if (variable->mode() == CONST) { - return Bailout("reference to const context slot"); - } HValue* context = BuildContextChainWalk(variable); HLoadContextSlot* instr = new(zone()) HLoadContextSlot(context, variable); return ast_context()->ReturnInstruction(instr, expr->id()); @@ -3805,8 +3802,8 @@ void HGraphBuilder::HandleCompoundAssignment(Assignment* expr) { if (proxy != NULL) { Variable* var = proxy->var(); - if (var->mode() == CONST || var->mode() == LET) { - return Bailout("unsupported let or const compound assignment"); + if (var->mode() == LET) { + return Bailout("unsupported let compound assignment"); } CHECK_ALIVE(VisitForValue(operation)); @@ -3821,6 +3818,9 @@ void HGraphBuilder::HandleCompoundAssignment(Assignment* expr) { case Variable::PARAMETER: case Variable::LOCAL: + if (var->mode() == CONST) { + return Bailout("unsupported const compound assignment"); + } Bind(var, Top()); break; @@ -3841,10 +3841,23 @@ void HGraphBuilder::HandleCompoundAssignment(Assignment* expr) { } } + HStoreContextSlot::Mode mode; + + switch (var->mode()) { + case LET: + mode = HStoreContextSlot::kCheckDeoptimize; + break; + case CONST: + return ast_context()->ReturnValue(Pop()); + case CONST_HARMONY: + // This case is checked statically so no need to + // perform checks here + UNREACHABLE(); + default: + mode = HStoreContextSlot::kNoCheck; + } + HValue* context = BuildContextChainWalk(var); - HStoreContextSlot::Mode mode = - (var->mode() == LET || var->mode() == CONST_HARMONY) - ? HStoreContextSlot::kAssignCheck : HStoreContextSlot::kAssign; HStoreContextSlot* instr = new(zone()) HStoreContextSlot(context, var->index(), mode, Top()); AddInstruction(instr); @@ -3955,17 +3968,19 @@ void HGraphBuilder::VisitAssignment(Assignment* expr) { HandlePropertyAssignment(expr); } else if (proxy != NULL) { Variable* var = proxy->var(); + if (var->mode() == CONST) { if (expr->op() != Token::INIT_CONST) { - return Bailout("non-initializer assignment to const"); + CHECK_ALIVE(VisitForValue(expr->value())); + return ast_context()->ReturnValue(Pop()); } - if (!var->IsStackAllocated()) { - return Bailout("assignment to const context slot"); + + if (var->IsStackAllocated()) { + // We insert a use of the old value to detect unsupported uses of const + // variables (e.g. initialization inside a loop). + HValue* old_value = environment()->Lookup(var); + AddInstruction(new HUseConst(old_value)); } - // We insert a use of the old value to detect unsupported uses of const - // variables (e.g. initialization inside a loop). - HValue* old_value = environment()->Lookup(var); - AddInstruction(new HUseConst(old_value)); } else if (var->mode() == CONST_HARMONY) { if (expr->op() != Token::INIT_CONST_HARMONY) { return Bailout("non-initializer assignment to const"); @@ -4004,7 +4019,6 @@ void HGraphBuilder::VisitAssignment(Assignment* expr) { } case Variable::CONTEXT: { - ASSERT(var->mode() != CONST); // Bail out if we try to mutate a parameter value in a function using // the arguments object. We do not (yet) correctly handle the // arguments property of the function. @@ -4020,17 +4034,32 @@ void HGraphBuilder::VisitAssignment(Assignment* expr) { } CHECK_ALIVE(VisitForValue(expr->value())); - HValue* context = BuildContextChainWalk(var); HStoreContextSlot::Mode mode; if (expr->op() == Token::ASSIGN) { - mode = (var->mode() == LET || var->mode() == CONST_HARMONY) - ? HStoreContextSlot::kAssignCheck : HStoreContextSlot::kAssign; + switch (var->mode()) { + case LET: + mode = HStoreContextSlot::kCheckDeoptimize; + break; + case CONST: + return ast_context()->ReturnValue(Pop()); + case CONST_HARMONY: + // This case is checked statically so no need to + // perform checks here + UNREACHABLE(); + default: + mode = HStoreContextSlot::kNoCheck; + } + } else if (expr->op() == Token::INIT_VAR || + expr->op() == Token::INIT_LET || + expr->op() == Token::INIT_CONST_HARMONY) { + mode = HStoreContextSlot::kNoCheck; } else { - ASSERT(expr->op() == Token::INIT_VAR || - expr->op() == Token::INIT_LET || - expr->op() == Token::INIT_CONST_HARMONY); - mode = HStoreContextSlot::kAssign; + ASSERT(expr->op() == Token::INIT_CONST); + + mode = HStoreContextSlot::kCheckIgnoreAssignment; } + + HValue* context = BuildContextChainWalk(var); HStoreContextSlot* instr = new(zone()) HStoreContextSlot( context, var->index(), mode, Top()); AddInstruction(instr); @@ -5643,7 +5672,7 @@ void HGraphBuilder::VisitCountOperation(CountOperation* expr) { HValue* context = BuildContextChainWalk(var); HStoreContextSlot::Mode mode = (var->mode() == LET || var->mode() == CONST_HARMONY) - ? HStoreContextSlot::kAssignCheck : HStoreContextSlot::kAssign; + ? HStoreContextSlot::kCheckDeoptimize : HStoreContextSlot::kNoCheck; HStoreContextSlot* instr = new(zone()) HStoreContextSlot(context, var->index(), mode, after); AddInstruction(instr); @@ -6251,7 +6280,7 @@ void HGraphBuilder::HandleDeclaration(VariableProxy* proxy, if (var->IsContextSlot()) { HValue* context = environment()->LookupContext(); HStoreContextSlot* store = new HStoreContextSlot( - context, var->index(), HStoreContextSlot::kAssign, value); + context, var->index(), HStoreContextSlot::kNoCheck, value); AddInstruction(store); if (store->HasObservableSideEffects()) AddSimulate(proxy->id()); } else { diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 23db874..7883481 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -2165,9 +2165,17 @@ void LCodeGen::DoLoadContextSlot(LLoadContextSlot* instr) { Register context = ToRegister(instr->context()); Register result = ToRegister(instr->result()); __ mov(result, ContextOperand(context, instr->slot_index())); + if (instr->hydrogen()->RequiresHoleCheck()) { __ cmp(result, factory()->the_hole_value()); - DeoptimizeIf(equal, instr->environment()); + if (instr->hydrogen()->DeoptimizesOnHole()) { + DeoptimizeIf(equal, instr->environment()); + } else { + Label is_not_hole; + __ j(not_equal, &is_not_hole, Label::kNear); + __ mov(result, factory()->undefined_value()); + __ bind(&is_not_hole); + } } } @@ -2175,11 +2183,19 @@ void LCodeGen::DoLoadContextSlot(LLoadContextSlot* instr) { void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) { Register context = ToRegister(instr->context()); Register value = ToRegister(instr->value()); + + Label skip_assignment; + Operand target = ContextOperand(context, instr->slot_index()); if (instr->hydrogen()->RequiresHoleCheck()) { __ cmp(target, factory()->the_hole_value()); - DeoptimizeIf(equal, instr->environment()); + if (instr->hydrogen()->DeoptimizesOnHole()) { + DeoptimizeIf(equal, instr->environment()); + } else { + __ j(not_equal, &skip_assignment, Label::kNear); + } } + __ mov(target, value); if (instr->hydrogen()->NeedsWriteBarrier()) { HType type = instr->hydrogen()->value()->type(); @@ -2195,6 +2211,8 @@ void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) { EMIT_REMEMBERED_SET, check_needed); } + + __ bind(&skip_assignment); } diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index aba7516..eaef6ff 100644 --- a/src/mips/lithium-codegen-mips.cc +++ b/src/mips/lithium-codegen-mips.cc @@ -2180,10 +2180,19 @@ void LCodeGen::DoStoreGlobalGeneric(LStoreGlobalGeneric* instr) { void LCodeGen::DoLoadContextSlot(LLoadContextSlot* instr) { Register context = ToRegister(instr->context()); Register result = ToRegister(instr->result()); + __ lw(result, ContextOperand(context, instr->slot_index())); if (instr->hydrogen()->RequiresHoleCheck()) { __ LoadRoot(at, Heap::kTheHoleValueRootIndex); - DeoptimizeIf(eq, instr->environment(), result, Operand(at)); + + if (instr->hydrogen()->DeoptimizesOnHole()) { + DeoptimizeIf(eq, instr->environment(), result, Operand(at)); + } else { + Label is_not_hole; + __ Branch(&is_not_hole, ne, result, Operand(at)); + __ LoadRoot(result, Heap::kUndefinedValueRootIndex); + __ bind(&is_not_hole); + } } } @@ -2191,13 +2200,22 @@ void LCodeGen::DoLoadContextSlot(LLoadContextSlot* instr) { void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) { Register context = ToRegister(instr->context()); Register value = ToRegister(instr->value()); + Register scratch = scratch0(); MemOperand target = ContextOperand(context, instr->slot_index()); + + Label skip_assignment; + if (instr->hydrogen()->RequiresHoleCheck()) { - Register scratch = scratch0(); __ lw(scratch, target); __ LoadRoot(at, Heap::kTheHoleValueRootIndex); - DeoptimizeIf(eq, instr->environment(), scratch, Operand(at)); + + if (instr->hydrogen()->DeoptimizesOnHole()) { + DeoptimizeIf(eq, instr->environment(), scratch, Operand(at)); + } else { + __ Branch(&skip_assignment, ne, scratch, Operand(at)); + } } + __ sw(value, target); if (instr->hydrogen()->NeedsWriteBarrier()) { HType type = instr->hydrogen()->value()->type(); @@ -2212,6 +2230,8 @@ void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) { EMIT_REMEMBERED_SET, check_needed); } + + __ bind(&skip_assignment); } diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 293a1db..2443140 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -2069,7 +2069,14 @@ void LCodeGen::DoLoadContextSlot(LLoadContextSlot* instr) { __ movq(result, ContextOperand(context, instr->slot_index())); if (instr->hydrogen()->RequiresHoleCheck()) { __ CompareRoot(result, Heap::kTheHoleValueRootIndex); - DeoptimizeIf(equal, instr->environment()); + if (instr->hydrogen()->DeoptimizesOnHole()) { + DeoptimizeIf(equal, instr->environment()); + } else { + Label is_not_hole; + __ j(not_equal, &is_not_hole, Label::kNear); + __ movq(result, factory()->undefined_value(), RelocInfo::NONE); + __ bind(&is_not_hole); + } } } @@ -2077,12 +2084,20 @@ void LCodeGen::DoLoadContextSlot(LLoadContextSlot* instr) { void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) { Register context = ToRegister(instr->context()); Register value = ToRegister(instr->value()); + Operand target = ContextOperand(context, instr->slot_index()); + + Label skip_assignment; if (instr->hydrogen()->RequiresHoleCheck()) { __ CompareRoot(target, Heap::kTheHoleValueRootIndex); - DeoptimizeIf(equal, instr->environment()); + if (instr->hydrogen()->DeoptimizesOnHole()) { + DeoptimizeIf(equal, instr->environment()); + } else { + __ j(not_equal, &skip_assignment, Label::kNear); + } } __ movq(target, value); + if (instr->hydrogen()->NeedsWriteBarrier()) { HType type = instr->hydrogen()->value()->type(); SmiCheck check_needed = @@ -2097,6 +2112,8 @@ void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) { EMIT_REMEMBERED_SET, check_needed); } + + __ bind(&skip_assignment); } -- 2.7.4