Quick: Fix and enable DCE and improve GVN/DCE cleanup.
When eliminating a move by renaming its source register,
check that it doesn't conflict with vreg usage by insns
between the defining insn and the move.
Improve the GVN/DCE cleanup so that it can handle cases
where GVN or DCE is individually disabled in the pass driver
but not in the disable_opt flags.
Bug: 19419671
Change-Id: I49bb67b81509f51fbaf90c6016c509962be43736
diff --git a/compiler/dex/bb_optimizations.h b/compiler/dex/bb_optimizations.h
index 0850f42..eb87c29 100644
--- a/compiler/dex/bb_optimizations.h
+++ b/compiler/dex/bb_optimizations.h
@@ -270,7 +270,25 @@
CompilationUnit* c_unit = down_cast<PassMEDataHolder*>(data)->c_unit;
DCHECK(c_unit != nullptr);
c_unit->mir_graph->EliminateDeadCodeEnd();
- down_cast<PassMEDataHolder*>(data)->dirty = !c_unit->mir_graph->MirSsaRepUpToDate();
+ }
+};
+
+/**
+ * @class GlobalValueNumberingCleanupPass
+ * @brief Performs the cleanup after global value numbering pass and the dependent
+ * dead code elimination pass that needs the GVN data.
+ */
+class GlobalValueNumberingCleanupPass : public PassME {
+ public:
+ GlobalValueNumberingCleanupPass()
+ : PassME("GVNCleanup", kNoNodes, "") {
+ }
+
+ void Start(PassDataHolder* data) const OVERRIDE {
+ DCHECK(data != nullptr);
+ CompilationUnit* c_unit = down_cast<const PassMEDataHolder*>(data)->c_unit;
+ DCHECK(c_unit != nullptr);
+ return c_unit->mir_graph->GlobalValueNumberingCleanup();
}
};
diff --git a/compiler/dex/gvn_dead_code_elimination.cc b/compiler/dex/gvn_dead_code_elimination.cc
index d7f36f7..bd7bd71 100644
--- a/compiler/dex/gvn_dead_code_elimination.cc
+++ b/compiler/dex/gvn_dead_code_elimination.cc
@@ -347,6 +347,21 @@
return false;
}
+bool GvnDeadCodeElimination::VRegChains::IsVRegUsed(uint16_t first_change, uint16_t last_change,
+ int v_reg, MIRGraph* mir_graph) const {
+ DCHECK_LE(first_change, last_change);
+ DCHECK_LE(last_change, mir_data_.size());
+ for (size_t c = first_change; c != last_change; ++c) {
+ SSARepresentation* ssa_rep = mir_data_[c].mir->ssa_rep;
+ for (int i = 0; i != ssa_rep->num_uses; ++i) {
+ if (mir_graph->SRegToVReg(ssa_rep->uses[i]) == v_reg) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
void GvnDeadCodeElimination::VRegChains::RenameSRegUses(uint16_t first_change, uint16_t last_change,
int old_s_reg, int new_s_reg, bool wide) {
for (size_t c = first_change; c != last_change; ++c) {
@@ -672,8 +687,14 @@
uint16_t src_name =
(d->wide_def ? lvn_->GetSregValueWide(src_s_reg) : lvn_->GetSregValue(src_s_reg));
if (value_name == src_name) {
- RecordPassKillMoveByRenamingSrcDef(check_change, c);
- return;
+ // Check if the move's destination vreg is unused between check_change and the move.
+ uint32_t new_dest_v_reg = mir_graph_->SRegToVReg(d->mir->ssa_rep->defs[0]);
+ if (!vreg_chains_.IsVRegUsed(check_change + 1u, c, new_dest_v_reg, mir_graph_) &&
+ (!d->wide_def ||
+ !vreg_chains_.IsVRegUsed(check_change + 1u, c, new_dest_v_reg + 1, mir_graph_))) {
+ RecordPassKillMoveByRenamingSrcDef(check_change, c);
+ return;
+ }
}
}
}
diff --git a/compiler/dex/gvn_dead_code_elimination.h b/compiler/dex/gvn_dead_code_elimination.h
index f2378f2..bc75a01 100644
--- a/compiler/dex/gvn_dead_code_elimination.h
+++ b/compiler/dex/gvn_dead_code_elimination.h
@@ -111,6 +111,8 @@
void RemoveChange(uint16_t change);
bool IsTopChange(uint16_t change) const;
bool IsSRegUsed(uint16_t first_change, uint16_t last_change, int s_reg) const;
+ bool IsVRegUsed(uint16_t first_change, uint16_t last_change, int v_reg,
+ MIRGraph* mir_graph) const;
void RenameSRegUses(uint16_t first_change, uint16_t last_change,
int old_s_reg, int new_s_reg, bool wide);
void RenameVRegUses(uint16_t first_change, uint16_t last_change,
diff --git a/compiler/dex/gvn_dead_code_elimination_test.cc b/compiler/dex/gvn_dead_code_elimination_test.cc
index 4d2b8b3..3eb372c 100644
--- a/compiler/dex/gvn_dead_code_elimination_test.cc
+++ b/compiler/dex/gvn_dead_code_elimination_test.cc
@@ -1030,6 +1030,40 @@
}
}
+TEST_F(GvnDeadCodeEliminationTestSimple, NoRename4) {
+ static const MIRDef mirs[] = {
+ DEF_CONST(3, Instruction::CONST, 0u, 1000u),
+ DEF_UNIQUE_REF(3, Instruction::NEW_INSTANCE, 1u),
+ DEF_CONST(3, Instruction::CONST, 2u, 100u),
+ DEF_CONST(3, Instruction::CONST, 3u, 200u),
+ DEF_BINOP(3, Instruction::OR_INT_2ADDR, 4u, 2u, 3u), // 3. Find definition of the move src.
+ DEF_MOVE(3, Instruction::MOVE, 5u, 0u), // 4. Uses move dest vreg.
+ DEF_MOVE(3, Instruction::MOVE, 6u, 4u), // 2. Find overwritten move src.
+ DEF_CONST(3, Instruction::CONST, 7u, 2000u), // 1. Overwrites 4u, look for moves.
+ };
+
+ static const int32_t sreg_to_vreg_map[] = { 0, 1, 2, 3, 2, 4, 0, 2 };
+ PrepareSRegToVRegMap(sreg_to_vreg_map);
+
+ PrepareMIRs(mirs);
+ PerformGVN_DCE();
+
+ ASSERT_EQ(arraysize(mirs), value_names_.size());
+ static const size_t diff_indexes[] = { 0, 1, 2, 3, 4, 7 };
+ ExpectValueNamesNE(diff_indexes);
+ EXPECT_EQ(value_names_[0], value_names_[5]);
+ EXPECT_EQ(value_names_[4], value_names_[6]);
+
+ static const bool eliminated[] = {
+ false, false, false, false, false, false, false, false
+ };
+ 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;
+ }
+}
+
TEST_F(GvnDeadCodeEliminationTestSimple, Simple1) {
static const IFieldDef ifields[] = {
{ 0u, 1u, 0u, false, kDexMemAccessObject },
diff --git a/compiler/dex/mir_graph.h b/compiler/dex/mir_graph.h
index 0db54bf..7f9698b 100644
--- a/compiler/dex/mir_graph.h
+++ b/compiler/dex/mir_graph.h
@@ -1101,6 +1101,7 @@
bool EliminateDeadCodeGate();
bool EliminateDeadCode(BasicBlock* bb);
void EliminateDeadCodeEnd();
+ void GlobalValueNumberingCleanup();
bool EliminateSuspendChecksGate();
bool EliminateSuspendChecks(BasicBlock* bb);
diff --git a/compiler/dex/mir_optimization.cc b/compiler/dex/mir_optimization.cc
index 467c14e..0d5da32 100644
--- a/compiler/dex/mir_optimization.cc
+++ b/compiler/dex/mir_optimization.cc
@@ -1409,14 +1409,10 @@
LOG(WARNING) << "GVN failed for " << PrettyMethod(cu_->method_idx, *cu_->dex_file);
cu_->disable_opt |= (1u << kGvnDeadCodeElimination);
}
-
- if ((cu_->disable_opt & (1 << kGvnDeadCodeElimination)) != 0) {
- EliminateDeadCodeEnd();
- } // else preserve GVN data for CSE.
}
bool MIRGraph::EliminateDeadCodeGate() {
- if ((cu_->disable_opt & (1 << kGvnDeadCodeElimination)) != 0) {
+ if ((cu_->disable_opt & (1 << kGvnDeadCodeElimination)) != 0 || temp_.gvn.gvn == nullptr) {
return false;
}
DCHECK(temp_scoped_alloc_ != nullptr);
@@ -1437,11 +1433,21 @@
}
void MIRGraph::EliminateDeadCodeEnd() {
- DCHECK_EQ(temp_.gvn.dce != nullptr, (cu_->disable_opt & (1 << kGvnDeadCodeElimination)) == 0);
- if (temp_.gvn.dce != nullptr) {
- delete temp_.gvn.dce;
- temp_.gvn.dce = nullptr;
+ if (kIsDebugBuild) {
+ // DCE can make some previously dead vregs alive again. Make sure the obsolete
+ // live-in information is not used anymore.
+ AllNodesIterator iter(this);
+ for (BasicBlock* bb = iter.Next(); bb != nullptr; bb = iter.Next()) {
+ if (bb->data_flow_info != nullptr) {
+ bb->data_flow_info->live_in_v = nullptr;
+ }
+ }
}
+}
+
+void MIRGraph::GlobalValueNumberingCleanup() {
+ delete temp_.gvn.dce;
+ temp_.gvn.dce = nullptr;
delete temp_.gvn.gvn;
temp_.gvn.gvn = nullptr;
temp_.gvn.ifield_ids = nullptr;
diff --git a/compiler/dex/pass_driver_me_opts.cc b/compiler/dex/pass_driver_me_opts.cc
index 2e871da..3e193b4 100644
--- a/compiler/dex/pass_driver_me_opts.cc
+++ b/compiler/dex/pass_driver_me_opts.cc
@@ -46,6 +46,7 @@
pass_manager->AddPass(new CodeLayout);
pass_manager->AddPass(new GlobalValueNumberingPass);
pass_manager->AddPass(new DeadCodeEliminationPass);
+ pass_manager->AddPass(new GlobalValueNumberingCleanupPass);
pass_manager->AddPass(new ConstantPropagation);
pass_manager->AddPass(new MethodUseCount);
pass_manager->AddPass(new BBOptimizations);
diff --git a/compiler/dex/quick/quick_compiler.cc b/compiler/dex/quick/quick_compiler.cc
index 39eb117..73cfe92 100644
--- a/compiler/dex/quick/quick_compiler.cc
+++ b/compiler/dex/quick/quick_compiler.cc
@@ -575,7 +575,7 @@
// (1 << kNullCheckElimination) |
// (1 << kClassInitCheckElimination) |
// (1 << kGlobalValueNumbering) |
- (1 << kGvnDeadCodeElimination) |
+ // (1 << kGvnDeadCodeElimination) |
// (1 << kLocalValueNumbering) |
// (1 << kPromoteRegs) |
// (1 << kTrackLiveTemps) |