From: kmillikin@chromium.org Date: Mon, 21 Feb 2011 16:49:39 +0000 (+0000) Subject: Change the baseline compiler to match the Hydrogen graph builder. X-Git-Tag: upstream/4.7.83~20142 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=123dbb2f5efabeb4b5111d6e086861bab69afafa;p=platform%2Fupstream%2Fv8.git 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 --- 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 --- 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 --- 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 --- 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);