ART: Fix overlapping instruction IDs in inliner
Inliner creates the inner graph so that it generates instruction IDs
higher than the outer graph. This was broken because the inliner
would create instructions in the outer graph before the inner graph
is inlined.
The bug cannot be triggered because the offending instruction would
share the same ID as the first inner HLocal, which is removed before
the inner graph is inlined. The added DCHECKs reveal the hidden problem
and make it safe for HLocals to be removed in the future.
Change-Id: I486eb0f3987e20c50cbec0fb06332229e07fbae9
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index 3f67e48..3e3719e 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -1010,6 +1010,8 @@
// at runtime, we change this call as if it was a virtual call.
invoke_type = kVirtual;
}
+
+ const int32_t caller_instruction_counter = graph_->GetCurrentInstructionId();
HGraph* callee_graph = new (graph_->GetArena()) HGraph(
graph_->GetArena(),
callee_dex_file,
@@ -1019,7 +1021,7 @@
invoke_type,
graph_->IsDebuggable(),
/* osr */ false,
- graph_->GetCurrentInstructionId());
+ caller_instruction_counter);
callee_graph->SetArtMethod(resolved_method);
OptimizingCompilerStats inline_stats;
@@ -1219,7 +1221,16 @@
}
number_of_inlined_instructions_ += number_of_instructions;
+ DCHECK_EQ(caller_instruction_counter, graph_->GetCurrentInstructionId())
+ << "No instructions can be added to the outer graph while inner graph is being built";
+
+ const int32_t callee_instruction_counter = callee_graph->GetCurrentInstructionId();
+ graph_->SetCurrentInstructionId(callee_instruction_counter);
*return_replacement = callee_graph->InlineInto(graph_, invoke_instruction);
+
+ DCHECK_EQ(callee_instruction_counter, callee_graph->GetCurrentInstructionId())
+ << "No instructions can be added to the inner graph during inlining into the outer graph";
+
return true;
}
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 87b9c02..0e0b83e 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -1976,34 +1976,6 @@
at->MergeWithInlined(first);
exit_block_->ReplaceWith(to);
- // Update all predecessors of the exit block (now the `to` block)
- // to not `HReturn` but `HGoto` instead.
- bool returns_void = to->GetPredecessors()[0]->GetLastInstruction()->IsReturnVoid();
- if (to->GetPredecessors().size() == 1) {
- HBasicBlock* predecessor = to->GetPredecessors()[0];
- HInstruction* last = predecessor->GetLastInstruction();
- if (!returns_void) {
- return_value = last->InputAt(0);
- }
- predecessor->AddInstruction(new (allocator) HGoto(last->GetDexPc()));
- predecessor->RemoveInstruction(last);
- } else {
- if (!returns_void) {
- // There will be multiple returns.
- return_value = new (allocator) HPhi(
- allocator, kNoRegNumber, 0, HPhi::ToPhiType(invoke->GetType()), to->GetDexPc());
- to->AddPhi(return_value->AsPhi());
- }
- for (HBasicBlock* predecessor : to->GetPredecessors()) {
- HInstruction* last = predecessor->GetLastInstruction();
- if (!returns_void) {
- return_value->AsPhi()->AddInput(last->InputAt(0));
- }
- predecessor->AddInstruction(new (allocator) HGoto(last->GetDexPc()));
- predecessor->RemoveInstruction(last);
- }
- }
-
// Update the meta information surrounding blocks:
// (1) the graph they are now in,
// (2) the reverse post order of that graph,
@@ -2048,11 +2020,36 @@
// Only `to` can become a back edge, as the inlined blocks
// are predecessors of `to`.
UpdateLoopAndTryInformationOfNewBlock(to, at, /* replace_if_back_edge */ true);
- }
- // Update the next instruction id of the outer graph, so that instructions
- // added later get bigger ids than those in the inner graph.
- outer_graph->SetCurrentInstructionId(GetNextInstructionId());
+ // Update all predecessors of the exit block (now the `to` block)
+ // to not `HReturn` but `HGoto` instead.
+ bool returns_void = to->GetPredecessors()[0]->GetLastInstruction()->IsReturnVoid();
+ if (to->GetPredecessors().size() == 1) {
+ HBasicBlock* predecessor = to->GetPredecessors()[0];
+ HInstruction* last = predecessor->GetLastInstruction();
+ if (!returns_void) {
+ return_value = last->InputAt(0);
+ }
+ predecessor->AddInstruction(new (allocator) HGoto(last->GetDexPc()));
+ predecessor->RemoveInstruction(last);
+ } else {
+ if (!returns_void) {
+ // There will be multiple returns.
+ return_value = new (allocator) HPhi(
+ allocator, kNoRegNumber, 0, HPhi::ToPhiType(invoke->GetType()), to->GetDexPc());
+ to->AddPhi(return_value->AsPhi());
+ }
+ for (HBasicBlock* predecessor : to->GetPredecessors()) {
+ HInstruction* last = predecessor->GetLastInstruction();
+ if (!returns_void) {
+ DCHECK(last->IsReturn());
+ return_value->AsPhi()->AddInput(last->InputAt(0));
+ }
+ predecessor->AddInstruction(new (allocator) HGoto(last->GetDexPc()));
+ predecessor->RemoveInstruction(last);
+ }
+ }
+ }
// Walk over the entry block and:
// - Move constants from the entry block to the outer_graph's entry block,
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index e3dbe16..b684cc6 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -387,6 +387,7 @@
}
void SetCurrentInstructionId(int32_t id) {
+ DCHECK_GE(id, current_instruction_id_);
current_instruction_id_ = id;
}