Change the baseline compiler to match the Hydrogen graph builder.

The Hydrogen graph translation does not build a branch for unary negation in
an effect context, so the baseline compiler should not do so either.

Review URL: http://codereview.chromium.org/6546050

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

diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index f04a00e..9b589e6 100644 (file)
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -3414,17 +3414,23 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
 
     case Token::NOT: {
       Comment cmnt(masm_, "[ UnaryOperation (NOT)");
-      Label materialize_true, materialize_false;
-      Label* if_true = NULL;
-      Label* if_false = NULL;
-      Label* fall_through = NULL;
-
-      // Notice that the labels are swapped.
-      context()->PrepareTest(&materialize_true, &materialize_false,
-                             &if_false, &if_true, &fall_through);
-      if (context()->IsTest()) ForwardBailoutToChild(expr);
-      VisitForControl(expr->expression(), if_true, if_false, fall_through);
-      context()->Plug(if_false, if_true);  // Labels swapped.
+      if (context()->IsEffect()) {
+        // Unary NOT has no side effects so it's only necessary to visit the
+        // subexpression.  Match the optimizing compiler by not branching.
+        VisitForEffect(expr->expression());
+      } else {
+        Label materialize_true, materialize_false;
+        Label* if_true = NULL;
+        Label* if_false = NULL;
+        Label* fall_through = NULL;
+
+        // Notice that the labels are swapped.
+        context()->PrepareTest(&materialize_true, &materialize_false,
+                               &if_false, &if_true, &fall_through);
+        if (context()->IsTest()) ForwardBailoutToChild(expr);
+        VisitForControl(expr->expression(), if_true, if_false, fall_through);
+        context()->Plug(if_false, if_true);  // Labels swapped.
+      }
       break;
     }
 

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index a5c94c6..d32e9aa 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -3782,17 +3782,22 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
 
     case Token::NOT: {
       Comment cmnt(masm_, "[ UnaryOperation (NOT)");
-
-      Label materialize_true, materialize_false;
-      Label* if_true = NULL;
-      Label* if_false = NULL;
-      Label* fall_through = NULL;
-      // Notice that the labels are swapped.
-      context()->PrepareTest(&materialize_true, &materialize_false,
-                             &if_false, &if_true, &fall_through);
-      if (context()->IsTest()) ForwardBailoutToChild(expr);
-      VisitForControl(expr->expression(), if_true, if_false, fall_through);
-      context()->Plug(if_false, if_true);  // Labels swapped.
+      if (context()->IsEffect()) {
+        // Unary NOT has no side effects so it's only necessary to visit the
+        // subexpression.  Match the optimizing compiler by not branching.
+        VisitForEffect(expr->expression());
+      } else {
+        Label materialize_true, materialize_false;
+        Label* if_true = NULL;
+        Label* if_false = NULL;
+        Label* fall_through = NULL;
+        // Notice that the labels are swapped.
+        context()->PrepareTest(&materialize_true, &materialize_false,
+                               &if_false, &if_true, &fall_through);
+        if (context()->IsTest()) ForwardBailoutToChild(expr);
+        VisitForControl(expr->expression(), if_true, if_false, fall_through);
+        context()->Plug(if_false, if_true);  // Labels swapped.
+      }
       break;
     }
 

diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index a28bcb7..11b07d7 100644 (file)
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -3114,16 +3114,22 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
 
     case Token::NOT: {
       Comment cmnt(masm_, "[ UnaryOperation (NOT)");
-      Label materialize_true, materialize_false;
-      Label* if_true = NULL;
-      Label* if_false = NULL;
-      Label* fall_through = NULL;
-      // Notice that the labels are swapped.
-      context()->PrepareTest(&materialize_true, &materialize_false,
-                             &if_false, &if_true, &fall_through);
-      if (context()->IsTest()) ForwardBailoutToChild(expr);
-      VisitForControl(expr->expression(), if_true, if_false, fall_through);
-      context()->Plug(if_false, if_true);  // Labels swapped.
+      if (context()->IsEffect()) {
+        // Unary NOT has no side effects so it's only necessary to visit the
+        // subexpression.  Match the optimizing compiler by not branching.
+        VisitForEffect(expr->expression());
+      } else {
+        Label materialize_true, materialize_false;
+        Label* if_true = NULL;
+        Label* if_false = NULL;
+        Label* fall_through = NULL;
+        // Notice that the labels are swapped.
+        context()->PrepareTest(&materialize_true, &materialize_false,
+                               &if_false, &if_true, &fall_through);
+        if (context()->IsTest()) ForwardBailoutToChild(expr);
+        VisitForControl(expr->expression(), if_true, if_false, fall_through);
+        context()->Plug(if_false, if_true);  // Labels swapped.
+      }
       break;
     }
 

diff --git a/test/mjsunit/regress/regress-1167.js b/test/mjsunit/regress/regress-1167.js
index ea37ba0..8437d83 100644 (file)
--- a/test/mjsunit/regress/regress-1167.js
+++ b/test/mjsunit/regress/regress-1167.js
@@ -27,7 +27,7 @@
 
 // Deoptimization after a logical not in an effect context should not see a
 // value for the logical not expression.
-function test(n) {
+function test0(n) {
   var a = new Array(n);
   for (var i = 0; i < n; ++i) {
     // ~ of a non-numeric value is used to trigger deoptimization.
@@ -38,6 +38,35 @@ function test(n) {
 // OSR (after deoptimization) is used to observe the stack height mismatch.
 for (var i = 0; i < 5; ++i) {
   for (var j = 1; j < 12; ++j) {
-    test(j * 1000);
+    test0(j * 1000);
   }
 }
+
+
+// Similar test with a different subexpression of unary !.
+function test1(n) {
+  var a = new Array(n);
+  for (var i = 0; i < n; ++i) {
+    a[i] = void(!(- 'object')) % ~(delete 4);
+  }
+}
+
+for (i = 0; i < 5; ++i) {
+  for (j = 1; j < 12; ++j) {
+    test1(j * 1000);
+  }
+}
+
+
+// A similar issue, different subexpression of unary ! (e0 !== e1 is
+// translated into !(e0 == e1)) and different effect context.
+function side_effect() { }
+function observe(x, y) { return x; }
+function test2(x) {
+  return observe(this,
+                 (((side_effect.observe <= side_effect.side_effect) !== false),
+                  x + 1));
+}
+
+for (var i = 0; i < 1000000; ++i) test2(0);
+test2(test2);