From 4c7ed144e1c82994ab97b5f8499f541d39fa0a9d Mon Sep 17 00:00:00 2001 From: "jarin@chromium.org" Date: Sun, 16 Feb 2014 05:51:10 +0000 Subject: [PATCH] Comparison in effect context lazy deopt fix. R=jkummerow@chromium.org BUG= Review URL: https://codereview.chromium.org/163623002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19396 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/hydrogen.cc | 25 +++++++----- src/hydrogen.h | 28 +++++++------ .../regress/comparison-in-effect-context-deopt.js | 47 ++++++++++++++++++++++ 3 files changed, 79 insertions(+), 21 deletions(-) create mode 100644 test/mjsunit/regress/comparison-in-effect-context-deopt.js diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 05d31ec..fb9806e 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -4448,7 +4448,7 @@ void HOptimizedGraphBuilder::VisitSwitchStatement(SwitchStatement* stmt) { combined_type, ScriptPositionToSourcePosition(stmt->tag()->position()), ScriptPositionToSourcePosition(clause->label()->position()), - clause->id()); + PUSH_BEFORE_SIMULATE, clause->id()); HBasicBlock* next_test_block = graph()->CreateBasicBlock(); HBasicBlock* body_block = graph()->CreateBasicBlock(); @@ -9087,13 +9087,12 @@ HValue* HOptimizedGraphBuilder::BuildBinaryOperation( // after phis, which are the result of BuildBinaryOperation when we // inlined some complex subgraph. if (result->HasObservableSideEffects() || result->IsPhi()) { - if (push_sim_result == NO_PUSH_BEFORE_SIMULATE) { - Add(expr->id(), REMOVABLE_SIMULATE); - } else { - ASSERT(push_sim_result == PUSH_BEFORE_SIMULATE); + if (push_sim_result == PUSH_BEFORE_SIMULATE) { Push(result); Add(expr->id(), REMOVABLE_SIMULATE); Drop(1); + } else { + Add(expr->id(), REMOVABLE_SIMULATE); } } return result; @@ -9611,11 +9610,14 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { return ast_context()->ReturnInstruction(result, expr->id()); } + PushBeforeSimulateBehavior push_behavior = + ast_context()->IsEffect() ? NO_PUSH_BEFORE_SIMULATE + : PUSH_BEFORE_SIMULATE; HControlInstruction* compare = BuildCompareInstruction( op, left, right, left_type, right_type, combined_type, ScriptPositionToSourcePosition(expr->left()->position()), ScriptPositionToSourcePosition(expr->right()->position()), - expr->id()); + push_behavior, expr->id()); if (compare == NULL) return; // Bailed out. return ast_context()->ReturnControl(compare, expr->id()); } @@ -9630,6 +9632,7 @@ HControlInstruction* HOptimizedGraphBuilder::BuildCompareInstruction( Type* combined_type, HSourcePosition left_position, HSourcePosition right_position, + PushBeforeSimulateBehavior push_sim_result, BailoutId bailout_id) { // Cases handled below depend on collected type feedback. They should // soft deoptimize when there is no type feedback. @@ -9694,9 +9697,13 @@ HControlInstruction* HOptimizedGraphBuilder::BuildCompareInstruction( result->set_observed_input_representation(1, left_rep); result->set_observed_input_representation(2, right_rep); if (result->HasObservableSideEffects()) { - Push(result); - AddSimulate(bailout_id, REMOVABLE_SIMULATE); - Drop(1); + if (push_sim_result == PUSH_BEFORE_SIMULATE) { + Push(result); + AddSimulate(bailout_id, REMOVABLE_SIMULATE); + Drop(1); + } else { + AddSimulate(bailout_id, REMOVABLE_SIMULATE); + } } // TODO(jkummerow): Can we make this more efficient? HBranch* branch = New(result); diff --git a/src/hydrogen.h b/src/hydrogen.h index 5f1ef79..8668698 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -2468,23 +2468,27 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { void HandleLiteralCompareNil(CompareOperation* expr, Expression* sub_expr, NilValue nil); - HControlInstruction* BuildCompareInstruction(Token::Value op, - HValue* left, - HValue* right, - Type* left_type, - Type* right_type, - Type* combined_type, - HSourcePosition left_position, - HSourcePosition right_position, - BailoutId bailout_id); - - HInstruction* BuildStringCharCodeAt(HValue* string, - HValue* index); enum PushBeforeSimulateBehavior { PUSH_BEFORE_SIMULATE, NO_PUSH_BEFORE_SIMULATE }; + + HControlInstruction* BuildCompareInstruction( + Token::Value op, + HValue* left, + HValue* right, + Type* left_type, + Type* right_type, + Type* combined_type, + HSourcePosition left_position, + HSourcePosition right_position, + PushBeforeSimulateBehavior push_sim_result, + BailoutId bailout_id); + + HInstruction* BuildStringCharCodeAt(HValue* string, + HValue* index); + HValue* BuildBinaryOperation( BinaryOperation* expr, HValue* left, diff --git a/test/mjsunit/regress/comparison-in-effect-context-deopt.js b/test/mjsunit/regress/comparison-in-effect-context-deopt.js new file mode 100644 index 0000000..b28dff7 --- /dev/null +++ b/test/mjsunit/regress/comparison-in-effect-context-deopt.js @@ -0,0 +1,47 @@ +// 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 lazyDeopt() { + %DeoptimizeFunction(test); + return "deopt"; +} + +var x = { toString : lazyDeopt }; + +function g(x) { + return "result"; +} + +function test(x) { + return g(void(x == "")); +} + +test(x); +%OptimizeFunctionOnNextCall(test); +assertEquals("result", test(x)); -- 2.7.4