Revision: 6871
Author: kmilli...@chromium.org
Date: Mon Feb 21 08:49:39 2011
Log: 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
http://code.google.com/p/v8/source/detail?r=6871
Modified:
/branches/bleeding_edge/src/arm/full-codegen-arm.cc
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/x64/full-codegen-x64.cc
/branches/bleeding_edge/test/mjsunit/regress/regress-1167.js
=======================================
--- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Wed Feb 16 01:20:16
2011
+++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Mon Feb 21 08:49:39
2011
@@ -3414,17 +3414,23 @@
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;
}
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Wed Feb 16
01:20:16 2011
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Mon Feb 21
08:49:39 2011
@@ -3782,17 +3782,22 @@
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;
}
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Mon Feb 14 15:41:47
2011
+++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Mon Feb 21 08:49:39
2011
@@ -3114,16 +3114,22 @@
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;
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/regress/regress-1167.js Thu Feb 17
05:05:49 2011
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-1167.js Mon Feb 21
08:49:39 2011
@@ -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 @@
// 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);
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
