From b0afb777310b82e45913d910b20398807a2a5c57 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Mon, 10 Jun 2013 11:33:23 +0000 Subject: [PATCH] Fix parallel recompilation wrt transition maps dependency. R=ulan@chromium.org BUG= Review URL: https://chromiumcodereview.appspot.com/15896038 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15032 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-codegen-arm.cc | 8 ---- src/arm/lithium-codegen-arm.h | 2 - src/hydrogen-instructions.h | 9 +++- src/hydrogen.cc | 2 +- src/ia32/lithium-codegen-ia32.cc | 8 ---- src/ia32/lithium-codegen-ia32.h | 2 - src/mips/lithium-codegen-mips.cc | 8 ---- src/mips/lithium-codegen-mips.h | 2 - src/x64/lithium-codegen-x64.cc | 8 ---- src/x64/lithium-codegen-x64.h | 2 - test/mjsunit/parallel-invalidate-transition-map.js | 56 ++++++++++++++++++++++ 11 files changed, 65 insertions(+), 42 deletions(-) create mode 100644 test/mjsunit/parallel-invalidate-transition-map.js diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index d42ae72..beb63d1 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -88,11 +88,6 @@ void LCodeGen::FinishCode(Handle code) { } PopulateDeoptimizationData(code); info()->CommitDependentMaps(code); - - for (int i = 0 ; i < transition_maps_.length(); i++) { - transition_maps_.at(i)->AddDependentCode( - DependentCode::kTransitionGroup, code); - } } @@ -4276,9 +4271,6 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) { } if (!transition.is_null()) { - if (transition->CanBeDeprecated()) { - transition_maps_.Add(transition, info()->zone()); - } __ mov(scratch, Operand(transition)); __ str(scratch, FieldMemOperand(object, HeapObject::kMapOffset)); if (instr->hydrogen()->NeedsWriteBarrierForMap()) { diff --git a/src/arm/lithium-codegen-arm.h b/src/arm/lithium-codegen-arm.h index 45e4af6..a22d192 100644 --- a/src/arm/lithium-codegen-arm.h +++ b/src/arm/lithium-codegen-arm.h @@ -56,7 +56,6 @@ class LCodeGen BASE_EMBEDDED { deoptimizations_(4, info->zone()), deopt_jump_table_(4, info->zone()), deoptimization_literals_(8, info->zone()), - transition_maps_(0, info->zone()), inlined_function_count_(0), scope_(info->scope()), status_(UNUSED), @@ -409,7 +408,6 @@ class LCodeGen BASE_EMBEDDED { ZoneList deoptimizations_; ZoneList deopt_jump_table_; ZoneList > deoptimization_literals_; - ZoneList > transition_maps_; int inlined_function_count_; Scope* const scope_; Status status_; diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 6b9ff0c..813e43d 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -5689,6 +5689,7 @@ class HStoreNamedField: public HTemplateInstruction<2> { = Representation::Tagged()) : access_(access), field_representation_(field_representation), + transition_(), transition_unique_id_(), new_space_dominator_(NULL) { SetOperandAt(0, obj); @@ -5720,7 +5721,13 @@ class HStoreNamedField: public HTemplateInstruction<2> { HObjectAccess access() const { return access_; } Handle transition() const { return transition_; } UniqueValueId transition_unique_id() const { return transition_unique_id_; } - void set_transition(Handle map) { transition_ = map; } + void SetTransition(Handle map, CompilationInfo* info) { + ASSERT(transition_.is_null()); // Only set once. + if (map->CanBeDeprecated()) { + map->AddDependentCompilationInfo(DependentCode::kTransitionGroup, info); + } + transition_ = map; + } HValue* new_space_dominator() const { return new_space_dominator_; } bool NeedsWriteBarrier() { diff --git a/src/hydrogen.cc b/src/hydrogen.cc index ba2ef50..60edbb7 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -6253,7 +6253,7 @@ HInstruction* HOptimizedGraphBuilder::BuildStoreNamedField( if (transition_to_field) { Handle transition(lookup->GetTransitionMapFromMap(*map)); - instr->set_transition(transition); + instr->SetTransition(transition, top_info()); // TODO(fschneider): Record the new map type of the object in the IR to // enable elimination of redundant checks after the transition store. instr->SetGVNFlag(kChangesMaps); diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 14657b9..ded9d07 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -110,11 +110,6 @@ void LCodeGen::FinishCode(Handle code) { Deoptimizer::EnsureRelocSpaceForLazyDeoptimization(code); } info()->CommitDependentMaps(code); - - for (int i = 0 ; i < transition_maps_.length(); i++) { - transition_maps_.at(i)->AddDependentCode( - DependentCode::kTransitionGroup, code); - } } @@ -4302,9 +4297,6 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) { } if (!transition.is_null()) { - if (transition->CanBeDeprecated()) { - transition_maps_.Add(transition, info()->zone()); - } if (!instr->hydrogen()->NeedsWriteBarrierForMap()) { __ mov(FieldOperand(object, HeapObject::kMapOffset), transition); } else { diff --git a/src/ia32/lithium-codegen-ia32.h b/src/ia32/lithium-codegen-ia32.h index 70ecf5c..33f0eda 100644 --- a/src/ia32/lithium-codegen-ia32.h +++ b/src/ia32/lithium-codegen-ia32.h @@ -58,7 +58,6 @@ class LCodeGen BASE_EMBEDDED { deoptimizations_(4, info->zone()), jump_table_(4, info->zone()), deoptimization_literals_(8, info->zone()), - transition_maps_(0, info->zone()), inlined_function_count_(0), scope_(info->scope()), status_(UNUSED), @@ -408,7 +407,6 @@ class LCodeGen BASE_EMBEDDED { ZoneList deoptimizations_; ZoneList jump_table_; ZoneList > deoptimization_literals_; - ZoneList > transition_maps_; int inlined_function_count_; Scope* const scope_; Status status_; diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index cafccec..82a340c 100644 --- a/src/mips/lithium-codegen-mips.cc +++ b/src/mips/lithium-codegen-mips.cc @@ -88,11 +88,6 @@ void LCodeGen::FinishCode(Handle code) { } PopulateDeoptimizationData(code); info()->CommitDependentMaps(code); - - for (int i = 0 ; i < transition_maps_.length(); i++) { - transition_maps_.at(i)->AddDependentCode( - DependentCode::kTransitionGroup, code); - } } @@ -3966,9 +3961,6 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) { } if (!transition.is_null()) { - if (transition->CanBeDeprecated()) { - transition_maps_.Add(transition, info()->zone()); - } __ li(scratch, Operand(transition)); __ sw(scratch, FieldMemOperand(object, HeapObject::kMapOffset)); if (instr->hydrogen()->NeedsWriteBarrierForMap()) { diff --git a/src/mips/lithium-codegen-mips.h b/src/mips/lithium-codegen-mips.h index d96755c..ee01383 100644 --- a/src/mips/lithium-codegen-mips.h +++ b/src/mips/lithium-codegen-mips.h @@ -55,7 +55,6 @@ class LCodeGen BASE_EMBEDDED { deoptimizations_(4, info->zone()), deopt_jump_table_(4, info->zone()), deoptimization_literals_(8, info->zone()), - transition_maps_(0, info->zone()), inlined_function_count_(0), scope_(info->scope()), status_(UNUSED), @@ -411,7 +410,6 @@ class LCodeGen BASE_EMBEDDED { ZoneList deoptimizations_; ZoneList deopt_jump_table_; ZoneList > deoptimization_literals_; - ZoneList > transition_maps_; int inlined_function_count_; Scope* const scope_; Status status_; diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index b07655d..49c1ab9 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -93,11 +93,6 @@ void LCodeGen::FinishCode(Handle code) { } PopulateDeoptimizationData(code); info()->CommitDependentMaps(code); - - for (int i = 0 ; i < transition_maps_.length(); i++) { - transition_maps_.at(i)->AddDependentCode( - DependentCode::kTransitionGroup, code); - } } @@ -3995,9 +3990,6 @@ void LCodeGen::DoStoreNamedField(LStoreNamedField* instr) { } if (!transition.is_null()) { - if (transition->CanBeDeprecated()) { - transition_maps_.Add(transition, info()->zone()); - } if (!instr->hydrogen()->NeedsWriteBarrierForMap()) { __ Move(FieldOperand(object, HeapObject::kMapOffset), transition); } else { diff --git a/src/x64/lithium-codegen-x64.h b/src/x64/lithium-codegen-x64.h index 462a7c5..c3f99c4 100644 --- a/src/x64/lithium-codegen-x64.h +++ b/src/x64/lithium-codegen-x64.h @@ -57,7 +57,6 @@ class LCodeGen BASE_EMBEDDED { deoptimizations_(4, info->zone()), jump_table_(4, info->zone()), deoptimization_literals_(8, info->zone()), - transition_maps_(0, info->zone()), inlined_function_count_(0), scope_(info->scope()), status_(UNUSED), @@ -361,7 +360,6 @@ class LCodeGen BASE_EMBEDDED { ZoneList deoptimizations_; ZoneList jump_table_; ZoneList > deoptimization_literals_; - ZoneList > transition_maps_; int inlined_function_count_; Scope* const scope_; Status status_; diff --git a/test/mjsunit/parallel-invalidate-transition-map.js b/test/mjsunit/parallel-invalidate-transition-map.js new file mode 100644 index 0000000..42a266f --- /dev/null +++ b/test/mjsunit/parallel-invalidate-transition-map.js @@ -0,0 +1,56 @@ +// 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: --track-fields --track-double-fields --allow-natives-syntax +// Flags: --parallel-recompilation --parallel-recompilation-delay=50 + +function assertUnoptimized(fun) { + assertTrue(%GetOptimizationStatus(fun) != 1); +} + +function new_object() { + var o = {}; + o.a = 1; + o.b = 2; + return o; +} + +function add_field(obj) { + obj.c = 3; +} + +add_field(new_object()); +add_field(new_object()); +%OptimizeFunctionOnNextCall(add_field, "parallel"); + +var o = new_object(); +add_field(o); // Trigger optimization. +assertUnoptimized(add_field); // Not yet optimized. +o.c = 2.2; // Invalidate transition map. +%CompleteOptimization(add_field); // Conclude optimization with... +assertUnoptimized(add_field); // ... bailing out due to map dependency. + -- 2.7.4