Fix loop optimization in the presence of environment uses.
We should not remove instructions that have deoptimize as
users, or that have environment uses in a debuggable setup.
bug: 62536525
bug: 33775412
Test: 656-loop-deopt
Change-Id: Iaec1a0b6e90c6a0169f18c6985f00fd8baf2dece
diff --git a/compiler/optimizing/loop_optimization.cc b/compiler/optimizing/loop_optimization.cc
index 9c8a632..d249313 100644
--- a/compiler/optimizing/loop_optimization.cc
+++ b/compiler/optimizing/loop_optimization.cc
@@ -422,6 +422,23 @@
// Optimization.
//
+bool HLoopOptimization::CanRemoveCycle() {
+ for (HInstruction* i : *iset_) {
+ // We can never remove instructions that have environment
+ // uses when we compile 'debuggable'.
+ if (i->HasEnvironmentUses() && graph_->IsDebuggable()) {
+ return false;
+ }
+ // A deoptimization should never have an environment input removed.
+ for (const HUseListNode<HEnvironment*>& use : i->GetEnvUses()) {
+ if (use.GetUser()->GetHolder()->IsDeoptimize()) {
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
void HLoopOptimization::SimplifyInduction(LoopNode* node) {
HBasicBlock* header = node->loop_info->GetHeader();
HBasicBlock* preheader = node->loop_info->GetPreHeader();
@@ -435,10 +452,15 @@
iset_->clear(); // prepare phi induction
if (TrySetPhiInduction(phi, /*restrict_uses*/ true) &&
TryAssignLastValue(node->loop_info, phi, preheader, /*collect_loop_uses*/ false)) {
- 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.
+ if (CanRemoveCycle()) {
+ for (HInstruction* i : *iset_) {
+ RemoveFromCycle(i);
+ }
+ simplified_ = true;
}
- simplified_ = true;
}
}
}
@@ -1527,11 +1549,10 @@
return true;
}
-bool HLoopOptimization::TryReplaceWithLastValue(HInstruction* instruction, HBasicBlock* block) {
- // Try to replace outside uses with the last value. Environment uses can consume this
- // value too, since any first true use is outside the loop (although this may imply
- // that de-opting may look "ahead" a bit on the phi value). If there are only environment
- // uses, the value is dropped altogether, since the computations have no effect.
+bool HLoopOptimization::TryReplaceWithLastValue(HLoopInformation* loop_info,
+ HInstruction* instruction,
+ HBasicBlock* block) {
+ // Try to replace outside uses with the last value.
if (induction_range_.CanGenerateLastValue(instruction)) {
HInstruction* replacement = induction_range_.GenerateLastValue(instruction, graph_, block);
const HUseList<HInstruction*>& uses = instruction->GetUses();
@@ -1540,6 +1561,11 @@
size_t index = it->GetIndex();
++it; // increment before replacing
if (iset_->find(user) == iset_->end()) { // not excluded?
+ if (kIsDebugBuild) {
+ // We have checked earlier in 'IsOnlyUsedAfterLoop' that the use is after the loop.
+ HLoopInformation* other_loop_info = user->GetBlock()->GetLoopInformation();
+ CHECK(other_loop_info == nullptr || !other_loop_info->IsIn(*loop_info));
+ }
user->ReplaceInput(replacement, index);
induction_range_.Replace(user, instruction, replacement); // update induction
}
@@ -1550,9 +1576,13 @@
size_t index = it->GetIndex();
++it; // increment before replacing
if (iset_->find(user->GetHolder()) == iset_->end()) { // not excluded?
- user->RemoveAsUserOfInput(index);
- user->SetRawEnvAt(index, replacement);
- replacement->AddEnvUseAt(user, index);
+ HLoopInformation* other_loop_info = user->GetHolder()->GetBlock()->GetLoopInformation();
+ // Only update environment uses after the loop.
+ if (other_loop_info == nullptr || !other_loop_info->IsIn(*loop_info)) {
+ user->RemoveAsUserOfInput(index);
+ user->SetRawEnvAt(index, replacement);
+ replacement->AddEnvUseAt(user, index);
+ }
}
}
induction_simplication_count_++;
@@ -1571,7 +1601,7 @@
int32_t use_count = 0;
return IsOnlyUsedAfterLoop(loop_info, instruction, collect_loop_uses, &use_count) &&
(use_count == 0 ||
- (!IsEarlyExit(loop_info) && TryReplaceWithLastValue(instruction, block)));
+ (!IsEarlyExit(loop_info) && TryReplaceWithLastValue(loop_info, instruction, block)));
}
void HLoopOptimization::RemoveDeadInstructions(const HInstructionList& list) {