From babce318d10e7222616df36b8cb3a49a7ee6ab53 Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Tue, 23 Jul 2013 09:18:42 +0000 Subject: [PATCH] Eliminate map checks of constant values. R=ulan@chromium.org Review URL: https://chromiumcodereview.appspot.com/19954005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15819 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-arm.cc | 13 ++++++-- src/arm/lithium-codegen-arm.cc | 13 ++++---- src/code-stubs-hydrogen.cc | 3 +- src/flag-definitions.h | 3 ++ src/hydrogen-instructions.cc | 18 ++++++++++- src/hydrogen-instructions.h | 26 +++++++++++----- src/hydrogen.cc | 16 +++++----- src/ia32/lithium-codegen-ia32.cc | 10 +++---- src/ia32/lithium-ia32.cc | 8 +++-- src/objects-inl.h | 6 ++++ src/objects.h | 1 + src/x64/lithium-codegen-x64.cc | 10 +++---- src/x64/lithium-x64.cc | 8 +++-- test/mjsunit/omit-constant-mapcheck.js | 55 ++++++++++++++++++++++++++++++++++ 14 files changed, 151 insertions(+), 39 deletions(-) create mode 100644 test/mjsunit/omit-constant-mapcheck.js diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index b68d22f..b98b462 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -2035,9 +2035,14 @@ LInstruction* LChunkBuilder::DoCheckInstanceType(HCheckInstanceType* instr) { LInstruction* LChunkBuilder::DoCheckPrototypeMaps(HCheckPrototypeMaps* instr) { - LUnallocated* temp1 = TempRegister(); - LOperand* temp2 = TempRegister(); + LUnallocated* temp1 = NULL; + LOperand* temp2 = NULL; + if (!instr->CanOmitPrototypeChecks()) { + temp1 = TempRegister(); + temp2 = TempRegister(); + } LCheckPrototypeMaps* result = new(zone()) LCheckPrototypeMaps(temp1, temp2); + if (instr->CanOmitPrototypeChecks()) return result; return AssignEnvironment(result); } @@ -2049,8 +2054,10 @@ LInstruction* LChunkBuilder::DoCheckFunction(HCheckFunction* instr) { LInstruction* LChunkBuilder::DoCheckMaps(HCheckMaps* instr) { - LOperand* value = UseRegisterAtStart(instr->value()); + LOperand* value = NULL; + if (!instr->CanOmitMapChecks()) value = UseRegisterAtStart(instr->value()); LInstruction* result = new(zone()) LCheckMaps(value); + if (instr->CanOmitMapChecks()) return result; return AssignEnvironment(result); } diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 1b5d90b..7278c95 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -5232,6 +5232,7 @@ void LCodeGen::DoCheckMapCommon(Register map_reg, void LCodeGen::DoCheckMaps(LCheckMaps* instr) { + if (instr->hydrogen()->CanOmitMapChecks()) return; Register map_reg = scratch0(); LOperand* input = instr->value(); ASSERT(input->IsRegister()); @@ -5304,6 +5305,8 @@ void LCodeGen::DoClampTToUint8(LClampTToUint8* instr) { void LCodeGen::DoCheckPrototypeMaps(LCheckPrototypeMaps* instr) { + if (instr->hydrogen()->CanOmitPrototypeChecks()) return; + Register prototype_reg = ToRegister(instr->temp()); Register map_reg = ToRegister(instr->temp2()); @@ -5312,12 +5315,10 @@ void LCodeGen::DoCheckPrototypeMaps(LCheckPrototypeMaps* instr) { ASSERT(prototypes->length() == maps->length()); - if (!instr->hydrogen()->CanOmitPrototypeChecks()) { - for (int i = 0; i < prototypes->length(); i++) { - __ LoadHeapObject(prototype_reg, prototypes->at(i)); - __ ldr(map_reg, FieldMemOperand(prototype_reg, HeapObject::kMapOffset)); - DoCheckMapCommon(map_reg, maps->at(i), instr->environment()); - } + for (int i = 0; i < prototypes->length(); i++) { + __ LoadHeapObject(prototype_reg, prototypes->at(i)); + __ ldr(map_reg, FieldMemOperand(prototype_reg, HeapObject::kMapOffset)); + DoCheckMapCommon(map_reg, maps->at(i), instr->environment()); } } diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index 388fd62..a1db370 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -884,7 +884,8 @@ HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { // Check that the map of the global has not changed: use a placeholder map // that will be replaced later with the global object's map. Handle placeholder_map = isolate()->factory()->meta_map(); - AddInstruction(HCheckMaps::New(receiver, placeholder_map, zone())); + AddInstruction(HCheckMaps::New( + receiver, placeholder_map, zone(), top_info())); HValue* cell = Add(placeholder_cell, Representation::Tagged()); HObjectAccess access(HObjectAccess::ForCellPayload(isolate())); diff --git a/src/flag-definitions.h b/src/flag-definitions.h index 63cf663..1b0e33c 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -307,6 +307,9 @@ DEFINE_int(parallel_recompilation_delay, 0, DEFINE_bool(omit_prototype_checks_for_leaf_maps, true, "do not emit prototype checks if all prototypes have leaf maps, " "deoptimize the optimized code if the layout of the maps changes.") +DEFINE_bool(omit_map_checks_for_leaf_maps, true, + "do not emit check maps for constant values that have a leaf map, " + "deoptimize the optimized code if the layout of the maps changes.") // Experimental profiler changes. DEFINE_bool(experimental_profiler, true, "enable all profiler experiments") diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index dfa5553..44afb8b 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -1680,7 +1680,7 @@ void HCheckMaps::PrintDataTo(StringStream* stream) { for (int i = 1; i < map_set()->length(); ++i) { stream->Add(",%p", *map_set()->at(i)); } - stream->Add("]"); + stream->Add("]%s", CanOmitMapChecks() ? "(omitted)" : ""); } @@ -2775,6 +2775,22 @@ HLoadNamedFieldPolymorphic::HLoadNamedFieldPolymorphic(HValue* context, } +HCheckMaps* HCheckMaps::New(HValue* value, + Handle map, + Zone* zone, + CompilationInfo* info, + HValue* typecheck) { + HCheckMaps* check_map = new(zone) HCheckMaps(value, zone, typecheck); + check_map->map_set_.Add(map, zone); + if (map->CanOmitMapChecks() && + value->IsConstant() && + HConstant::cast(value)->InstanceOf(map)) { + check_map->omit(info); + } + return check_map; +} + + void HCheckMaps::FinalizeUniqueValueId() { if (!map_unique_ids_.is_empty()) return; Zone* zone = block()->zone(); diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 83fe181..12ad198 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -2738,12 +2738,7 @@ class HLoadExternalArrayPointer: public HUnaryOperation { class HCheckMaps: public HTemplateInstruction<2> { public: static HCheckMaps* New(HValue* value, Handle map, Zone* zone, - HValue *typecheck = NULL) { - HCheckMaps* check_map = new(zone) HCheckMaps(value, zone, typecheck); - check_map->map_set_.Add(map, zone); - return check_map; - } - + CompilationInfo* info, HValue *typecheck = NULL); static HCheckMaps* New(HValue* value, SmallMapList* maps, Zone* zone, HValue *typecheck = NULL) { HCheckMaps* check_map = new(zone) HCheckMaps(value, zone, typecheck); @@ -2777,6 +2772,8 @@ class HCheckMaps: public HTemplateInstruction<2> { return check_map; } + bool CanOmitMapChecks() { return omit_; } + virtual bool HasEscapingOperandAt(int index) { return false; } virtual Representation RequiredInputRepresentation(int index) { return Representation::Tagged(); @@ -2812,7 +2809,7 @@ class HCheckMaps: public HTemplateInstruction<2> { private: // Clients should use one of the static New* methods above. HCheckMaps(HValue* value, Zone *zone, HValue* typecheck) - : map_unique_ids_(0, zone) { + : omit_(false), map_unique_ids_(0, zone) { SetOperandAt(0, value); // Use the object value for the dependency if NULL is passed. // TODO(titzer): do GVN flags already express this dependency? @@ -2824,6 +2821,16 @@ class HCheckMaps: public HTemplateInstruction<2> { SetGVNFlag(kDependsOnElementsKind); } + void omit(CompilationInfo* info) { + omit_ = true; + for (int i = 0; i < map_set_.length(); i++) { + Handle map = map_set_.at(i); + map->AddDependentCompilationInfo(DependentCode::kPrototypeCheckGroup, + info); + } + } + + bool omit_; SmallMapList map_set_; ZoneList map_unique_ids_; }; @@ -3302,6 +3309,11 @@ class HConstant: public HTemplateInstruction<0> { return handle_; } + bool InstanceOf(Handle map) { + return handle_->IsJSObject() && + Handle::cast(handle_)->map() == *map; + } + bool IsSpecialDouble() const { return has_double_value_ && (BitCast(double_value_) == BitCast(-0.0) || diff --git a/src/hydrogen.cc b/src/hydrogen.cc index fc3d145..b293ec4 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -1045,7 +1045,7 @@ HValue* HGraphBuilder::BuildCheckHeapObject(HValue* obj) { HValue* HGraphBuilder::BuildCheckMap(HValue* obj, Handle map) { - HCheckMaps* check = HCheckMaps::New(obj, map, zone()); + HCheckMaps* check = HCheckMaps::New(obj, map, zone(), top_info()); AddInstruction(check); return check; } @@ -1208,7 +1208,7 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess( if (is_store && (fast_elements || fast_smi_only_elements) && store_mode != STORE_NO_TRANSITION_HANDLE_COW) { HCheckMaps* check_cow_map = HCheckMaps::New( - elements, isolate()->factory()->fixed_array_map(), zone); + elements, isolate()->factory()->fixed_array_map(), zone, top_info()); check_cow_map->ClearGVNFlag(kDependsOnElementsKind); AddInstruction(check_cow_map); } @@ -1276,7 +1276,8 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess( length); } else { HCheckMaps* check_cow_map = HCheckMaps::New( - elements, isolate()->factory()->fixed_array_map(), zone); + elements, isolate()->factory()->fixed_array_map(), + zone, top_info()); check_cow_map->ClearGVNFlag(kDependsOnElementsKind); AddInstruction(check_cow_map); } @@ -4450,7 +4451,7 @@ void HOptimizedGraphBuilder::VisitArrayLiteral(ArrayLiteral* expr) { // De-opt if elements kind changed from boilerplate_elements_kind. Handle map = Handle(original_boilerplate_object->map(), isolate()); - AddInstruction(HCheckMaps::New(literal, map, zone())); + AddInstruction(HCheckMaps::New(literal, map, zone(), top_info())); } // The array is expected in the bailout environment during computation @@ -4541,7 +4542,7 @@ static Representation ComputeLoadStoreRepresentation(Handle type, void HOptimizedGraphBuilder::AddCheckMap(HValue* object, Handle map) { BuildCheckHeapObject(object); - AddInstruction(HCheckMaps::New(object, map, zone())); + AddInstruction(HCheckMaps::New(object, map, zone(), top_info())); } @@ -5506,7 +5507,8 @@ HInstruction* HOptimizedGraphBuilder::BuildMonomorphicElementAccess( Handle map, bool is_store, KeyedAccessStoreMode store_mode) { - HCheckMaps* mapcheck = HCheckMaps::New(object, map, zone(), dependency); + HCheckMaps* mapcheck = HCheckMaps::New( + object, map, zone(), top_info(), dependency); AddInstruction(mapcheck); if (dependency) { mapcheck->ClearGVNFlag(kDependsOnElementsKind); @@ -5690,7 +5692,7 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess( if (is_store && !IsFastDoubleElementsKind(elements_kind)) { AddInstruction(HCheckMaps::New( elements, isolate()->factory()->fixed_array_map(), - zone(), mapcompare)); + zone(), top_info(), mapcompare)); } if (map->IsJSArray()) { HInstruction* length = AddLoad(object, HObjectAccess::ForArrayLength(), diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 38d2011..d1bb4aa 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -5802,6 +5802,7 @@ void LCodeGen::DoCheckMapCommon(Register reg, void LCodeGen::DoCheckMaps(LCheckMaps* instr) { + if (instr->hydrogen()->CanOmitMapChecks()) return; LOperand* input = instr->value(); ASSERT(input->IsRegister()); Register reg = ToRegister(input); @@ -5992,6 +5993,7 @@ void LCodeGen::DoClampTToUint8NoSSE2(LClampTToUint8NoSSE2* instr) { void LCodeGen::DoCheckPrototypeMaps(LCheckPrototypeMaps* instr) { + if (instr->hydrogen()->CanOmitPrototypeChecks()) return; Register reg = ToRegister(instr->temp()); ZoneList >* prototypes = instr->prototypes(); @@ -5999,11 +6001,9 @@ void LCodeGen::DoCheckPrototypeMaps(LCheckPrototypeMaps* instr) { ASSERT(prototypes->length() == maps->length()); - if (!instr->hydrogen()->CanOmitPrototypeChecks()) { - for (int i = 0; i < prototypes->length(); i++) { - __ LoadHeapObject(reg, prototypes->at(i)); - DoCheckMapCommon(reg, maps->at(i), instr); - } + for (int i = 0; i < prototypes->length(); i++) { + __ LoadHeapObject(reg, prototypes->at(i)); + DoCheckMapCommon(reg, maps->at(i), instr); } } diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc index aebe26b..20d0a98 100644 --- a/src/ia32/lithium-ia32.cc +++ b/src/ia32/lithium-ia32.cc @@ -2063,8 +2063,10 @@ LInstruction* LChunkBuilder::DoCheckInstanceType(HCheckInstanceType* instr) { LInstruction* LChunkBuilder::DoCheckPrototypeMaps(HCheckPrototypeMaps* instr) { - LUnallocated* temp = TempRegister(); + LUnallocated* temp = NULL; + if (!instr->CanOmitPrototypeChecks()) temp = TempRegister(); LCheckPrototypeMaps* result = new(zone()) LCheckPrototypeMaps(temp); + if (instr->CanOmitPrototypeChecks()) return result; return AssignEnvironment(result); } @@ -2081,8 +2083,10 @@ LInstruction* LChunkBuilder::DoCheckFunction(HCheckFunction* instr) { LInstruction* LChunkBuilder::DoCheckMaps(HCheckMaps* instr) { - LOperand* value = UseRegisterAtStart(instr->value()); + LOperand* value = NULL; + if (!instr->CanOmitMapChecks()) value = UseRegisterAtStart(instr->value()); LCheckMaps* result = new(zone()) LCheckMaps(value); + if (instr->CanOmitMapChecks()) return result; return AssignEnvironment(result); } diff --git a/src/objects-inl.h b/src/objects-inl.h index c12a12a..88ac91c 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -3669,6 +3669,12 @@ bool Map::CanOmitPrototypeChecks() { } +bool Map::CanOmitMapChecks() { + return !HasTransitionArray() && !is_dictionary_map() && + FLAG_omit_map_checks_for_leaf_maps; +} + + int DependentCode::number_of_entries(DependencyGroup group) { if (length() == 0) return 0; return Smi::cast(get(group))->value(); diff --git a/src/objects.h b/src/objects.h index f197b23..f04c60b 100644 --- a/src/objects.h +++ b/src/objects.h @@ -5626,6 +5626,7 @@ class Map: public HeapObject { inline void NotifyLeafMapLayoutChange(); inline bool CanOmitPrototypeChecks(); + inline bool CanOmitMapChecks(); void AddDependentCompilationInfo(DependentCode::DependencyGroup group, CompilationInfo* info); diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 475c405..062a1d2 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -4954,6 +4954,7 @@ void LCodeGen::DoCheckMapCommon(Register reg, void LCodeGen::DoCheckMaps(LCheckMaps* instr) { + if (instr->hydrogen()->CanOmitMapChecks()) return; LOperand* input = instr->value(); ASSERT(input->IsRegister()); Register reg = ToRegister(input); @@ -5021,6 +5022,7 @@ void LCodeGen::DoClampTToUint8(LClampTToUint8* instr) { void LCodeGen::DoCheckPrototypeMaps(LCheckPrototypeMaps* instr) { + if (instr->hydrogen()->CanOmitPrototypeChecks()) return; Register reg = ToRegister(instr->temp()); ZoneList >* prototypes = instr->prototypes(); @@ -5028,11 +5030,9 @@ void LCodeGen::DoCheckPrototypeMaps(LCheckPrototypeMaps* instr) { ASSERT(prototypes->length() == maps->length()); - if (!instr->hydrogen()->CanOmitPrototypeChecks()) { - for (int i = 0; i < prototypes->length(); i++) { - __ LoadHeapObject(reg, prototypes->at(i)); - DoCheckMapCommon(reg, maps->at(i), instr); - } + for (int i = 0; i < prototypes->length(); i++) { + __ LoadHeapObject(reg, prototypes->at(i)); + DoCheckMapCommon(reg, maps->at(i), instr); } } diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc index 2cec68b..5bd5dc7 100644 --- a/src/x64/lithium-x64.cc +++ b/src/x64/lithium-x64.cc @@ -1949,8 +1949,10 @@ LInstruction* LChunkBuilder::DoCheckInstanceType(HCheckInstanceType* instr) { LInstruction* LChunkBuilder::DoCheckPrototypeMaps(HCheckPrototypeMaps* instr) { - LUnallocated* temp = TempRegister(); + LUnallocated* temp = NULL; + if (!instr->CanOmitPrototypeChecks()) temp = TempRegister(); LCheckPrototypeMaps* result = new(zone()) LCheckPrototypeMaps(temp); + if (instr->CanOmitPrototypeChecks()) return result; return AssignEnvironment(result); } @@ -1962,8 +1964,10 @@ LInstruction* LChunkBuilder::DoCheckFunction(HCheckFunction* instr) { LInstruction* LChunkBuilder::DoCheckMaps(HCheckMaps* instr) { - LOperand* value = UseRegisterAtStart(instr->value()); + LOperand* value = NULL; + if (!instr->CanOmitMapChecks()) value = UseRegisterAtStart(instr->value()); LCheckMaps* result = new(zone()) LCheckMaps(value); + if (instr->CanOmitMapChecks()) return result; return AssignEnvironment(result); } diff --git a/test/mjsunit/omit-constant-mapcheck.js b/test/mjsunit/omit-constant-mapcheck.js new file mode 100644 index 0000000..6894275 --- /dev/null +++ b/test/mjsunit/omit-constant-mapcheck.js @@ -0,0 +1,55 @@ +// Copyright 2013 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Flags: --allow-natives-syntax + +var g1 = { a:1 } + +function load() { + return g1.a; +} + +assertEquals(1, load()); +assertEquals(1, load()); +%OptimizeFunctionOnNextCall(load); +assertEquals(1, load()); +delete g1.a; +assertEquals(undefined, load()); + +var g2 = { a:2 } + +function load2() { + return g2.a; +} + +assertEquals(2, load2()); +assertEquals(2, load2()); +%OptimizeFunctionOnNextCall(load2); +assertEquals(2, load2()); +g2.b = 10; +g2.a = 5; +assertEquals(5, load2()); -- 2.7.4