From 671e48a4895cc1a0b7a1458d608f8c4f9b5cf85c Mon Sep 17 00:00:00 2001 From: Aart Bik Date: Wed, 9 Aug 2017 13:16:56 -0700 Subject: Fix performance regression. Rationale: One "improvement" overlooked in the previous CL hoists a try-test out of the optimization to make sure we don't change HIR when not needed. However, the try-test may affect the outcome of the test, so that was bad, bad! Bug: 64091002 Test: test-art-host Change-Id: Icf5f73e7cbeb209ee5fa5f6c1bef64fe127bb2fd --- compiler/optimizing/loop_optimization.cc | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'compiler/optimizing/loop_optimization.cc') diff --git a/compiler/optimizing/loop_optimization.cc b/compiler/optimizing/loop_optimization.cc index 4143462c71..027ba7741c 100644 --- a/compiler/optimizing/loop_optimization.cc +++ b/compiler/optimizing/loop_optimization.cc @@ -490,13 +490,18 @@ void HLoopOptimization::SimplifyInduction(LoopNode* node) { for (HInstructionIterator it(header->GetPhis()); !it.Done(); it.Advance()) { HPhi* phi = it.Current()->AsPhi(); if (TrySetPhiInduction(phi, /*restrict_uses*/ true) && - CanRemoveCycle() && TryAssignLastValue(node->loop_info, phi, preheader, /*collect_loop_uses*/ false)) { - simplified_ = true; - for (HInstruction* i : *iset_) { - RemoveFromCycle(i); + // Note that it's ok to have replaced uses after the loop with the last value, without + // being able to remove the cycle. Environment uses (which are the reason we may not be + // able to remove the cycle) within the loop will still hold the right value. We must + // have tried first, however, to replace outside uses. + if (CanRemoveCycle()) { + simplified_ = true; + for (HInstruction* i : *iset_) { + RemoveFromCycle(i); + } + DCHECK(CheckInductionSetFullyRemoved(iset_)); } - DCHECK(CheckInductionSetFullyRemoved(iset_)); } } } -- cgit v1.2.3-59-g8ed1b