Fix a bug in SSA construction.
If a join block does not have an incoming value for a local
from a predecessor block, we should not create a phi. The
verifier has made sure the local is updated before any
following reads after this block.
Change-Id: Id2785efc73c9fb3224826fff2f4b4ad215905ff4
diff --git a/compiler/optimizing/ssa_builder.cc b/compiler/optimizing/ssa_builder.cc
index 33084df..1284a97 100644
--- a/compiler/optimizing/ssa_builder.cc
+++ b/compiler/optimizing/ssa_builder.cc
@@ -95,14 +95,26 @@
// All predecessors have already been visited because we are visiting in reverse post order.
// We merge the values of all locals, creating phis if those values differ.
for (size_t local = 0; local < current_locals_->Size(); local++) {
+ bool one_predecessor_has_no_value = false;
bool is_different = false;
HInstruction* value = ValueOfLocal(block->GetPredecessors().Get(0), local);
- for (size_t i = 1; i < block->GetPredecessors().Size(); i++) {
- if (ValueOfLocal(block->GetPredecessors().Get(i), local) != value) {
+
+ for (size_t i = 0, e = block->GetPredecessors().Size(); i < e; ++i) {
+ HInstruction* current = ValueOfLocal(block->GetPredecessors().Get(i), local);
+ if (current == nullptr) {
+// one_predecessor_has_no_value = true;
+// break;
+ } else if (current != value) {
is_different = true;
- break;
}
}
+
+ if (one_predecessor_has_no_value) {
+ // If one predecessor has no value for this local, we trust the verifier has
+ // successfully checked that there is a store dominating any read after this block.
+ continue;
+ }
+
if (is_different) {
HPhi* phi = new (GetGraph()->GetArena()) HPhi(
GetGraph()->GetArena(), local, block->GetPredecessors().Size(), Primitive::kPrimVoid);
diff --git a/compiler/optimizing/ssa_test.cc b/compiler/optimizing/ssa_test.cc
index d104619..485ea27 100644
--- a/compiler/optimizing/ssa_test.cc
+++ b/compiler/optimizing/ssa_test.cc
@@ -459,4 +459,34 @@
TestCode(data, expected);
}
+TEST(SsaTest, LocalInIf) {
+ // Test that we do not create a phi in the join block when one predecessor
+ // does not update the local.
+ const char* expected =
+ "BasicBlock 0, succ: 1\n"
+ " 0: IntConstant 0 [3, 3]\n"
+ " 1: IntConstant 4\n"
+ " 2: Goto\n"
+ "BasicBlock 1, pred: 0, succ: 2, 5\n"
+ " 3: Equal(0, 0) [4]\n"
+ " 4: If(3)\n"
+ "BasicBlock 2, pred: 1, succ: 3\n"
+ " 5: Goto\n"
+ "BasicBlock 3, pred: 2, 5, succ: 4\n"
+ " 6: ReturnVoid\n"
+ "BasicBlock 4, pred: 3\n"
+ " 7: Exit\n"
+ // Synthesized block to avoid critical edge.
+ "BasicBlock 5, pred: 1, succ: 3\n"
+ " 8: Goto\n";
+
+ const uint16_t data[] = TWO_REGISTERS_CODE_ITEM(
+ Instruction::CONST_4 | 0 | 0,
+ Instruction::IF_EQ, 3,
+ Instruction::CONST_4 | 4 << 12 | 1 << 8,
+ Instruction::RETURN_VOID);
+
+ TestCode(data, expected);
+}
+
} // namespace art
diff --git a/test/402-optimizing-control-flow/src/Main.java b/test/402-optimizing-control-flow/src/Main.java
index 3339ef4..c9c24dd 100644
--- a/test/402-optimizing-control-flow/src/Main.java
+++ b/test/402-optimizing-control-flow/src/Main.java
@@ -40,6 +40,9 @@
result = $opt$testForLoop(42);
expectEquals(44, result);
+
+ result = $opt$testIfWithLocal(5);
+ expectEquals(7, result);
}
static int $opt$testIfEq1(int a) {
@@ -73,4 +76,14 @@
for (; a != 44; a++) {}
return a;
}
+
+ static int $opt$testIfWithLocal(int a) {
+ if (a == 5) {
+ int f = 2;
+ a += f;
+ }
+ // The SSA builder should not create a phi for f.
+
+ return a;
+ }
}