Eliminate suspend checks on back-edges to return insn.

This optimization seems to have been broken for a long time.

Change-Id: I62ec85c71bb5253917ad9465a952911e917f6b52

5 files changed, 37 insertions, 28 deletions

diff --git a/compiler/dex/global_value_numbering.cc b/compiler/dex/global_value_numbering.cc
index dbe98506b7..531ed6045a 100644
--- a/compiler/dex/global_value_numbering.cc
+++ b/compiler/dex/global_value_numbering.cc
@@ -108,11 +108,7 @@ LocalValueNumbering* GlobalValueNumbering::PrepareBasicBlock(BasicBlock* bb,
          bb->last_mir_insn->dalvikInsn.opcode == Instruction::RETURN ||
          bb->last_mir_insn->dalvikInsn.opcode == Instruction::RETURN_OBJECT ||
          bb->last_mir_insn->dalvikInsn.opcode == Instruction::RETURN_WIDE) &&
-        (bb->first_mir_insn == bb->last_mir_insn ||
-         (static_cast<int>(bb->first_mir_insn->dalvikInsn.opcode) == kMirOpPhi &&
-          (bb->first_mir_insn->next == bb->last_mir_insn ||
-           (static_cast<int>(bb->first_mir_insn->next->dalvikInsn.opcode) == kMirOpPhi &&
-            bb->first_mir_insn->next->next == bb->last_mir_insn))))) {
+        bb->GetFirstNonPhiInsn() == bb->last_mir_insn) {
       merge_type = LocalValueNumbering::kReturnMerge;
     }
     // At least one predecessor must have been processed before this bb.
diff --git a/compiler/dex/mir_graph.cc b/compiler/dex/mir_graph.cc
index 8190f95275..3d85a4b86b 100644
--- a/compiler/dex/mir_graph.cc
+++ b/compiler/dex/mir_graph.cc
@@ -1167,6 +1167,14 @@ bool BasicBlock::RemoveMIRList(MIR* first_list_mir, MIR* last_list_mir) {
   return true;
 }
 
+MIR* BasicBlock::GetFirstNonPhiInsn() {
+  MIR* mir = first_mir_insn;
+  while (mir != nullptr && static_cast<int>(mir->dalvikInsn.opcode) == kMirOpPhi) {
+    mir = mir->next;
+  }
+  return mir;
+}
+
 MIR* BasicBlock::GetNextUnconditionalMir(MIRGraph* mir_graph, MIR* current) {
   MIR* next_mir = nullptr;
 
diff --git a/compiler/dex/mir_graph.h b/compiler/dex/mir_graph.h
index 63b1f2d95f..c8ea972d9d 100644
--- a/compiler/dex/mir_graph.h
+++ b/compiler/dex/mir_graph.h
@@ -444,6 +444,11 @@ class BasicBlock : public DeletableArenaObject<kArenaAllocBB> {
   void UpdatePredecessor(BasicBlockId old_pred, BasicBlockId new_pred);
 
   /**
+   * @brief Return first non-Phi insn.
+   */
+  MIR* GetFirstNonPhiInsn();
+
+  /**
    * @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 9d52807725..1f630f78bb 100644
--- a/compiler/dex/mir_optimization.cc
+++ b/compiler/dex/mir_optimization.cc
@@ -477,29 +477,25 @@ bool MIRGraph::BasicBlockOpt(BasicBlock* bb) {
             }
           }
           break;
-        case Instruction::GOTO:
-        case Instruction::GOTO_16:
-        case Instruction::GOTO_32:
-        case Instruction::IF_EQ:
-        case Instruction::IF_NE:
-        case Instruction::IF_LT:
-        case Instruction::IF_GE:
-        case Instruction::IF_GT:
-        case Instruction::IF_LE:
-        case Instruction::IF_EQZ:
-        case Instruction::IF_NEZ:
-        case Instruction::IF_LTZ:
-        case Instruction::IF_GEZ:
-        case Instruction::IF_GTZ:
-        case Instruction::IF_LEZ:
-          // If we've got a backwards branch to return, no need to suspend check.
-          if ((IsBackedge(bb, bb->taken) && GetBasicBlock(bb->taken)->dominates_return) ||
-              (IsBackedge(bb, bb->fall_through) &&
-                          GetBasicBlock(bb->fall_through)->dominates_return)) {
-            mir->optimization_flags |= MIR_IGNORE_SUSPEND_CHECK;
-            if (cu_->verbose) {
-              LOG(INFO) << "Suppressed suspend check on branch to return at 0x" << std::hex
-                        << mir->offset;
+        case Instruction::RETURN_VOID:
+        case Instruction::RETURN:
+        case Instruction::RETURN_WIDE:
+        case Instruction::RETURN_OBJECT:
+          if (bb->GetFirstNonPhiInsn() == mir) {
+            // This is a simple return BB. Eliminate suspend checks on predecessor back-edges.
+            for (BasicBlockId pred_id : bb->predecessors) {
+              BasicBlock* pred_bb = GetBasicBlock(pred_id);
+              DCHECK(pred_bb != nullptr);
+              if (IsBackedge(pred_bb, bb->id) && pred_bb->last_mir_insn != nullptr &&
+                  (IsInstructionIfCc(pred_bb->last_mir_insn->dalvikInsn.opcode) ||
+                   IsInstructionIfCcZ(pred_bb->last_mir_insn->dalvikInsn.opcode) ||
+                   IsInstructionGoto(pred_bb->last_mir_insn->dalvikInsn.opcode))) {
+                pred_bb->last_mir_insn->optimization_flags |= MIR_IGNORE_SUSPEND_CHECK;
+                if (cu_->verbose) {
+                  LOG(INFO) << "Suppressed suspend check on branch to return at 0x" << std::hex
+                            << pred_bb->last_mir_insn->offset;
+                }
+              }
             }
           }
           break;
diff --git a/compiler/utils/dex_instruction_utils.h b/compiler/utils/dex_instruction_utils.h
index 09d9419c0b..ad7d750dea 100644
--- a/compiler/utils/dex_instruction_utils.h
+++ b/compiler/utils/dex_instruction_utils.h
@@ -58,6 +58,10 @@ constexpr bool IsInstructionInvokeStatic(Instruction::Code opcode) {
   return opcode == Instruction::INVOKE_STATIC || opcode == Instruction::INVOKE_STATIC_RANGE;
 }
 
+constexpr bool IsInstructionGoto(Instruction::Code opcode) {
+  return Instruction::GOTO <= opcode && opcode <= Instruction::GOTO_32;
+}
+
 constexpr bool IsInstructionIfCc(Instruction::Code opcode) {
   return Instruction::IF_EQ <= opcode && opcode <= Instruction::IF_LE;
 }