From: fschneider@chromium.org Date: Tue, 27 Sep 2011 13:03:19 +0000 (+0000) Subject: Improve our simple elimination of hole checks. X-Git-Tag: upstream/4.7.83~18347 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=27e1a8d41435a900ce583abaeed4d1425948932f;p=platform%2Fupstream%2Fv8.git Improve our simple elimination of hole checks. Currently we avoid checking for the hole value after array loads, if the result is only used by instructions that definitely deoptimize in case of the hole value (HChange instructions). This change performs the same procedure for loading from deleteable/read-only global variable where we can also avoid the check in the same cases. Review URL: http://codereview.chromium.org/8054008 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9453 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index e7b477f36..84959397b 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -1739,7 +1739,7 @@ LInstruction* LChunkBuilder::DoConstant(HConstant* instr) { LInstruction* LChunkBuilder::DoLoadGlobalCell(HLoadGlobalCell* instr) { LLoadGlobalCell* result = new LLoadGlobalCell; - return instr->check_hole_value() + return instr->RequiresHoleCheck() ? AssignEnvironment(DefineAsRegister(result)) : DefineAsRegister(result); } @@ -1756,7 +1756,7 @@ LInstruction* LChunkBuilder::DoStoreGlobalCell(HStoreGlobalCell* instr) { LOperand* temp = TempRegister(); LOperand* value = UseTempRegister(instr->value()); LInstruction* result = new LStoreGlobalCell(value, temp); - if (instr->check_hole_value()) result = AssignEnvironment(result); + if (instr->RequiresHoleCheck()) result = AssignEnvironment(result); return result; } diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index dc6749ccf..f6750a299 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -2207,7 +2207,7 @@ void LCodeGen::DoLoadGlobalCell(LLoadGlobalCell* instr) { Register result = ToRegister(instr->result()); __ mov(ip, Operand(Handle(instr->hydrogen()->cell()))); __ ldr(result, FieldMemOperand(ip, JSGlobalPropertyCell::kValueOffset)); - if (instr->hydrogen()->check_hole_value()) { + if (instr->hydrogen()->RequiresHoleCheck()) { __ LoadRoot(ip, Heap::kTheHoleValueRootIndex); __ cmp(result, ip); DeoptimizeIf(eq, instr->environment()); @@ -2239,7 +2239,7 @@ void LCodeGen::DoStoreGlobalCell(LStoreGlobalCell* instr) { // been deleted from the property dictionary. In that case, we need // to update the property details in the property dictionary to mark // it as no longer deleted. - if (instr->hydrogen()->check_hole_value()) { + if (instr->hydrogen()->RequiresHoleCheck()) { __ ldr(scratch2, FieldMemOperand(scratch, JSGlobalPropertyCell::kValueOffset)); __ LoadRoot(ip, Heap::kTheHoleValueRootIndex); @@ -2541,13 +2541,9 @@ void LCodeGen::DoLoadKeyedFastDoubleElement( Operand(FixedDoubleArray::kHeaderSize - kHeapObjectTag)); } - if (instr->hydrogen()->RequiresHoleCheck()) { - // TODO(danno): If no hole check is required, there is no need to allocate - // elements into a temporary register, instead scratch can be used. - __ ldr(scratch, MemOperand(elements, sizeof(kHoleNanLower32))); - __ cmp(scratch, Operand(kHoleNanUpper32)); - DeoptimizeIf(eq, instr->environment()); - } + __ ldr(scratch, MemOperand(elements, sizeof(kHoleNanLower32))); + __ cmp(scratch, Operand(kHoleNanUpper32)); + DeoptimizeIf(eq, instr->environment()); __ vldr(result, elements, 0); } diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 1497526f2..d7e26da2e 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -1464,7 +1464,7 @@ void HLoadKeyedFastElement::PrintDataTo(StringStream* stream) { } -bool HLoadKeyedFastElement::RequiresHoleCheck() const { +bool HLoadKeyedFastElement::RequiresHoleCheck() { for (HUseIterator it(uses()); !it.Done(); it.Advance()) { HValue* use = it.value(); if (!use->IsChange()) return true; @@ -1481,11 +1481,6 @@ void HLoadKeyedFastDoubleElement::PrintDataTo(StringStream* stream) { } -bool HLoadKeyedFastDoubleElement::RequiresHoleCheck() const { - return true; -} - - void HLoadKeyedGeneric::PrintDataTo(StringStream* stream) { object()->PrintNameTo(stream); stream->Add("["); @@ -1639,7 +1634,18 @@ void HStoreKeyedSpecializedArrayElement::PrintDataTo( void HLoadGlobalCell::PrintDataTo(StringStream* stream) { stream->Add("[%p]", *cell()); - if (check_hole_value()) stream->Add(" (deleteable/read-only)"); + if (!details_.IsDontDelete()) stream->Add(" (deleteable)"); + if (details_.IsReadOnly()) stream->Add(" (read-only)"); +} + + +bool HLoadGlobalCell::RequiresHoleCheck() { + if (details_.IsDontDelete() && !details_.IsReadOnly()) return false; + for (HUseIterator it(uses()); !it.Done(); it.Advance()) { + HValue* use = it.value(); + if (!use->IsChange()) return true; + } + return false; } @@ -1651,6 +1657,8 @@ void HLoadGlobalGeneric::PrintDataTo(StringStream* stream) { void HStoreGlobalCell::PrintDataTo(StringStream* stream) { stream->Add("[%p] = ", *cell()); value()->PrintNameTo(stream); + if (!details_.IsDontDelete()) stream->Add(" (deleteable)"); + if (details_.IsReadOnly()) stream->Add(" (read-only)"); } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index c4b44e896..18fda5de6 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -3201,15 +3201,15 @@ class HUnknownOSRValue: public HTemplateInstruction<0> { class HLoadGlobalCell: public HTemplateInstruction<0> { public: - HLoadGlobalCell(Handle cell, bool check_hole_value) - : cell_(cell), check_hole_value_(check_hole_value) { + HLoadGlobalCell(Handle cell, PropertyDetails details) + : cell_(cell), details_(details) { set_representation(Representation::Tagged()); SetFlag(kUseGVN); SetFlag(kDependsOnGlobalVars); } Handle cell() const { return cell_; } - bool check_hole_value() const { return check_hole_value_; } + bool RequiresHoleCheck(); virtual void PrintDataTo(StringStream* stream); @@ -3232,7 +3232,7 @@ class HLoadGlobalCell: public HTemplateInstruction<0> { private: Handle cell_; - bool check_hole_value_; + PropertyDetails details_; }; @@ -3273,15 +3273,17 @@ class HStoreGlobalCell: public HUnaryOperation { public: HStoreGlobalCell(HValue* value, Handle cell, - bool check_hole_value) + PropertyDetails details) : HUnaryOperation(value), cell_(cell), - check_hole_value_(check_hole_value) { + details_(details) { SetFlag(kChangesGlobalVars); } Handle cell() const { return cell_; } - bool check_hole_value() const { return check_hole_value_; } + bool RequiresHoleCheck() { + return !details_.IsDontDelete() || details_.IsReadOnly(); + } virtual Representation RequiredInputRepresentation(int index) const { return Representation::Tagged(); @@ -3292,7 +3294,7 @@ class HStoreGlobalCell: public HUnaryOperation { private: Handle cell_; - bool check_hole_value_; + PropertyDetails details_; }; @@ -3543,7 +3545,7 @@ class HLoadKeyedFastElement: public HTemplateInstruction<2> { virtual void PrintDataTo(StringStream* stream); - bool RequiresHoleCheck() const; + bool RequiresHoleCheck(); DECLARE_CONCRETE_INSTRUCTION(LoadKeyedFastElement) @@ -3574,8 +3576,6 @@ class HLoadKeyedFastDoubleElement: public HTemplateInstruction<2> { virtual void PrintDataTo(StringStream* stream); - bool RequiresHoleCheck() const; - DECLARE_CONCRETE_INSTRUCTION(LoadKeyedFastDoubleElement) protected: diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 96dd0ee0e..c62b9833c 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -3168,8 +3168,8 @@ void HGraphBuilder::VisitVariableProxy(VariableProxy* expr) { if (type == kUseCell) { Handle global(info()->global_object()); Handle cell(global->GetPropertyCell(&lookup)); - bool check_hole = !lookup.IsDontDelete() || lookup.IsReadOnly(); - HLoadGlobalCell* instr = new(zone()) HLoadGlobalCell(cell, check_hole); + HLoadGlobalCell* instr = + new(zone()) HLoadGlobalCell(cell, lookup.GetPropertyDetails()); return ast_context()->ReturnInstruction(instr, expr->id()); } else { HValue* context = environment()->LookupContext(); @@ -3630,10 +3630,10 @@ void HGraphBuilder::HandleGlobalVariableAssignment(Variable* var, LookupResult lookup; GlobalPropertyAccess type = LookupGlobalProperty(var, &lookup, true); if (type == kUseCell) { - bool check_hole = !lookup.IsDontDelete() || lookup.IsReadOnly(); Handle global(info()->global_object()); Handle cell(global->GetPropertyCell(&lookup)); - HInstruction* instr = new(zone()) HStoreGlobalCell(value, cell, check_hole); + HInstruction* instr = + new(zone()) HStoreGlobalCell(value, cell, lookup.GetPropertyDetails()); instr->set_position(position); AddInstruction(instr); if (instr->HasSideEffects()) AddSimulate(ast_id); diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 62fcd74dc..9e1fd34af 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -2073,7 +2073,7 @@ void LCodeGen::DoReturn(LReturn* instr) { void LCodeGen::DoLoadGlobalCell(LLoadGlobalCell* instr) { Register result = ToRegister(instr->result()); __ mov(result, Operand::Cell(instr->hydrogen()->cell())); - if (instr->hydrogen()->check_hole_value()) { + if (instr->hydrogen()->RequiresHoleCheck()) { __ cmp(result, factory()->the_hole_value()); DeoptimizeIf(equal, instr->environment()); } @@ -2107,7 +2107,7 @@ void LCodeGen::DoStoreGlobalCell(LStoreGlobalCell* instr) { // been deleted from the property dictionary. In that case, we need // to update the property details in the property dictionary to mark // it as no longer deleted. We deoptimize in that case. - if (instr->hydrogen()->check_hole_value()) { + if (instr->hydrogen()->RequiresHoleCheck()) { __ cmp(FieldOperand(object, offset), factory()->the_hole_value()); DeoptimizeIf(equal, instr->environment()); } @@ -2369,16 +2369,14 @@ void LCodeGen::DoLoadKeyedFastDoubleElement( LLoadKeyedFastDoubleElement* instr) { XMMRegister result = ToDoubleRegister(instr->result()); - if (instr->hydrogen()->RequiresHoleCheck()) { - int offset = FixedDoubleArray::kHeaderSize - kHeapObjectTag + - sizeof(kHoleNanLower32); - Operand hole_check_operand = BuildFastArrayOperand( - instr->elements(), instr->key(), - FAST_DOUBLE_ELEMENTS, - offset); - __ cmp(hole_check_operand, Immediate(kHoleNanUpper32)); - DeoptimizeIf(equal, instr->environment()); - } + int offset = FixedDoubleArray::kHeaderSize - kHeapObjectTag + + sizeof(kHoleNanLower32); + Operand hole_check_operand = BuildFastArrayOperand( + instr->elements(), instr->key(), + FAST_DOUBLE_ELEMENTS, + offset); + __ cmp(hole_check_operand, Immediate(kHoleNanUpper32)); + DeoptimizeIf(equal, instr->environment()); Operand double_load_operand = BuildFastArrayOperand( instr->elements(), instr->key(), FAST_DOUBLE_ELEMENTS, diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc index 77a6bfce3..856106c79 100644 --- a/src/ia32/lithium-ia32.cc +++ b/src/ia32/lithium-ia32.cc @@ -1785,7 +1785,7 @@ LInstruction* LChunkBuilder::DoConstant(HConstant* instr) { LInstruction* LChunkBuilder::DoLoadGlobalCell(HLoadGlobalCell* instr) { LLoadGlobalCell* result = new LLoadGlobalCell; - return instr->check_hole_value() + return instr->RequiresHoleCheck() ? AssignEnvironment(DefineAsRegister(result)) : DefineAsRegister(result); } @@ -1804,7 +1804,7 @@ LInstruction* LChunkBuilder::DoStoreGlobalCell(HStoreGlobalCell* instr) { new LStoreGlobalCell(UseTempRegister(instr->value()), TempRegister(), TempRegister()); - return instr->check_hole_value() ? AssignEnvironment(result) : result; + return instr->RequiresHoleCheck() ? AssignEnvironment(result) : result; } diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 8c058b0d1..45aaad754 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -2015,7 +2015,7 @@ void LCodeGen::DoLoadGlobalCell(LLoadGlobalCell* instr) { __ movq(result, instr->hydrogen()->cell(), RelocInfo::GLOBAL_PROPERTY_CELL); __ movq(result, Operand(result, 0)); } - if (instr->hydrogen()->check_hole_value()) { + if (instr->hydrogen()->RequiresHoleCheck()) { __ CompareRoot(result, Heap::kTheHoleValueRootIndex); DeoptimizeIf(equal, instr->environment()); } @@ -2047,7 +2047,7 @@ void LCodeGen::DoStoreGlobalCell(LStoreGlobalCell* instr) { // been deleted from the property dictionary. In that case, we need // to update the property details in the property dictionary to mark // it as no longer deleted. We deoptimize in that case. - if (instr->hydrogen()->check_hole_value()) { + if (instr->hydrogen()->RequiresHoleCheck()) { __ CompareRoot(Operand(address, 0), Heap::kTheHoleValueRootIndex); DeoptimizeIf(equal, instr->environment()); } @@ -2316,17 +2316,15 @@ void LCodeGen::DoLoadKeyedFastDoubleElement( LLoadKeyedFastDoubleElement* instr) { XMMRegister result(ToDoubleRegister(instr->result())); - if (instr->hydrogen()->RequiresHoleCheck()) { - int offset = FixedDoubleArray::kHeaderSize - kHeapObjectTag + - sizeof(kHoleNanLower32); - Operand hole_check_operand = BuildFastArrayOperand( - instr->elements(), - instr->key(), - FAST_DOUBLE_ELEMENTS, - offset); - __ cmpl(hole_check_operand, Immediate(kHoleNanUpper32)); - DeoptimizeIf(equal, instr->environment()); - } + int offset = FixedDoubleArray::kHeaderSize - kHeapObjectTag + + sizeof(kHoleNanLower32); + Operand hole_check_operand = BuildFastArrayOperand( + instr->elements(), + instr->key(), + FAST_DOUBLE_ELEMENTS, + offset); + __ cmpl(hole_check_operand, Immediate(kHoleNanUpper32)); + DeoptimizeIf(equal, instr->environment()); Operand double_load_operand = BuildFastArrayOperand( instr->elements(), instr->key(), FAST_DOUBLE_ELEMENTS, diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc index 725a58d8d..a67a59320 100644 --- a/src/x64/lithium-x64.cc +++ b/src/x64/lithium-x64.cc @@ -1722,7 +1722,7 @@ LInstruction* LChunkBuilder::DoConstant(HConstant* instr) { LInstruction* LChunkBuilder::DoLoadGlobalCell(HLoadGlobalCell* instr) { LLoadGlobalCell* result = new LLoadGlobalCell; - return instr->check_hole_value() + return instr->RequiresHoleCheck() ? AssignEnvironment(DefineAsRegister(result)) : DefineAsRegister(result); } @@ -1740,7 +1740,7 @@ LInstruction* LChunkBuilder::DoStoreGlobalCell(HStoreGlobalCell* instr) { new LStoreGlobalCell(UseTempRegister(instr->value()), TempRegister(), TempRegister()); - return instr->check_hole_value() ? AssignEnvironment(result) : result; + return instr->RequiresHoleCheck() ? AssignEnvironment(result) : result; }