Fix wrong bailout id in polymorphic stores.
authorverwaest@chromium.org <verwaest@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 19 Jul 2013 08:45:47 +0000 (08:45 +0000)
committerverwaest@chromium.org <verwaest@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 19 Jul 2013 08:45:47 +0000 (08:45 +0000)
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

src/hydrogen.cc
src/hydrogen.h
test/mjsunit/regress/poly_count_operation.js [new file with mode: 0644]

index 183bdde..53847b6 100644 (file)
@@ -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);
index 7d9a7af..1912806 100644 (file)
@@ -1741,8 +1741,7 @@ class HOptimizedGraphBuilder: public HGraphBuilder, public AstVisitor {
                                                 HValue* object,
                                                 SmallMapList* types,
                                                 Handle<String> 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<String> name,
diff --git a/test/mjsunit/regress/poly_count_operation.js b/test/mjsunit/regress/poly_count_operation.js
new file mode 100644 (file)
index 0000000..a8a1ed2
--- /dev/null
@@ -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);