From 22f2fd83977afc9fc813bb86c48023d44fcda5ea Mon Sep 17 00:00:00 2001 From: "jkummerow@chromium.org" Date: Wed, 17 Jul 2013 13:13:38 +0000 Subject: [PATCH] Synchronize Compare-Literal behavior in FullCodegen and Hydrogen BUG=chromium:260345 R=danno@chromium.org Review URL: https://codereview.chromium.org/19582002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15723 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/hydrogen.cc | 91 +++++--------------- src/hydrogen.h | 4 +- test/mjsunit/regress/regress-crbug-260345.js | 59 +++++++++++++ 3 files changed, 83 insertions(+), 71 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-260345.js diff --git a/src/hydrogen.cc b/src/hydrogen.cc index c1e749001b..7d120b7f81 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -8132,68 +8132,16 @@ void HOptimizedGraphBuilder::VisitArithmeticExpression(BinaryOperation* expr) { void HOptimizedGraphBuilder::HandleLiteralCompareTypeof(CompareOperation* expr, - HTypeof* typeof_expr, + Expression* sub_expr, Handle check) { - // Note: The HTypeof itself is removed during canonicalization, if possible. - HValue* value = typeof_expr->value(); + CHECK_ALIVE(VisitForValue(sub_expr)); + HValue* value = Pop(); HTypeofIsAndBranch* instr = new(zone()) HTypeofIsAndBranch(value, check); instr->set_position(expr->position()); return ast_context()->ReturnControl(instr, expr->id()); } -static bool MatchLiteralCompareNil(HValue* left, - Token::Value op, - HValue* right, - Handle nil, - HValue** expr) { - if (left->IsConstant() && - HConstant::cast(left)->handle().is_identical_to(nil) && - Token::IsEqualityOp(op)) { - *expr = right; - return true; - } - return false; -} - - -static bool MatchLiteralCompareTypeof(HValue* left, - Token::Value op, - HValue* right, - HTypeof** typeof_expr, - Handle* check) { - if (left->IsTypeof() && - Token::IsEqualityOp(op) && - right->IsConstant() && - HConstant::cast(right)->handle()->IsString()) { - *typeof_expr = HTypeof::cast(left); - *check = Handle::cast(HConstant::cast(right)->handle()); - return true; - } - return false; -} - - -static bool IsLiteralCompareTypeof(HValue* left, - Token::Value op, - HValue* right, - HTypeof** typeof_expr, - Handle* check) { - return MatchLiteralCompareTypeof(left, op, right, typeof_expr, check) || - MatchLiteralCompareTypeof(right, op, left, typeof_expr, check); -} - - -static bool IsLiteralCompareNil(HValue* left, - Token::Value op, - HValue* right, - Handle nil, - HValue** expr) { - return MatchLiteralCompareNil(left, op, right, nil, expr) || - MatchLiteralCompareNil(right, op, left, nil, expr); -} - - static bool IsLiteralCompareBool(HValue* left, Token::Value op, HValue* right) { @@ -8207,6 +8155,22 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { ASSERT(!HasStackOverflow()); ASSERT(current_block() != NULL); ASSERT(current_block()->HasPredecessor()); + + // Check for a few fast cases. The AST visiting behavior must be in sync + // with the full codegen: We don't push both left and right values onto + // the expression stack when one side is a special-case literal. + Expression* sub_expr = NULL; + Handle check; + if (expr->IsLiteralCompareTypeof(&sub_expr, &check)) { + return HandleLiteralCompareTypeof(expr, sub_expr, check); + } + if (expr->IsLiteralCompareUndefined(&sub_expr)) { + return HandleLiteralCompareNil(expr, sub_expr, kUndefinedValue); + } + if (expr->IsLiteralCompareNull(&sub_expr)) { + return HandleLiteralCompareNil(expr, sub_expr, kNullValue); + } + if (IsClassOfTest(expr)) { CallRuntime* call = expr->left()->AsCallRuntime(); ASSERT(call->arguments()->length() == 1); @@ -8235,19 +8199,6 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { HValue* left = Pop(); Token::Value op = expr->op(); - HTypeof* typeof_expr = NULL; - Handle check; - if (IsLiteralCompareTypeof(left, op, right, &typeof_expr, &check)) { - return HandleLiteralCompareTypeof(expr, typeof_expr, check); - } - HValue* sub_expr = NULL; - Factory* f = isolate()->factory(); - if (IsLiteralCompareNil(left, op, right, f->undefined_value(), &sub_expr)) { - return HandleLiteralCompareNil(expr, sub_expr, kUndefinedValue); - } - if (IsLiteralCompareNil(left, op, right, f->null_value(), &sub_expr)) { - return HandleLiteralCompareNil(expr, sub_expr, kNullValue); - } if (IsLiteralCompareBool(left, op, right)) { HCompareObjectEqAndBranch* result = new(zone()) HCompareObjectEqAndBranch(left, right); @@ -8374,12 +8325,14 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { void HOptimizedGraphBuilder::HandleLiteralCompareNil(CompareOperation* expr, - HValue* value, + Expression* sub_expr, NilValue nil) { ASSERT(!HasStackOverflow()); ASSERT(current_block() != NULL); ASSERT(current_block()->HasPredecessor()); ASSERT(expr->op() == Token::EQ || expr->op() == Token::EQ_STRICT); + CHECK_ALIVE(VisitForValue(sub_expr)); + HValue* value = Pop(); HIfContinuation continuation; if (expr->op() == Token::EQ_STRICT) { IfBuilder if_nil(this); diff --git a/src/hydrogen.h b/src/hydrogen.h index 4c80280719..7d9a7affb8 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -1765,10 +1765,10 @@ class HOptimizedGraphBuilder: public HGraphBuilder, public AstVisitor { SmallMapList* types, Handle name); void HandleLiteralCompareTypeof(CompareOperation* expr, - HTypeof* typeof_expr, + Expression* sub_expr, Handle check); void HandleLiteralCompareNil(CompareOperation* expr, - HValue* value, + Expression* sub_expr, NilValue nil); HInstruction* BuildStringCharCodeAt(HValue* context, diff --git a/test/mjsunit/regress/regress-crbug-260345.js b/test/mjsunit/regress/regress-crbug-260345.js new file mode 100644 index 0000000000..75832ab4be --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-260345.js @@ -0,0 +1,59 @@ +// Copyright 2013 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. + +var steps = 100000; +var undefined_values = [undefined, "go on"]; +var null_values = [null, "go on"]; + +function get_undefined_object(i) { + return undefined_values[(i / steps) | 0]; +} + +function test_undefined() { + var objects = 0; + for (var i = 0; i < 2 * steps; i++) { + undefined == get_undefined_object(i) && objects++; + } + return objects; +} + +assertEquals(steps, test_undefined()); + + +function get_null_object(i) { + return null_values[(i / steps) | 0]; +} + +function test_null() { + var objects = 0; + for (var i = 0; i < 2 * steps; i++) { + null == get_null_object(i) && objects++; + } + return objects; +} + +assertEquals(steps, test_null());