From: verwaest@chromium.org Date: Fri, 19 Jul 2013 08:45:47 +0000 (+0000) Subject: Fix wrong bailout id in polymorphic stores. X-Git-Tag: upstream/4.7.83~13298 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=be472d82fd706a475da5388e48f4f1c2cb15a33b;p=platform%2Fupstream%2Fv8.git Fix wrong bailout id in polymorphic stores. BUG=chromium:259787 R=titzer@chromium.org, ulan@chromium.org Review URL: https://chromiumcodereview.appspot.com/19528005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15763 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 183bdde..53847b6 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -4829,19 +4829,16 @@ bool HOptimizedGraphBuilder::TryStorePolymorphicAsMonomorphic( store = BuildStoreNamedField( object, name, store_value, types->at(count - 1), &lookup), true); - if (result_value != NULL) Push(result_value); - Push(store_value); + Push(result_value); store->set_position(position); AddInstruction(store); AddSimulate(assignment_id); - if (result_value != NULL) Drop(1); ast_context()->ReturnValue(Pop()); return true; } void HOptimizedGraphBuilder::HandlePolymorphicStoreNamedField( - BailoutId id, int position, BailoutId assignment_id, HValue* object, @@ -4882,10 +4879,7 @@ void HOptimizedGraphBuilder::HandlePolymorphicStoreNamedField( instr->set_position(position); // Goto will add the HSimulate for the store. AddInstruction(instr); - if (!ast_context()->IsEffect()) { - if (result_value != NULL) Push(result_value); - Push(store_value); - } + if (!ast_context()->IsEffect()) Push(result_value); current_block()->Goto(join); set_current_block(if_false); @@ -4904,8 +4898,7 @@ void HOptimizedGraphBuilder::HandlePolymorphicStoreNamedField( if (join != NULL) { if (!ast_context()->IsEffect()) { - if (result_value != NULL) Push(result_value); - Push(store_value); + Push(result_value); } current_block()->Goto(join); } else { @@ -4914,24 +4907,21 @@ void HOptimizedGraphBuilder::HandlePolymorphicStoreNamedField( // unoptimized code). if (instr->HasObservableSideEffects()) { if (ast_context()->IsEffect()) { - AddSimulate(id, REMOVABLE_SIMULATE); + AddSimulate(assignment_id, REMOVABLE_SIMULATE); } else { - if (result_value != NULL) Push(result_value); - Push(store_value); - AddSimulate(id, REMOVABLE_SIMULATE); - Drop(result_value != NULL ? 2 : 1); + Push(result_value); + AddSimulate(assignment_id, REMOVABLE_SIMULATE); + Drop(1); } } - return ast_context()->ReturnValue( - result_value != NULL ? result_value : store_value); + return ast_context()->ReturnValue(result_value); } } ASSERT(join != NULL); - join->SetJoinId(id); + join->SetJoinId(assignment_id); set_current_block(join); if (!ast_context()->IsEffect()) { - if (result_value != NULL) Drop(1); ast_context()->ReturnValue(Pop()); } } @@ -4950,7 +4940,7 @@ void HOptimizedGraphBuilder::HandlePropertyAssignment(Assignment* expr) { if (expr->IsUninitialized()) AddSoftDeoptimize(); return BuildStoreNamed(expr, expr->id(), expr->position(), - expr->AssignmentId(), prop, object, value); + expr->AssignmentId(), prop, object, value, value); } else { // Keyed store. CHECK_ALIVE(VisitForValue(prop->key())); @@ -5043,7 +5033,7 @@ void HOptimizedGraphBuilder::BuildStoreNamed(Expression* expr, AddCheckConstantFunction(holder, object, map); // Don't try to inline if the result_value is different from the // store_value. That case isn't handled yet by the inlining. - if (result_value == NULL && + if (result_value == store_value && FLAG_inline_accessors && TryInlineSetter(setter, id, assignment_id, store_value)) { return; @@ -5062,21 +5052,19 @@ void HOptimizedGraphBuilder::BuildStoreNamed(Expression* expr, } else if (types != NULL && types->length() > 1) { Drop(2); return HandlePolymorphicStoreNamedField( - id, position, assignment_id, object, + position, id, object, store_value, result_value, types, name); } else { Drop(2); instr = BuildStoreNamedGeneric(object, name, store_value); } - if (result_value != NULL) Push(result_value); - Push(store_value); + Push(result_value); instr->set_position(position); AddInstruction(instr); if (instr->HasObservableSideEffects()) { - AddSimulate(assignment_id, REMOVABLE_SIMULATE); + AddSimulate(id, REMOVABLE_SIMULATE); } - if (result_value != NULL) Drop(1); return ast_context()->ReturnValue(Pop()); } @@ -5207,7 +5195,7 @@ void HOptimizedGraphBuilder::HandleCompoundAssignment(Assignment* expr) { } return BuildStoreNamed(prop, expr->id(), expr->position(), - expr->AssignmentId(), prop, object, instr); + expr->AssignmentId(), prop, object, instr, instr); } else { // Keyed property. CHECK_ALIVE(VisitForValue(prop->obj())); @@ -7692,8 +7680,7 @@ void HOptimizedGraphBuilder::VisitCountOperation(CountOperation* expr) { } after = BuildIncrement(returns_original_input, expr); - - HValue* result = returns_original_input ? Pop() : NULL; + HValue* result = returns_original_input ? Pop() : after; return BuildStoreNamed(prop, expr->id(), expr->position(), expr->AssignmentId(), prop, object, after, result); diff --git a/src/hydrogen.h b/src/hydrogen.h index 7d9a7af..1912806 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -1741,8 +1741,7 @@ class HOptimizedGraphBuilder: public HGraphBuilder, public AstVisitor { HValue* object, SmallMapList* types, Handle name); - void HandlePolymorphicStoreNamedField(BailoutId id, - int position, + void HandlePolymorphicStoreNamedField(int position, BailoutId assignment_id, HValue* object, HValue* value, @@ -1838,7 +1837,7 @@ class HOptimizedGraphBuilder: public HGraphBuilder, public AstVisitor { Property* prop, HValue* object, HValue* store_value, - HValue* result_value = NULL); + HValue* result_value); HInstruction* BuildStoreNamedField(HValue* object, Handle name, diff --git a/test/mjsunit/regress/poly_count_operation.js b/test/mjsunit/regress/poly_count_operation.js new file mode 100644 index 0000000..a8a1ed2 --- /dev/null +++ b/test/mjsunit/regress/poly_count_operation.js @@ -0,0 +1,155 @@ +// 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 o1 = {x:1}; +var o2 = {}; +var deopt_getter = false; +var deopt_setter = false; + +function f_mono(o) { + return 5 + o.x++; +} + +var to_deopt = f_mono; + +var v = 1; +var g = 0; +var s = 0; + +Object.defineProperty(o2, "x", + {get:function() { + g++; + if (deopt_getter) { + deopt_getter = false; + %DeoptimizeFunction(to_deopt); + } + return v; + }, + set:function(new_v) { + v = new_v; + s++; + if (deopt_setter) { + deopt_setter = false; + %DeoptimizeFunction(to_deopt); + } + }}); + +assertEquals(6, f_mono(o2)); +assertEquals(1, g); +assertEquals(1, s); +assertEquals(7, f_mono(o2)); +assertEquals(2, g); +assertEquals(2, s); +%OptimizeFunctionOnNextCall(f_mono); +deopt_setter = true; +assertEquals(8, f_mono(o2)); +assertEquals(3, g); +assertEquals(3, s); + +function f_poly(o) { + return 5 + o.x++; +} + +v = 1; +to_deopt = f_poly; + +f_poly(o1); +f_poly(o1); +assertEquals(6, f_poly(o2)); +assertEquals(4, g); +assertEquals(4, s); +assertEquals(7, f_poly(o2)); +assertEquals(5, g); +assertEquals(5, s); +%OptimizeFunctionOnNextCall(f_poly); +deopt_setter = true; +assertEquals(8, f_poly(o2)); +assertEquals(6, g); +assertEquals(6, s); + +%OptimizeFunctionOnNextCall(f_poly); +v = undefined; +assertEquals(NaN, f_poly(o2)); +assertEquals(7, g); +assertEquals(7, s); + +function f_pre(o) { + return 5 + ++o.x; +} + +v = 1; +to_deopt = f_pre; + +f_pre(o1); +f_pre(o1); +assertEquals(7, f_pre(o2)); +assertEquals(8, g); +assertEquals(8, s); +assertEquals(8, f_pre(o2)); +assertEquals(9, g); +assertEquals(9, s); +%OptimizeFunctionOnNextCall(f_pre); +deopt_setter = true; +assertEquals(9, f_pre(o2)); +assertEquals(10, g); +assertEquals(10, s); + +%OptimizeFunctionOnNextCall(f_pre); +v = undefined; +assertEquals(NaN, f_pre(o2)); +assertEquals(11, g); +assertEquals(11, s); + + +function f_get(o) { + return 5 + o.x++; +} + +v = 1; +to_deopt = f_get; + +f_get(o1); +f_get(o1); +assertEquals(6, f_get(o2)); +assertEquals(12, g); +assertEquals(12, s); +assertEquals(7, f_get(o2)); +assertEquals(13, g); +assertEquals(13, s); +%OptimizeFunctionOnNextCall(f_get); +deopt_getter = true; +assertEquals(8, f_get(o2)); +assertEquals(14, g); +assertEquals(14, s); + +%OptimizeFunctionOnNextCall(f_get); +v = undefined; +assertEquals(NaN, f_get(o2)); +assertEquals(15, g); +assertEquals(15, s);