From eb502fe5999b7d963eeef3553c930747883c7349 Mon Sep 17 00:00:00 2001 From: "jarin@chromium.org" Date: Thu, 6 Feb 2014 09:36:55 +0000 Subject: [PATCH] Binary operation deoptimization fix. R=jkummerow@chromium.org BUG= Review URL: https://codereview.chromium.org/132453009 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19132 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/hydrogen.cc | 22 ++++++-- src/hydrogen.h | 13 ++++- .../regress/binop-in-effect-context-deopt.js | 65 ++++++++++++++++++++++ 3 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 test/mjsunit/regress/binop-in-effect-context-deopt.js diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 591f00f..8037c8d 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -5980,7 +5980,8 @@ void HOptimizedGraphBuilder::HandleCompoundAssignment(Assignment* expr) { HValue* right = Pop(); HValue* left = Pop(); - Push(BuildBinaryOperation(operation, left, right)); + Push(BuildBinaryOperation(operation, left, right, PUSH_BEFORE_SIMULATE)); + BuildStore(expr, prop, expr->id(), expr->AssignmentId(), expr->IsUninitialized()); } else { @@ -9056,7 +9057,8 @@ HValue* HGraphBuilder::TruncateToNumber(HValue* value, Type** expected) { HValue* HOptimizedGraphBuilder::BuildBinaryOperation( BinaryOperation* expr, HValue* left, - HValue* right) { + HValue* right, + PushBeforeSimulateBehavior push_sim_result) { Type* left_type = expr->left()->bounds().lower; Type* right_type = expr->right()->bounds().lower; Type* result_type = expr->bounds().lower; @@ -9080,9 +9082,14 @@ HValue* HOptimizedGraphBuilder::BuildBinaryOperation( // after phis, which are the result of BuildBinaryOperation when we // inlined some complex subgraph. if (result->HasObservableSideEffects() || result->IsPhi()) { - Push(result); - Add(expr->id(), REMOVABLE_SIMULATE); - Drop(1); + if (push_sim_result == NO_PUSH_BEFORE_SIMULATE) { + Add(expr->id(), REMOVABLE_SIMULATE); + } else { + ASSERT(push_sim_result == PUSH_BEFORE_SIMULATE); + Push(result); + Add(expr->id(), REMOVABLE_SIMULATE); + Drop(1); + } } return result; } @@ -9462,7 +9469,10 @@ void HOptimizedGraphBuilder::VisitArithmeticExpression(BinaryOperation* expr) { SetSourcePosition(expr->position()); HValue* right = Pop(); HValue* left = Pop(); - HValue* result = BuildBinaryOperation(expr, left, right); + HValue* result = + BuildBinaryOperation(expr, left, right, + ast_context()->IsEffect() ? NO_PUSH_BEFORE_SIMULATE + : PUSH_BEFORE_SIMULATE); if (FLAG_emit_opt_code_positions && result->IsBinaryOperation()) { HBinaryOperation::cast(result)->SetOperandPositions( zone(), expr->left()->position(), expr->right()->position()); diff --git a/src/hydrogen.h b/src/hydrogen.h index 2ee87e7..1a87875 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -2435,9 +2435,16 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { HInstruction* BuildStringCharCodeAt(HValue* string, HValue* index); - HValue* BuildBinaryOperation(BinaryOperation* expr, - HValue* left, - HValue* right); + + enum PushBeforeSimulateBehavior { + PUSH_BEFORE_SIMULATE, + NO_PUSH_BEFORE_SIMULATE + }; + HValue* BuildBinaryOperation( + BinaryOperation* expr, + HValue* left, + HValue* right, + PushBeforeSimulateBehavior push_sim_result); HInstruction* BuildIncrement(bool returns_original_input, CountOperation* expr); HInstruction* BuildLoadKeyedGeneric(HValue* object, diff --git a/test/mjsunit/regress/binop-in-effect-context-deopt.js b/test/mjsunit/regress/binop-in-effect-context-deopt.js new file mode 100644 index 0000000..fb7280a --- /dev/null +++ b/test/mjsunit/regress/binop-in-effect-context-deopt.js @@ -0,0 +1,65 @@ +// 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 BinopInEffectContextDeoptAndOsr() { + function f(a, deopt, osr) { + var result = (a + 10, "result"); + var dummy = deopt + 0; + if (osr) while (%GetOptimizationStatus(f) == 2) {} + return result; + } + + assertEquals("result", f(true, 3, false)); + assertEquals("result", f(true, 3, false)); + %OptimizeFunctionOnNextCall(f); + assertEquals("result", f(true, "foo", true)); +})(); + + +(function BinopInEffectContextLazyDeopt() { + function deopt_f() { + %DeoptimizeFunction(f); + return "dummy"; + } + + function h() { + return { toString : deopt_f }; + } + + function g(x) { + } + + function f() { + return g(void(h() + "")); + }; + + f(); + %OptimizeFunctionOnNextCall(f); + f(); +})(); -- 2.7.4