Quick: Fix DCE to rename SSA regs for degenerate Phis.
If we're inserting a new Phi and all its inputs are the same
SSA reg (for example, we allow a single-input Phi), some
subsequent insn may actually refer to that reg, so we need
to rename it to keep the graph in a consistent state.
(cherry picked from commit f60715c2c0a2398822a143ffb6b75507bffca703)
Change-Id: I46822b0db21c187601b79d2ff1a650494558772a
diff --git a/compiler/dex/gvn_dead_code_elimination.cc b/compiler/dex/gvn_dead_code_elimination.cc
index 4f0e9d1..a4319c3 100644
--- a/compiler/dex/gvn_dead_code_elimination.cc
+++ b/compiler/dex/gvn_dead_code_elimination.cc
@@ -533,7 +533,7 @@
// Just before we kill mir_to_kill, we need to replace the previous SSA reg assigned to the
// same dalvik reg to keep consistency with subsequent instructions. However, if there's no
- // defining MIR for that dalvik reg, the preserved valus must come from its predecessors
+ // defining MIR for that dalvik reg, the preserved values must come from its predecessors
// and we need to create a new Phi (a degenerate Phi if there's only a single predecessor).
if (def_change == kNPos) {
if (wide) {
@@ -541,7 +541,21 @@
DCHECK_EQ(mir_graph_->SRegToVReg(new_s_reg) + 1, mir_graph_->SRegToVReg(new_s_reg + 1));
CreatePhi(new_s_reg + 1); // High word Phi.
}
- return CreatePhi(new_s_reg);
+ MIR* phi = CreatePhi(new_s_reg);
+ // If this is a degenerate Phi with all inputs being the same SSA reg, we need to its uses.
+ DCHECK_NE(phi->ssa_rep->num_uses, 0u);
+ int old_s_reg = phi->ssa_rep->uses[0];
+ bool all_same = true;
+ for (size_t i = 1u, num = phi->ssa_rep->num_uses; i != num; ++i) {
+ if (phi->ssa_rep->uses[i] != old_s_reg) {
+ all_same = false;
+ break;
+ }
+ }
+ if (all_same) {
+ vreg_chains_.RenameSRegUses(0u, last_change, old_s_reg, new_s_reg, wide);
+ }
+ return phi;
} else {
DCHECK_LT(def_change, last_change);
DCHECK_LE(last_change, vreg_chains_.NumMIRs());
diff --git a/compiler/dex/gvn_dead_code_elimination_test.cc b/compiler/dex/gvn_dead_code_elimination_test.cc
index f9f0882..efb32bb 100644
--- a/compiler/dex/gvn_dead_code_elimination_test.cc
+++ b/compiler/dex/gvn_dead_code_elimination_test.cc
@@ -1629,6 +1629,52 @@
}
TEST_F(GvnDeadCodeEliminationTestDiamond, CreatePhi2) {
+ static const MIRDef mirs[] = {
+ DEF_CONST(3, Instruction::CONST, 0u, 1000),
+ DEF_MOVE(4, Instruction::MOVE, 1u, 0u),
+ DEF_CONST(4, Instruction::CONST, 2u, 1000),
+ };
+
+ static const int32_t sreg_to_vreg_map[] = { 0, 1, 0 };
+ PrepareSRegToVRegMap(sreg_to_vreg_map);
+
+ PrepareMIRs(mirs);
+ PerformGVN_DCE();
+
+ ASSERT_EQ(arraysize(mirs), value_names_.size());
+ EXPECT_EQ(value_names_[0], value_names_[1]);
+ EXPECT_EQ(value_names_[0], value_names_[2]);
+
+ static const bool eliminated[] = {
+ false, false, true,
+ };
+ static_assert(arraysize(eliminated) == arraysize(mirs), "array size mismatch");
+ for (size_t i = 0; i != arraysize(eliminated); ++i) {
+ bool actually_eliminated = (static_cast<int>(mirs_[i].dalvikInsn.opcode) == kMirOpNop);
+ EXPECT_EQ(eliminated[i], actually_eliminated) << i;
+ }
+ // Check that we've created a single-input Phi to replace the CONST 3u.
+ BasicBlock* bb4 = cu_.mir_graph->GetBasicBlock(4);
+ MIR* phi = bb4->first_mir_insn;
+ ASSERT_TRUE(phi != nullptr);
+ ASSERT_EQ(kMirOpPhi, static_cast<int>(phi->dalvikInsn.opcode));
+ ASSERT_EQ(1, phi->ssa_rep->num_uses);
+ EXPECT_EQ(0, phi->ssa_rep->uses[0]);
+ ASSERT_EQ(1, phi->ssa_rep->num_defs);
+ EXPECT_EQ(2, phi->ssa_rep->defs[0]);
+ EXPECT_EQ(0u, phi->dalvikInsn.vA);
+ MIR* move = phi->next;
+ ASSERT_TRUE(move != nullptr);
+ ASSERT_EQ(Instruction::MOVE, move->dalvikInsn.opcode);
+ ASSERT_EQ(1, move->ssa_rep->num_uses);
+ EXPECT_EQ(2, move->ssa_rep->uses[0]);
+ ASSERT_EQ(1, move->ssa_rep->num_defs);
+ EXPECT_EQ(1, move->ssa_rep->defs[0]);
+ EXPECT_EQ(1u, move->dalvikInsn.vA);
+ EXPECT_EQ(0u, move->dalvikInsn.vB);
+}
+
+TEST_F(GvnDeadCodeEliminationTestDiamond, CreatePhi3) {
static const IFieldDef ifields[] = {
{ 0u, 1u, 0u, false, kDexMemAccessWord },
};
