From 3f51e7d942c22edaab3a7e703a1e6a2dd6a26f77 Mon Sep 17 00:00:00 2001 From: Jean Christophe Beyler Date: Thu, 4 Sep 2014 08:34:28 -0700 Subject: ART: Fix x86_64 GenSelect case when destination is Ref Reference in x86_64 is a 64-bit solo register. As a result, the invocation of OpRegImm results in an error when Select opcode of the kind: ref = boolean ? null : null; because opRegImm does not support 64-bit destination for OpMov. The case above is only possible for ref because no one other constant except null is possible. Bug: 17327895 Change-Id: I7541e744ec1c8619711712fd17be72764efcf3a8 Signed-off-by: Serguei Katkov --- compiler/dex/quick/x86/int_x86.cc | 99 +++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 45 deletions(-) (limited to 'compiler') diff --git a/compiler/dex/quick/x86/int_x86.cc b/compiler/dex/quick/x86/int_x86.cc index a745339ca4..ef2d9a6816 100755 --- a/compiler/dex/quick/x86/int_x86.cc +++ b/compiler/dex/quick/x86/int_x86.cc @@ -274,7 +274,6 @@ void X86Mir2Lir::GenSelect(BasicBlock* bb, MIR* mir) { // Avoid using float regs here. RegisterClass src_reg_class = rl_src.ref ? kRefReg : kCoreReg; RegisterClass result_reg_class = rl_dest.ref ? kRefReg : kCoreReg; - rl_src = LoadValue(rl_src, src_reg_class); ConditionCode ccode = mir->meta.ccode; // The kMirOpSelect has two variants, one for constants and one for moves. @@ -283,58 +282,68 @@ void X86Mir2Lir::GenSelect(BasicBlock* bb, MIR* mir) { if (is_constant_case) { int true_val = mir->dalvikInsn.vB; int false_val = mir->dalvikInsn.vC; - rl_result = EvalLoc(rl_dest, result_reg_class, 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 - * mov t1, $false_case - * cmovnz result_reg, t1 - * 2) When the false case is zero and result_reg is not same as src_reg: - * xor result_reg, result_reg - * cmp $0, src_reg - * mov t1, $true_case - * cmovz result_reg, t1 - * 3) All other cases (we do compare first to set eflags): - * cmp $0, src_reg - * mov result_reg, $false_case - * mov t1, $true_case - * cmovz result_reg, t1 - */ - // FIXME: depending on how you use registers you could get a false != mismatch when dealing - // with different views of the same underlying physical resource (i.e. solo32 vs. solo64). - const bool result_reg_same_as_src = - (rl_src.location == kLocPhysReg && rl_src.reg.GetRegNum() == rl_result.reg.GetRegNum()); - const bool true_zero_case = (true_val == 0 && false_val != 0 && !result_reg_same_as_src); - const bool false_zero_case = (false_val == 0 && true_val != 0 && !result_reg_same_as_src); - const bool catch_all_case = !(true_zero_case || false_zero_case); - - if (true_zero_case || false_zero_case) { - OpRegReg(kOpXor, rl_result.reg, rl_result.reg); - } + // simplest strange case + if (true_val == false_val) { + rl_result = EvalLoc(rl_dest, result_reg_class, true); + LoadConstantNoClobber(rl_result.reg, true_val); + } else { + // TODO: use GenSelectConst32 and handle additional opcode patterns such as + // "cmp; setcc; movzx" or "cmp; sbb r0,r0; and r0,$mask; add r0,$literal". + rl_src = LoadValue(rl_src, src_reg_class); + rl_result = EvalLoc(rl_dest, result_reg_class, 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 + * mov t1, $false_case + * cmovnz result_reg, t1 + * 2) When the false case is zero and result_reg is not same as src_reg: + * xor result_reg, result_reg + * cmp $0, src_reg + * mov t1, $true_case + * cmovz result_reg, t1 + * 3) All other cases (we do compare first to set eflags): + * cmp $0, src_reg + * mov result_reg, $false_case + * mov t1, $true_case + * cmovz result_reg, t1 + */ + // FIXME: depending on how you use registers you could get a false != mismatch when dealing + // with different views of the same underlying physical resource (i.e. solo32 vs. solo64). + const bool result_reg_same_as_src = + (rl_src.location == kLocPhysReg && rl_src.reg.GetRegNum() == rl_result.reg.GetRegNum()); + const bool true_zero_case = (true_val == 0 && false_val != 0 && !result_reg_same_as_src); + const bool false_zero_case = (false_val == 0 && true_val != 0 && !result_reg_same_as_src); + const bool catch_all_case = !(true_zero_case || false_zero_case); + + if (true_zero_case || false_zero_case) { + OpRegReg(kOpXor, rl_result.reg, rl_result.reg); + } - if (true_zero_case || false_zero_case || catch_all_case) { - OpRegImm(kOpCmp, rl_src.reg, 0); - } + if (true_zero_case || false_zero_case || catch_all_case) { + OpRegImm(kOpCmp, rl_src.reg, 0); + } - if (catch_all_case) { - OpRegImm(kOpMov, rl_result.reg, false_val); - } + if (catch_all_case) { + OpRegImm(kOpMov, rl_result.reg, false_val); + } - if (true_zero_case || false_zero_case || catch_all_case) { - ConditionCode cc = true_zero_case ? NegateComparison(ccode) : ccode; - int immediateForTemp = true_zero_case ? false_val : true_val; - RegStorage temp1_reg = AllocTypedTemp(false, result_reg_class); - OpRegImm(kOpMov, temp1_reg, immediateForTemp); + if (true_zero_case || false_zero_case || catch_all_case) { + ConditionCode cc = true_zero_case ? NegateComparison(ccode) : ccode; + int immediateForTemp = true_zero_case ? false_val : true_val; + RegStorage temp1_reg = AllocTypedTemp(false, result_reg_class); + OpRegImm(kOpMov, temp1_reg, immediateForTemp); - OpCondRegReg(kOpCmov, cc, rl_result.reg, temp1_reg); + OpCondRegReg(kOpCmov, cc, rl_result.reg, temp1_reg); - FreeTemp(temp1_reg); + FreeTemp(temp1_reg); + } } } else { + rl_src = LoadValue(rl_src, src_reg_class); RegLocation rl_true = mir_graph_->GetSrc(mir, 1); RegLocation rl_false = mir_graph_->GetSrc(mir, 2); rl_true = LoadValue(rl_true, result_reg_class); -- cgit v1.2.3-59-g8ed1b