From d2179f2062ee6f2244642c06ab918676f16c614b Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Fri, 25 Apr 2014 09:52:11 +0000 Subject: [PATCH] Don't adopt the AST id from previous if id is none, since previous may have mismatching expected stack height. Additionally, harden merging of simulates after instructions with side effects and ensure there's a simulate before HEnterInlined. R=jarin@chromium.org Review URL: https://codereview.chromium.org/252583004 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20967 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/hydrogen-instructions.cc | 3 -- src/hydrogen-removable-simulates.cc | 40 +++++++++++++++------- .../regress/regress-lazy-deopt-inlining2.js | 24 +++++++++++++ 3 files changed, 51 insertions(+), 16 deletions(-) create mode 100644 test/mjsunit/regress/regress-lazy-deopt-inlining2.js diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index ceedafc..2253a0c 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -2595,9 +2595,6 @@ void HPhi::AddIndirectUsesTo(int* dest) { void HSimulate::MergeWith(ZoneList* list) { - if (!list->is_empty() && !HasAstId()) { - set_ast_id(list->last()->ast_id()); - } while (!list->is_empty()) { HSimulate* from = list->RemoveLast(); ZoneList* from_values = &from->values_; diff --git a/src/hydrogen-removable-simulates.cc b/src/hydrogen-removable-simulates.cc index f952832..bd8de0b 100644 --- a/src/hydrogen-removable-simulates.cc +++ b/src/hydrogen-removable-simulates.cc @@ -41,14 +41,17 @@ void HMergeRemovableSimulatesPhase::Run() { bool first = true; for (HInstructionIterator it(block); !it.Done(); it.Advance()) { HInstruction* current = it.Current(); + if (current->IsEnterInlined()) { + // Ensure there's a non-foldable HSimulate before an HEnterInlined to + // avoid folding across HEnterInlined. + ASSERT(!HSimulate::cast(current->previous())-> + is_candidate_for_removal()); + } if (current->IsLeaveInlined()) { - // Never fold simulates from inlined environments into simulates - // in the outer environment. - // (Before each HEnterInlined, there is a non-foldable HSimulate - // anyway, so we get the barrier in the other direction for free.) - // Simply remove all accumulated simulates without merging. This - // is safe because simulates after instructions with side effects - // are never added to the merge list. + // Never fold simulates from inlined environments into simulates in the + // outer environment. Simply remove all accumulated simulates without + // merging. This is safe because simulates after instructions with side + // effects are never added to the merge list. while (!mergelist.is_empty()) { mergelist.RemoveLast()->DeleteAndReplaceWith(NULL); } @@ -70,13 +73,24 @@ void HMergeRemovableSimulatesPhase::Run() { continue; } HSimulate* current_simulate = HSimulate::cast(current); - if ((current_simulate->previous()->HasObservableSideEffects() && - !current_simulate->next()->IsSimulate()) || - !current_simulate->is_candidate_for_removal()) { - // This simulate is not suitable for folding. - // Fold the ones accumulated so far. + if (!current_simulate->is_candidate_for_removal()) { + current_simulate->MergeWith(&mergelist); + } else if (current_simulate->ast_id().IsNone()) { + ASSERT(current_simulate->next()->IsEnterInlined()); + if (!mergelist.is_empty()) { + HSimulate* last = mergelist.RemoveLast(); + last->MergeWith(&mergelist); + } + } else if (current_simulate->previous()->HasObservableSideEffects()) { + while (current_simulate->next()->IsSimulate()) { + it.Advance(); + HSimulate* next_simulate = HSimulate::cast(it.Current()); + if (next_simulate->ast_id().IsNone()) break; + mergelist.Add(current_simulate, zone()); + current_simulate = next_simulate; + if (!current_simulate->is_candidate_for_removal()) break; + } current_simulate->MergeWith(&mergelist); - continue; } else { // Accumulate this simulate for folding later on. mergelist.Add(current_simulate, zone()); diff --git a/test/mjsunit/regress/regress-lazy-deopt-inlining2.js b/test/mjsunit/regress/regress-lazy-deopt-inlining2.js new file mode 100644 index 0000000..7b73b14 --- /dev/null +++ b/test/mjsunit/regress/regress-lazy-deopt-inlining2.js @@ -0,0 +1,24 @@ +// Copyright 2014 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +"use strict"; +function f1(d) { + return 1 + f2(1, f3(d), d); +} + +function f2(v0, v1, v2) { return v1; } + +function f3(d) { + if (d) %DeoptimizeFunction(f1); + return 2; +} + +%NeverOptimizeFunction(f3); + +f1(false); +f1(false); +%OptimizeFunctionOnNextCall(f1); +assertEquals(3, f1(true)); -- 2.7.4