Rewrite kMirOpSelect for all IF_ccZ opcodes.

Also improve special cases for ARM and add tests.

Change-Id: I06f575b9c7b547dbc431dbfadf2b927151fe16b9

6 files changed, 84 insertions, 56 deletions

diff --git a/compiler/dex/mir_graph.h b/compiler/dex/mir_graph.h
index 08bf647c95..1eb9ef9bef 100644
--- a/compiler/dex/mir_graph.h
+++ b/compiler/dex/mir_graph.h
@@ -259,7 +259,7 @@ struct MIR {
     BasicBlockId* phi_incoming;
     // Establish link from check instruction (kMirOpCheck) to the actual throwing instruction.
     MIR* throw_insn;
-    // Fused cmp branch condition.
+    // Branch condition for fused cmp or select.
     ConditionCode ccode;
     // IGET/IPUT lowering info index, points to MIRGraph::ifield_lowering_infos_. Due to limit on
     // the number of code points (64K) and size of IGET/IPUT insn (2), this will never exceed 32K.
diff --git a/compiler/dex/mir_optimization.cc b/compiler/dex/mir_optimization.cc
index 14d03a518c..243452e968 100644
--- a/compiler/dex/mir_optimization.cc
+++ b/compiler/dex/mir_optimization.cc
@@ -195,6 +195,28 @@ static SelectInstructionKind SelectKind(MIR* mir) {
   }
 }
 
+static constexpr ConditionCode kIfCcZConditionCodes[] = {
+    kCondEq, kCondNe, kCondLt, kCondGe, kCondGt, kCondLe
+};
+
+COMPILE_ASSERT(arraysize(kIfCcZConditionCodes) == Instruction::IF_LEZ - Instruction::IF_EQZ + 1,
+               if_ccz_ccodes_size1);
+
+static constexpr bool IsInstructionIfCcZ(Instruction::Code opcode) {
+  return Instruction::IF_EQZ <= opcode && opcode <= Instruction::IF_LEZ;
+}
+
+static constexpr ConditionCode ConditionCodeForIfCcZ(Instruction::Code opcode) {
+  return kIfCcZConditionCodes[opcode - Instruction::IF_EQZ];
+}
+
+COMPILE_ASSERT(ConditionCodeForIfCcZ(Instruction::IF_EQZ) == kCondEq, check_if_eqz_ccode);
+COMPILE_ASSERT(ConditionCodeForIfCcZ(Instruction::IF_NEZ) == kCondNe, check_if_nez_ccode);
+COMPILE_ASSERT(ConditionCodeForIfCcZ(Instruction::IF_LTZ) == kCondLt, check_if_ltz_ccode);
+COMPILE_ASSERT(ConditionCodeForIfCcZ(Instruction::IF_GEZ) == kCondGe, check_if_gez_ccode);
+COMPILE_ASSERT(ConditionCodeForIfCcZ(Instruction::IF_GTZ) == kCondGt, check_if_gtz_ccode);
+COMPILE_ASSERT(ConditionCodeForIfCcZ(Instruction::IF_LEZ) == kCondLe, check_if_lez_ccode);
+
 int MIRGraph::GetSSAUseCount(int s_reg) {
   return raw_use_counts_.Get(s_reg);
 }
@@ -313,35 +335,11 @@ bool MIRGraph::BasicBlockOpt(BasicBlock* bb) {
           }
           if (mir->next != NULL) {
             MIR* mir_next = mir->next;
-            Instruction::Code br_opcode = mir_next->dalvikInsn.opcode;
-            ConditionCode ccode = kCondNv;
-            switch (br_opcode) {
-              case Instruction::IF_EQZ:
-                ccode = kCondEq;
-                break;
-              case Instruction::IF_NEZ:
-                ccode = kCondNe;
-                break;
-              case Instruction::IF_LTZ:
-                ccode = kCondLt;
-                break;
-              case Instruction::IF_GEZ:
-                ccode = kCondGe;
-                break;
-              case Instruction::IF_GTZ:
-                ccode = kCondGt;
-                break;
-              case Instruction::IF_LEZ:
-                ccode = kCondLe;
-                break;
-              default:
-                break;
-            }
             // Make sure result of cmp is used by next insn and nowhere else
-            if ((ccode != kCondNv) &&
+            if (IsInstructionIfCcZ(mir->next->dalvikInsn.opcode) &&
                 (mir->ssa_rep->defs[0] == mir_next->ssa_rep->uses[0]) &&
                 (GetSSAUseCount(mir->ssa_rep->defs[0]) == 1)) {
-              mir_next->meta.ccode = ccode;
+              mir_next->meta.ccode = ConditionCodeForIfCcZ(mir_next->dalvikInsn.opcode);
               switch (opcode) {
                 case Instruction::CMPL_FLOAT:
                   mir_next->dalvikInsn.opcode =
@@ -409,8 +407,7 @@ bool MIRGraph::BasicBlockOpt(BasicBlock* bb) {
       // TUNING: expand to support IF_xx compare & branches
       if (!cu_->compiler_backend->IsPortable() &&
           (cu_->instruction_set == kThumb2 || cu_->instruction_set == kX86) &&
-          ((mir->dalvikInsn.opcode == Instruction::IF_EQZ) ||
-          (mir->dalvikInsn.opcode == Instruction::IF_NEZ))) {
+          IsInstructionIfCcZ(mir->dalvikInsn.opcode)) {
         BasicBlock* ft = GetBasicBlock(bb->fall_through);
         DCHECK(ft != NULL);
         BasicBlock* ft_ft = GetBasicBlock(ft->fall_through);
@@ -457,12 +454,7 @@ bool MIRGraph::BasicBlockOpt(BasicBlock* bb) {
                * NOTE: not updating other dataflow info (no longer used at this point).
                * If this changes, need to update i_dom, etc. here (and in CombineBlocks).
                */
-              if (opcode == Instruction::IF_NEZ) {
-                // Normalize.
-                MIR* tmp_mir = if_true;
-                if_true = if_false;
-                if_false = tmp_mir;
-              }
+              mir->meta.ccode = ConditionCodeForIfCcZ(mir->dalvikInsn.opcode);
               mir->dalvikInsn.opcode = static_cast<Instruction::Code>(kMirOpSelect);
               bool const_form = (SelectKind(if_true) == kSelectConst);
               if ((SelectKind(if_true) == kSelectMove)) {
diff --git a/compiler/dex/quick/arm/int_arm.cc b/compiler/dex/quick/arm/int_arm.cc
index 94c8844dc0..fb2096f1e9 100644
--- a/compiler/dex/quick/arm/int_arm.cc
+++ b/compiler/dex/quick/arm/int_arm.cc
@@ -172,19 +172,33 @@ void ArmMir2Lir::GenSelect(BasicBlock* bb, MIR* mir) {
   RegLocation rl_src = mir_graph_->GetSrc(mir, 0);
   RegLocation rl_dest = mir_graph_->GetDest(mir);
   rl_src = LoadValue(rl_src, kCoreReg);
+  ConditionCode ccode = mir->meta.ccode;
   if (mir->ssa_rep->num_uses == 1) {
     // CONST case
     int true_val = mir->dalvikInsn.vB;
     int false_val = mir->dalvikInsn.vC;
     rl_result = EvalLoc(rl_dest, kCoreReg, true);
-    if ((true_val == 1) && (false_val == 0)) {
+    // Change kCondNe to kCondEq for the special cases below.
+    if (ccode == kCondNe) {
+      ccode = kCondEq;
+      std::swap(true_val, false_val);
+    }
+    bool cheap_false_val = InexpensiveConstantInt(false_val);
+    if (cheap_false_val && ccode == kCondEq && (true_val == 0 || true_val == -1)) {
+      OpRegRegImm(kOpSub, rl_result.reg.GetReg(), rl_src.reg.GetReg(), -true_val);
+      DCHECK(last_lir_insn_->u.m.def_mask & ENCODE_CCODE);
+      OpIT(true_val == 0 ? kCondNe : kCondUge, "");
+      LoadConstant(rl_result.reg.GetReg(), false_val);
+      GenBarrier();  // Add a scheduling barrier to keep the IT shadow intact
+    } else if (cheap_false_val && ccode == kCondEq && true_val == 1) {
       OpRegRegImm(kOpRsub, rl_result.reg.GetReg(), rl_src.reg.GetReg(), 1);
-      OpIT(kCondUlt, "");
-      LoadConstant(rl_result.reg.GetReg(), 0);
+      DCHECK(last_lir_insn_->u.m.def_mask & ENCODE_CCODE);
+      OpIT(kCondLs, "");
+      LoadConstant(rl_result.reg.GetReg(), false_val);
       GenBarrier();  // Add a scheduling barrier to keep the IT shadow intact
-    } else if (InexpensiveConstantInt(true_val) && InexpensiveConstantInt(false_val)) {
+    } else if (cheap_false_val && InexpensiveConstantInt(true_val)) {
       OpRegImm(kOpCmp, rl_src.reg.GetReg(), 0);
-      OpIT(kCondEq, "E");
+      OpIT(ccode, "E");
       LoadConstant(rl_result.reg.GetReg(), true_val);
       LoadConstant(rl_result.reg.GetReg(), false_val);
       GenBarrier();  // Add a scheduling barrier to keep the IT shadow intact
@@ -195,7 +209,7 @@ void ArmMir2Lir::GenSelect(BasicBlock* bb, MIR* mir) {
       LoadConstant(t_reg1, true_val);
       LoadConstant(t_reg2, false_val);
       OpRegImm(kOpCmp, rl_src.reg.GetReg(), 0);
-      OpIT(kCondEq, "E");
+      OpIT(ccode, "E");
       OpRegCopy(rl_result.reg.GetReg(), t_reg1);
       OpRegCopy(rl_result.reg.GetReg(), t_reg2);
       GenBarrier();  // Add a scheduling barrier to keep the IT shadow intact
@@ -209,13 +223,13 @@ void ArmMir2Lir::GenSelect(BasicBlock* bb, MIR* mir) {
     rl_result = EvalLoc(rl_dest, kCoreReg, true);
     OpRegImm(kOpCmp, rl_src.reg.GetReg(), 0);
     if (rl_result.reg.GetReg() == rl_true.reg.GetReg()) {  // Is the "true" case already in place?
-      OpIT(kCondNe, "");
+      OpIT(NegateComparison(ccode), "");
       OpRegCopy(rl_result.reg.GetReg(), rl_false.reg.GetReg());
     } else if (rl_result.reg.GetReg() == rl_false.reg.GetReg()) {  // False case in place?
-      OpIT(kCondEq, "");
+      OpIT(ccode, "");
       OpRegCopy(rl_result.reg.GetReg(), rl_true.reg.GetReg());
     } else {  // Normal - select between the two.
-      OpIT(kCondEq, "E");
+      OpIT(ccode, "E");
       OpRegCopy(rl_result.reg.GetReg(), rl_true.reg.GetReg());
       OpRegCopy(rl_result.reg.GetReg(), rl_false.reg.GetReg());
     }
diff --git a/compiler/dex/quick/codegen_util.cc b/compiler/dex/quick/codegen_util.cc
index eb6f9d1f7e..14469b61c3 100644
--- a/compiler/dex/quick/codegen_util.cc
+++ b/compiler/dex/quick/codegen_util.cc
@@ -967,6 +967,22 @@ ConditionCode Mir2Lir::FlipComparisonOrder(ConditionCode before) {
   return res;
 }
 
+ConditionCode Mir2Lir::NegateComparison(ConditionCode before) {
+  ConditionCode res;
+  switch (before) {
+    case kCondEq: res = kCondNe; break;
+    case kCondNe: res = kCondEq; break;
+    case kCondLt: res = kCondGe; break;
+    case kCondGt: res = kCondLe; break;
+    case kCondLe: res = kCondGt; break;
+    case kCondGe: res = kCondLt; break;
+    default:
+      res = static_cast<ConditionCode>(0);
+      LOG(FATAL) << "Unexpected ccode " << before;
+  }
+  return res;
+}
+
 // TODO: move to mir_to_lir.cc
 Mir2Lir::Mir2Lir(CompilationUnit* cu, MIRGraph* mir_graph, ArenaAllocator* arena)
     : Backend(arena),
diff --git a/compiler/dex/quick/mir_to_lir.h b/compiler/dex/quick/mir_to_lir.h
index 19621b01ab..9e0e29995e 100644
--- a/compiler/dex/quick/mir_to_lir.h
+++ b/compiler/dex/quick/mir_to_lir.h
@@ -425,6 +425,7 @@ class Mir2Lir : public Backend {
     bool EvaluateBranch(Instruction::Code opcode, int src1, int src2);
     bool IsInexpensiveConstant(RegLocation rl_src);
     ConditionCode FlipComparisonOrder(ConditionCode before);
+    ConditionCode NegateComparison(ConditionCode before);
     virtual void InstallLiteralPools();
     void InstallSwitchTables();
     void InstallFillArrayData();
diff --git a/compiler/dex/quick/x86/int_x86.cc b/compiler/dex/quick/x86/int_x86.cc
index 5900990587..d12c05799a 100644
--- a/compiler/dex/quick/x86/int_x86.cc
+++ b/compiler/dex/quick/x86/int_x86.cc
@@ -190,6 +190,7 @@ void X86Mir2Lir::GenSelect(BasicBlock* bb, MIR* mir) {
   RegLocation rl_src = mir_graph_->GetSrc(mir, 0);
   RegLocation rl_dest = mir_graph_->GetDest(mir);
   rl_src = LoadValue(rl_src, kCoreReg);
+  ConditionCode ccode = mir->meta.ccode;
 
   // The kMirOpSelect has two variants, one for constants and one for moves.
   const bool is_constant_case = (mir->ssa_rep->num_uses == 1);
@@ -200,6 +201,8 @@ void X86Mir2Lir::GenSelect(BasicBlock* bb, MIR* mir) {
     rl_result = EvalLoc(rl_dest, kCoreReg, true);
 
     /*
+     * For ccode == kCondEq:
+     *
      * 1) When the true case is zero and result_reg is not same as src_reg:
      *     xor result_reg, result_reg
      *     cmp $0, src_reg
@@ -212,9 +215,9 @@ void X86Mir2Lir::GenSelect(BasicBlock* bb, MIR* mir) {
      *     cmovz result_reg, t1
      * 3) All other cases (we do compare first to set eflags):
      *     cmp $0, src_reg
-     *     mov result_reg, $true_case
-     *     mov t1, $false_case
-     *     cmovnz result_reg, t1
+     *     mov result_reg, $false_case
+     *     mov t1, $true_case
+     *     cmovz result_reg, t1
      */
     const bool result_reg_same_as_src = (rl_src.location == kLocPhysReg && rl_src.reg.GetReg() == rl_result.reg.GetReg());
     const bool true_zero_case = (true_val == 0 && false_val != 0 && !result_reg_same_as_src);
@@ -230,15 +233,15 @@ void X86Mir2Lir::GenSelect(BasicBlock* bb, MIR* mir) {
     }
 
     if (catch_all_case) {
-      OpRegImm(kOpMov, rl_result.reg.GetReg(), true_val);
+      OpRegImm(kOpMov, rl_result.reg.GetReg(), false_val);
     }
 
     if (true_zero_case || false_zero_case || catch_all_case) {
-      int immediateForTemp = false_zero_case ? true_val : false_val;
+      ConditionCode cc = true_zero_case ? NegateComparison(ccode) : ccode;
+      int immediateForTemp = true_zero_case ? false_val : true_val;
       int temp1_reg = AllocTemp();
       OpRegImm(kOpMov, temp1_reg, immediateForTemp);
 
-      ConditionCode cc = false_zero_case ? kCondEq : kCondNe;
       OpCondRegReg(kOpCmov, cc, rl_result.reg.GetReg(), temp1_reg);
 
       FreeTemp(temp1_reg);
@@ -251,6 +254,8 @@ void X86Mir2Lir::GenSelect(BasicBlock* bb, MIR* mir) {
     rl_result = EvalLoc(rl_dest, kCoreReg, true);
 
     /*
+     * For ccode == kCondEq:
+     *
      * 1) When true case is already in place:
      *     cmp $0, src_reg
      *     cmovnz result_reg, false_reg
@@ -259,20 +264,20 @@ void X86Mir2Lir::GenSelect(BasicBlock* bb, MIR* mir) {
      *     cmovz result_reg, true_reg
      * 3) When neither cases are in place:
      *     cmp $0, src_reg
-     *     mov result_reg, true_reg
-     *     cmovnz result_reg, false_reg
+     *     mov result_reg, false_reg
+     *     cmovz result_reg, true_reg
      */
 
     // kMirOpSelect is generated just for conditional cases when comparison is done with zero.
     OpRegImm(kOpCmp, rl_src.reg.GetReg(), 0);
 
     if (rl_result.reg.GetReg() == rl_true.reg.GetReg()) {
-      OpCondRegReg(kOpCmov, kCondNe, rl_result.reg.GetReg(), rl_false.reg.GetReg());
+      OpCondRegReg(kOpCmov, NegateComparison(ccode), rl_result.reg.GetReg(), rl_false.reg.GetReg());
     } else if (rl_result.reg.GetReg() == rl_false.reg.GetReg()) {
-      OpCondRegReg(kOpCmov, kCondEq, rl_result.reg.GetReg(), rl_true.reg.GetReg());
+      OpCondRegReg(kOpCmov, ccode, rl_result.reg.GetReg(), rl_true.reg.GetReg());
     } else {
-      OpRegCopy(rl_result.reg.GetReg(), rl_true.reg.GetReg());
-      OpCondRegReg(kOpCmov, kCondNe, rl_result.reg.GetReg(), rl_false.reg.GetReg());
+      OpRegCopy(rl_result.reg.GetReg(), rl_false.reg.GetReg());
+      OpCondRegReg(kOpCmov, ccode, rl_result.reg.GetReg(), rl_true.reg.GetReg());
     }
   }