Merge "Quick: Fix optimizations for empty if blocks."
diff --git a/compiler/dex/global_value_numbering.cc b/compiler/dex/global_value_numbering.cc
index e2b9987..94ba4fa 100644
--- a/compiler/dex/global_value_numbering.cc
+++ b/compiler/dex/global_value_numbering.cc
@@ -160,20 +160,10 @@
return location;
}
-bool GlobalValueNumbering::HasNullCheckLastInsn(const BasicBlock* pred_bb,
- BasicBlockId succ_id) {
- if (pred_bb->block_type != kDalvikByteCode || pred_bb->last_mir_insn == nullptr) {
- return false;
- }
- Instruction::Code last_opcode = pred_bb->last_mir_insn->dalvikInsn.opcode;
- return ((last_opcode == Instruction::IF_EQZ && pred_bb->fall_through == succ_id) ||
- (last_opcode == Instruction::IF_NEZ && pred_bb->taken == succ_id));
-}
-
bool GlobalValueNumbering::NullCheckedInAllPredecessors(
const ScopedArenaVector<uint16_t>& merge_names) const {
// Implicit parameters:
- // - *work_lvn: the LVN for which we're checking predecessors.
+ // - *work_lvn_: the LVN for which we're checking predecessors.
// - merge_lvns_: the predecessor LVNs.
DCHECK_EQ(merge_lvns_.size(), merge_names.size());
for (size_t i = 0, size = merge_lvns_.size(); i != size; ++i) {
@@ -198,7 +188,7 @@
bool GlobalValueNumbering::DivZeroCheckedInAllPredecessors(
const ScopedArenaVector<uint16_t>& merge_names) const {
// Implicit parameters:
- // - *work_lvn: the LVN for which we're checking predecessors.
+ // - *work_lvn_: the LVN for which we're checking predecessors.
// - merge_lvns_: the predecessor LVNs.
DCHECK_EQ(merge_lvns_.size(), merge_names.size());
for (size_t i = 0, size = merge_lvns_.size(); i != size; ++i) {
@@ -217,15 +207,11 @@
if (bb->predecessors.size() == 1u) {
BasicBlockId pred_id = bb->predecessors[0];
BasicBlock* pred_bb = mir_graph_->GetBasicBlock(pred_id);
- if (pred_bb->last_mir_insn != nullptr) {
- Instruction::Code opcode = pred_bb->last_mir_insn->dalvikInsn.opcode;
- if ((opcode == Instruction::IF_NEZ && pred_bb->taken == bb_id) ||
- (opcode == Instruction::IF_EQZ && pred_bb->fall_through == bb_id)) {
- DCHECK(lvns_[pred_id] != nullptr);
- uint16_t operand = lvns_[pred_id]->GetSregValue(pred_bb->last_mir_insn->ssa_rep->uses[0]);
- if (operand == cond) {
- return true;
- }
+ if (pred_bb->BranchesToSuccessorOnlyIfNotZero(bb_id)) {
+ DCHECK(lvns_[pred_id] != nullptr);
+ uint16_t operand = lvns_[pred_id]->GetSregValue(pred_bb->last_mir_insn->ssa_rep->uses[0]);
+ if (operand == cond) {
+ return true;
}
}
}
diff --git a/compiler/dex/global_value_numbering.h b/compiler/dex/global_value_numbering.h
index bd2f187..c514f75 100644
--- a/compiler/dex/global_value_numbering.h
+++ b/compiler/dex/global_value_numbering.h
@@ -194,7 +194,9 @@
return mir_graph_->GetBasicBlock(bb_id);
}
- static bool HasNullCheckLastInsn(const BasicBlock* pred_bb, BasicBlockId succ_id);
+ static bool HasNullCheckLastInsn(const BasicBlock* pred_bb, BasicBlockId succ_id) {
+ return pred_bb->BranchesToSuccessorOnlyIfNotZero(succ_id);
+ }
bool NullCheckedInAllPredecessors(const ScopedArenaVector<uint16_t>& merge_names) const;
diff --git a/compiler/dex/mir_graph.h b/compiler/dex/mir_graph.h
index f038397..dbe9062 100644
--- a/compiler/dex/mir_graph.h
+++ b/compiler/dex/mir_graph.h
@@ -452,6 +452,21 @@
MIR* GetFirstNonPhiInsn();
/**
+ * @brief Checks whether the block ends with if-nez or if-eqz that branches to
+ * the given successor only if the register in not zero.
+ */
+ bool BranchesToSuccessorOnlyIfNotZero(BasicBlockId succ_id) const {
+ if (last_mir_insn == nullptr) {
+ return false;
+ }
+ Instruction::Code last_opcode = last_mir_insn->dalvikInsn.opcode;
+ return ((last_opcode == Instruction::IF_EQZ && fall_through == succ_id) ||
+ (last_opcode == Instruction::IF_NEZ && taken == succ_id)) &&
+ // Make sure the other successor isn't the same (empty if), b/21614284.
+ (fall_through != taken);
+ }
+
+ /**
* @brief Used to obtain the next MIR that follows unconditionally.
* @details The implementation does not guarantee that a MIR does not
* follow even if this method returns nullptr.
diff --git a/compiler/dex/mir_optimization.cc b/compiler/dex/mir_optimization.cc
index 645511e..727d0fd 100644
--- a/compiler/dex/mir_optimization.cc
+++ b/compiler/dex/mir_optimization.cc
@@ -978,18 +978,12 @@
BasicBlock* pred_bb = GetBasicBlock(pred_id);
DCHECK(pred_bb != nullptr);
MIR* null_check_insn = nullptr;
- if (pred_bb->block_type == kDalvikByteCode) {
- // Check to see if predecessor had an explicit null-check.
- MIR* last_insn = pred_bb->last_mir_insn;
- if (last_insn != nullptr) {
- Instruction::Code last_opcode = last_insn->dalvikInsn.opcode;
- if ((last_opcode == Instruction::IF_EQZ && pred_bb->fall_through == bb->id) ||
- (last_opcode == Instruction::IF_NEZ && pred_bb->taken == bb->id)) {
- // Remember the null check insn if there's no other predecessor requiring null check.
- if (!copied_first || !vregs_to_check->IsBitSet(last_insn->dalvikInsn.vA)) {
- null_check_insn = last_insn;
- }
- }
+ // Check to see if predecessor had an explicit null-check.
+ if (pred_bb->BranchesToSuccessorOnlyIfNotZero(bb->id)) {
+ // Remember the null check insn if there's no other predecessor requiring null check.
+ if (!copied_first || !vregs_to_check->IsBitSet(pred_bb->last_mir_insn->dalvikInsn.vA)) {
+ null_check_insn = pred_bb->last_mir_insn;
+ DCHECK(null_check_insn != nullptr);
}
}
if (!copied_first) {
diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 7be78b7..6c00c82 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -19,4 +19,5 @@
b/17410612
b/21863767
b/21873167
+b/21614284
Done!
diff --git a/test/800-smali/smali/b_21614284.smali b/test/800-smali/smali/b_21614284.smali
new file mode 100644
index 0000000..3cb1bd0
--- /dev/null
+++ b/test/800-smali/smali/b_21614284.smali
@@ -0,0 +1,22 @@
+.class public LB21614284;
+.super Ljava/lang/Object;
+
+.field private a:I
+
+.method public constructor <init>()V
+ .registers 2
+ invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+ const v0, 42
+ iput v0, p0, LB21614284;->a:I
+ return-void
+.end method
+
+.method public static test(LB21614284;)I
+ .registers 2
+ # Empty if, testing p0.
+ if-nez p0, :label
+ :label
+ # p0 still needs a null check.
+ iget v0, p0, LB21614284;->a:I
+ return v0
+.end method
diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 2180186..ab4457e 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -86,6 +86,8 @@
testCases.add(new TestCase("b/21863767", "B21863767", "run", null, null,
null));
testCases.add(new TestCase("b/21873167", "B21873167", "test", null, null, null));
+ testCases.add(new TestCase("b/21614284", "B21614284", "test", new Object[] { null },
+ new NullPointerException(), null));
}
public void runTests() {
