Binary operation deoptimization fix.

R=jkummerow@chromium.org
BUG=

Review URL: https://codereview.chromium.org/132453009

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19132 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 591f00f..8037c8d 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -5980,7 +5980,8 @@ void HOptimizedGraphBuilder::HandleCompoundAssignment(Assignment* expr) {
     HValue* right = Pop();
     HValue* left = Pop();
 
-    Push(BuildBinaryOperation(operation, left, right));
+    Push(BuildBinaryOperation(operation, left, right, PUSH_BEFORE_SIMULATE));
+
     BuildStore(expr, prop, expr->id(),
                expr->AssignmentId(), expr->IsUninitialized());
   } else {
@@ -9056,7 +9057,8 @@ HValue* HGraphBuilder::TruncateToNumber(HValue* value, Type** expected) {
 HValue* HOptimizedGraphBuilder::BuildBinaryOperation(
     BinaryOperation* expr,
     HValue* left,
-    HValue* right) {
+    HValue* right,
+    PushBeforeSimulateBehavior push_sim_result) {
   Type* left_type = expr->left()->bounds().lower;
   Type* right_type = expr->right()->bounds().lower;
   Type* result_type = expr->bounds().lower;
@@ -9080,9 +9082,14 @@ HValue* HOptimizedGraphBuilder::BuildBinaryOperation(
   // after phis, which are the result of BuildBinaryOperation when we
   // inlined some complex subgraph.
   if (result->HasObservableSideEffects() || result->IsPhi()) {
-    Push(result);
-    Add<HSimulate>(expr->id(), REMOVABLE_SIMULATE);
-    Drop(1);
+    if (push_sim_result == NO_PUSH_BEFORE_SIMULATE) {
+      Add<HSimulate>(expr->id(), REMOVABLE_SIMULATE);
+    } else {
+      ASSERT(push_sim_result == PUSH_BEFORE_SIMULATE);
+      Push(result);
+      Add<HSimulate>(expr->id(), REMOVABLE_SIMULATE);
+      Drop(1);
+    }
   }
   return result;
 }
@@ -9462,7 +9469,10 @@ void HOptimizedGraphBuilder::VisitArithmeticExpression(BinaryOperation* expr) {
   SetSourcePosition(expr->position());
   HValue* right = Pop();
   HValue* left = Pop();
-  HValue* result = BuildBinaryOperation(expr, left, right);
+  HValue* result =
+      BuildBinaryOperation(expr, left, right,
+          ast_context()->IsEffect() ? NO_PUSH_BEFORE_SIMULATE
+                                    : PUSH_BEFORE_SIMULATE);
   if (FLAG_emit_opt_code_positions && result->IsBinaryOperation()) {
     HBinaryOperation::cast(result)->SetOperandPositions(
         zone(), expr->left()->position(), expr->right()->position());

diff --git a/src/hydrogen.h b/src/hydrogen.h
index 2ee87e7..1a87875 100644 (file)
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -2435,9 +2435,16 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
 
   HInstruction* BuildStringCharCodeAt(HValue* string,
                                       HValue* index);
-  HValue* BuildBinaryOperation(BinaryOperation* expr,
-                               HValue* left,
-                               HValue* right);
+
+  enum PushBeforeSimulateBehavior {
+    PUSH_BEFORE_SIMULATE,
+    NO_PUSH_BEFORE_SIMULATE
+  };
+  HValue* BuildBinaryOperation(
+      BinaryOperation* expr,
+      HValue* left,
+      HValue* right,
+      PushBeforeSimulateBehavior push_sim_result);
   HInstruction* BuildIncrement(bool returns_original_input,
                                CountOperation* expr);
   HInstruction* BuildLoadKeyedGeneric(HValue* object,

diff --git a/test/mjsunit/regress/binop-in-effect-context-deopt.js b/test/mjsunit/regress/binop-in-effect-context-deopt.js
new file mode 100644 (file)
index 0000000..fb7280a
--- /dev/null
+++ b/test/mjsunit/regress/binop-in-effect-context-deopt.js
@@ -0,0 +1,65 @@
+// Copyright 2014 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
+
+(function BinopInEffectContextDeoptAndOsr() {
+  function f(a, deopt, osr) {
+    var result = (a + 10, "result");
+    var dummy = deopt + 0;
+    if (osr) while (%GetOptimizationStatus(f) == 2) {}
+    return result;
+  }
+
+  assertEquals("result", f(true, 3, false));
+  assertEquals("result", f(true, 3, false));
+  %OptimizeFunctionOnNextCall(f);
+  assertEquals("result", f(true, "foo", true));
+})();
+
+
+(function BinopInEffectContextLazyDeopt() {
+  function deopt_f() {
+    %DeoptimizeFunction(f);
+    return "dummy";
+  }
+
+  function h() {
+    return { toString : deopt_f };
+  }
+
+  function g(x) {
+  }
+
+  function f() {
+    return g(void(h() + ""));
+  };
+
+  f();
+  %OptimizeFunctionOnNextCall(f);
+  f();
+})();