diff options
| -rw-r--r-- | compiler/dex/mir_graph.h | 2 | ||||
| -rw-r--r-- | compiler/dex/mir_optimization.cc | 60 | ||||
| -rw-r--r-- | compiler/dex/quick/arm/int_arm.cc | 32 | ||||
| -rw-r--r-- | compiler/dex/quick/codegen_util.cc | 16 | ||||
| -rw-r--r-- | compiler/dex/quick/mir_to_lir.h | 1 | ||||
| -rw-r--r-- | compiler/dex/quick/x86/int_x86.cc | 29 | ||||
| -rw-r--r-- | test/083-compiler-regressions/expected.txt | 1 | ||||
| -rw-r--r-- | test/083-compiler-regressions/src/Main.java | 112 |
8 files changed, 197 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()); } } diff --git a/test/083-compiler-regressions/expected.txt b/test/083-compiler-regressions/expected.txt index 541f0f8dec..ddd11f3300 100644 --- a/test/083-compiler-regressions/expected.txt +++ b/test/083-compiler-regressions/expected.txt @@ -14,3 +14,4 @@ LVNTests.testNPE1 passes LVNTests.testNPE2 passes longDivTest passes longModTest passes +testIfCcz passes diff --git a/test/083-compiler-regressions/src/Main.java b/test/083-compiler-regressions/src/Main.java index 81f5b14af0..3b4d5867ae 100644 --- a/test/083-compiler-regressions/src/Main.java +++ b/test/083-compiler-regressions/src/Main.java @@ -45,6 +45,7 @@ public class Main { LVNTests.testNPE2(); ZeroTests.longDivTest(); ZeroTests.longModTest(); + MirOpSelectTests.testIfCcz(); } public static void returnConstantTest() { @@ -8505,3 +8506,114 @@ class LVNTests { } } } + +class MirOpSelectTests { + private static int ifEqzThen0Else1(int i) { return (i == 0) ? 0 : 1; } + private static int ifEqzThen0Else8(int i) { return (i == 0) ? 0 : 8; } + private static int ifEqzThen1Else5(int i) { return (i == 0) ? 1 : 5; } + private static int ifEqzThenMinus1Else3(int i) { return (i == 0) ? -1 : 3; } + private static int ifEqzThen11Else23(int i) { return (i == 0) ? 11 : 23; } + private static int ifEqzThen54321Else87654321(int i) { return (i == 0) ? 54321 : 87654321; } + private static int ifNezThen0Else1(int i) { return (i != 0) ? 0 : 1; } + private static int ifNezThen0Else8(int i) { return (i != 0) ? 0 : 8; } + private static int ifNezThen1Else5(int i) { return (i != 0) ? 1 : 5; } + private static int ifNezThenMinus1Else3(int i) { return (i != 0) ? -1 : 3; } + private static int ifNezThen11Else23(int i) { return (i != 0) ? 11 : 23; } + private static int ifNezThen54321Else87654321(int i) { return (i != 0) ? 54321 : 87654321; } + private static int ifLtzThen3Else5(int i) { return (i < 0) ? 3 : 5; } + private static int ifGezThen7Else4(int i) { return (i >= 0) ? 7 : 4; } + private static int ifGtzThen2Else9(int i) { return (i > 0) ? 2 : 9; } + private static int ifLezThen8Else0(int i) { return (i <= 0) ? 8 : 0; } + + private static int ifEqz(int src, int thn, int els) { return (src == 0) ? thn : els; } + private static int ifNez(int src, int thn, int els) { return (src != 0) ? thn : els; } + private static int ifLtz(int src, int thn, int els) { return (src < 0) ? thn : els; } + private static int ifGez(int src, int thn, int els) { return (src >= 0) ? thn : els; } + private static int ifGtz(int src, int thn, int els) { return (src > 0) ? thn : els; } + private static int ifLez(int src, int thn, int els) { return (src <= 0) ? thn : els; } + + public static void testIfCcz() { + int[] results = new int[] { + ifEqzThen0Else1(-1), 1, + ifEqzThen0Else1(0), 0, + ifEqzThen0Else1(1), 1, + ifEqzThen0Else8(-1), 8, + ifEqzThen0Else8(0), 0, + ifEqzThen0Else8(1), 8, + ifEqzThen1Else5(-1), 5, + ifEqzThen1Else5(0), 1, + ifEqzThen1Else5(1), 5, + ifEqzThenMinus1Else3(-1), 3, + ifEqzThenMinus1Else3(0), -1, + ifEqzThenMinus1Else3(1), 3, + ifEqzThen11Else23(-1), 23, + ifEqzThen11Else23(0), 11, + ifEqzThen11Else23(1), 23, + ifEqzThen54321Else87654321(-1), 87654321, + ifEqzThen54321Else87654321(0), 54321, + ifEqzThen54321Else87654321(1), 87654321, + ifNezThen0Else1(-1), 0, + ifNezThen0Else1(0), 1, + ifNezThen0Else1(1), 0, + ifNezThen0Else8(-1), 0, + ifNezThen0Else8(0), 8, + ifNezThen0Else8(1), 0, + ifNezThen1Else5(-1), 1, + ifNezThen1Else5(0), 5, + ifNezThen1Else5(1), 1, + ifNezThenMinus1Else3(-1), -1, + ifNezThenMinus1Else3(0), 3, + ifNezThenMinus1Else3(1), -1, + ifNezThen11Else23(-1), 11, + ifNezThen11Else23(0), 23, + ifNezThen11Else23(1), 11, + ifNezThen54321Else87654321(-1), 54321, + ifNezThen54321Else87654321(0), 87654321, + ifNezThen54321Else87654321(1), 54321, + ifLtzThen3Else5(-1), 3, + ifLtzThen3Else5(0), 5, + ifLtzThen3Else5(1), 5, + ifGezThen7Else4(-1), 4, + ifGezThen7Else4(0), 7, + ifGezThen7Else4(1), 7, + ifGtzThen2Else9(-1), 9, + ifGtzThen2Else9(0), 9, + ifGtzThen2Else9(1), 2, + ifLezThen8Else0(-1), 8, + ifLezThen8Else0(0), 8, + ifLezThen8Else0(1), 0, + ifEqz(-1, 101, 201), 201, + ifEqz(0, 102, 202), 102, + ifEqz(1, 103, 203), 203, + ifNez(-1, 104, 204), 104, + ifNez(0, 105, 205), 205, + ifNez(1, 106, 206), 106, + ifLtz(-1, 107, 207), 107, + ifLtz(0, 108, 208), 208, + ifLtz(1, 109, 209), 209, + ifGez(-1, 110, 210), 210, + ifGez(0, 111, 211), 111, + ifGez(1, 112, 212), 112, + ifGtz(-1, 113, 213), 213, + ifGtz(0, 114, 214), 214, + ifGtz(1, 115, 215), 115, + ifLez(-1, 116, 216), 116, + ifLez(0, 117, 217), 117, + ifLez(1, 118, 218), 218, + }; + + boolean success = true; + StringBuilder fails = new StringBuilder(); + for (int i = 0; i != results.length; i += 2) { + if (results[i] != results[i + 1]) { + success = false; + fails.append("\n #" + (i / 2) + ": " + results[i] + " != " + results[i + 1]); + } + } + if (success) { + System.out.println("testIfCcz passes"); + } else { + System.out.println("testIfCcz fails for" + fails.toString()); + } + } +} |