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 <serguei.i.katkov@intel.com>
(cherry picked from commit 3f51e7d942c22edaab3a7e703a1e6a2dd6a26f77)
diff --git a/compiler/dex/quick/x86/int_x86.cc b/compiler/dex/quick/x86/int_x86.cc
index fdc46e2..7e9cd80 100755
--- a/compiler/dex/quick/x86/int_x86.cc
+++ b/compiler/dex/quick/x86/int_x86.cc
@@ -274,7 +274,6 @@
// 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 @@
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);
+ // 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) {
+ 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);
diff --git a/test/003-omnibus-opcodes/expected.txt b/test/003-omnibus-opcodes/expected.txt
index a62c498..b591a7a 100644
--- a/test/003-omnibus-opcodes/expected.txt
+++ b/test/003-omnibus-opcodes/expected.txt
@@ -72,4 +72,5 @@
UnresTest2...
UnresTest2 done
InternedString.run
+null
Done!
diff --git a/test/003-omnibus-opcodes/src/GenSelect.java b/test/003-omnibus-opcodes/src/GenSelect.java
new file mode 100644
index 0000000..7e3481b
--- /dev/null
+++ b/test/003-omnibus-opcodes/src/GenSelect.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class GenSelect {
+ public static String test(boolean b) {
+ String str1 = null;
+ String str2 = null;
+ String res = b ? str1 : str2;
+ return res;
+ }
+
+ public static void run() {
+ System.out.println(test(true));
+ }
+}
diff --git a/test/003-omnibus-opcodes/src/Main.java b/test/003-omnibus-opcodes/src/Main.java
index 25050df..a30ec15 100644
--- a/test/003-omnibus-opcodes/src/Main.java
+++ b/test/003-omnibus-opcodes/src/Main.java
@@ -70,6 +70,7 @@
th.printStackTrace();
}
InternedString.run();
+ GenSelect.run();
}
public static void assertTrue(boolean condition) {
