Comparison in effect context lazy deopt fix.

R=jkummerow@chromium.org
BUG=

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

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

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 05d31ecc636398cf2f038551ed89c6446235d9d7..fb9806e44f0889ed7605d600eaf6618a4d9040f7 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -4448,7 +4448,7 @@ void HOptimizedGraphBuilder::VisitSwitchStatement(SwitchStatement* stmt) {
         combined_type,
         ScriptPositionToSourcePosition(stmt->tag()->position()),
         ScriptPositionToSourcePosition(clause->label()->position()),
-        clause->id());
+        PUSH_BEFORE_SIMULATE, clause->id());
 
     HBasicBlock* next_test_block = graph()->CreateBasicBlock();
     HBasicBlock* body_block = graph()->CreateBasicBlock();
@@ -9087,13 +9087,12 @@ HValue* HOptimizedGraphBuilder::BuildBinaryOperation(
   // after phis, which are the result of BuildBinaryOperation when we
   // inlined some complex subgraph.
   if (result->HasObservableSideEffects() || result->IsPhi()) {
-    if (push_sim_result == NO_PUSH_BEFORE_SIMULATE) {
-      Add<HSimulate>(expr->id(), REMOVABLE_SIMULATE);
-    } else {
-      ASSERT(push_sim_result == PUSH_BEFORE_SIMULATE);
+    if (push_sim_result == PUSH_BEFORE_SIMULATE) {
       Push(result);
       Add<HSimulate>(expr->id(), REMOVABLE_SIMULATE);
       Drop(1);
+    } else {
+      Add<HSimulate>(expr->id(), REMOVABLE_SIMULATE);
     }
   }
   return result;
@@ -9611,11 +9610,14 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
     return ast_context()->ReturnInstruction(result, expr->id());
   }
 
+  PushBeforeSimulateBehavior push_behavior =
+    ast_context()->IsEffect() ? NO_PUSH_BEFORE_SIMULATE
+                              : PUSH_BEFORE_SIMULATE;
   HControlInstruction* compare = BuildCompareInstruction(
       op, left, right, left_type, right_type, combined_type,
       ScriptPositionToSourcePosition(expr->left()->position()),
       ScriptPositionToSourcePosition(expr->right()->position()),
-      expr->id());
+      push_behavior, expr->id());
   if (compare == NULL) return;  // Bailed out.
   return ast_context()->ReturnControl(compare, expr->id());
 }
@@ -9630,6 +9632,7 @@ HControlInstruction* HOptimizedGraphBuilder::BuildCompareInstruction(
     Type* combined_type,
     HSourcePosition left_position,
     HSourcePosition right_position,
+    PushBeforeSimulateBehavior push_sim_result,
     BailoutId bailout_id) {
   // Cases handled below depend on collected type feedback. They should
   // soft deoptimize when there is no type feedback.
@@ -9694,9 +9697,13 @@ HControlInstruction* HOptimizedGraphBuilder::BuildCompareInstruction(
       result->set_observed_input_representation(1, left_rep);
       result->set_observed_input_representation(2, right_rep);
       if (result->HasObservableSideEffects()) {
-        Push(result);
-        AddSimulate(bailout_id, REMOVABLE_SIMULATE);
-        Drop(1);
+        if (push_sim_result == PUSH_BEFORE_SIMULATE) {
+          Push(result);
+          AddSimulate(bailout_id, REMOVABLE_SIMULATE);
+          Drop(1);
+        } else {
+          AddSimulate(bailout_id, REMOVABLE_SIMULATE);
+        }
       }
       // TODO(jkummerow): Can we make this more efficient?
       HBranch* branch = New<HBranch>(result);

diff --git a/src/hydrogen.h b/src/hydrogen.h
index 5f1ef79b05cfc325e14fd9012364af3e6348a3ed..8668698fa96c7f2ad41920af7819f634aa92f8d9 100644 (file)
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -2468,23 +2468,27 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
   void HandleLiteralCompareNil(CompareOperation* expr,
                                Expression* sub_expr,
                                NilValue nil);
-  HControlInstruction* BuildCompareInstruction(Token::Value op,
-                                               HValue* left,
-                                               HValue* right,
-                                               Type* left_type,
-                                               Type* right_type,
-                                               Type* combined_type,
-                                               HSourcePosition left_position,
-                                               HSourcePosition right_position,
-                                               BailoutId bailout_id);
-
-  HInstruction* BuildStringCharCodeAt(HValue* string,
-                                      HValue* index);
 
   enum PushBeforeSimulateBehavior {
     PUSH_BEFORE_SIMULATE,
     NO_PUSH_BEFORE_SIMULATE
   };
+
+  HControlInstruction* BuildCompareInstruction(
+      Token::Value op,
+      HValue* left,
+      HValue* right,
+      Type* left_type,
+      Type* right_type,
+      Type* combined_type,
+      HSourcePosition left_position,
+      HSourcePosition right_position,
+      PushBeforeSimulateBehavior push_sim_result,
+      BailoutId bailout_id);
+
+  HInstruction* BuildStringCharCodeAt(HValue* string,
+                                      HValue* index);
+
   HValue* BuildBinaryOperation(
       BinaryOperation* expr,
       HValue* left,

diff --git a/test/mjsunit/regress/comparison-in-effect-context-deopt.js b/test/mjsunit/regress/comparison-in-effect-context-deopt.js
new file mode 100644 (file)
index 0000000..b28dff7
--- /dev/null
+++ b/test/mjsunit/regress/comparison-in-effect-context-deopt.js
@@ -0,0 +1,47 @@
+// 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 lazyDeopt() {
+  %DeoptimizeFunction(test);
+  return "deopt";
+}
+
+var x = { toString : lazyDeopt };
+
+function g(x) {
+  return "result";
+}
+
+function test(x) {
+  return g(void(x == ""));
+}
+
+test(x);
+%OptimizeFunctionOnNextCall(test);
+assertEquals("result", test(x));