Don't change Add/Xor into Ror for constant 0 am: 148894a451
Original change: https://android-review.googlesource.com/c/platform/art/+/2973517
Change-Id: Ica167030d5cad693afc9540e86ec1fc45b2a2837
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index d39e9ad..ae778b4 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -621,8 +621,7 @@
size_t rdist = Int64FromConstant(ushr->GetRight()->AsConstant());
size_t ldist = Int64FromConstant(shl->GetRight()->AsConstant());
if (((ldist + rdist) & (reg_bits - 1)) == 0) {
- ReplaceRotateWithRor(op, ushr, shl);
- return true;
+ return ReplaceRotateWithRor(op, ushr, shl);
}
return false;
}
@@ -643,6 +642,10 @@
// OP dst, dst, tmp
// with
// Ror dst, x, d
+//
+// Requires `d` to be non-zero for the HAdd and HXor case. If `d` is 0 the shifts and rotate are
+// no-ops and the `OP` is never executed. This is fine for HOr since the result is the same, but the
+// result is different for HAdd and HXor.
bool InstructionSimplifierVisitor::TryReplaceWithRotateRegisterNegPattern(HBinaryOperation* op,
HUShr* ushr,
HShl* shl) {
@@ -650,11 +653,20 @@
DCHECK(ushr->GetRight()->IsNeg() || shl->GetRight()->IsNeg());
bool neg_is_left = shl->GetRight()->IsNeg();
HNeg* neg = neg_is_left ? shl->GetRight()->AsNeg() : ushr->GetRight()->AsNeg();
- // And the shift distance being negated is the distance being shifted the other way.
- if (neg->InputAt(0) == (neg_is_left ? ushr->GetRight() : shl->GetRight())) {
- ReplaceRotateWithRor(op, ushr, shl);
+ HInstruction* value = neg->InputAt(0);
+
+ // The shift distance being negated is the distance being shifted the other way.
+ if (value != (neg_is_left ? ushr->GetRight() : shl->GetRight())) {
+ return false;
}
- return false;
+
+ const bool needs_non_zero_value = !op->IsOr();
+ if (needs_non_zero_value) {
+ if (!value->IsConstant() || value->AsConstant()->IsArithmeticZero()) {
+ return false;
+ }
+ }
+ return ReplaceRotateWithRor(op, ushr, shl);
}
// Try replacing code looking like (x >>> d OP x << (#bits - d)):
diff --git a/test/557-checker-instruct-simplifier-ror/src/Main.java b/test/557-checker-instruct-simplifier-ror/src/Main.java
index 5d4bb7a..667b35f 100644
--- a/test/557-checker-instruct-simplifier-ror/src/Main.java
+++ b/test/557-checker-instruct-simplifier-ror/src/Main.java
@@ -503,6 +503,7 @@
}
// (j << distance) + (j >>> -distance)
+ // We can't perform the optimization as distance might be `0`, resulting in the wrong value.
/// CHECK-START: long Main.rol_long_reg_v_negv_add(long, int) instruction_simplifier (before)
/// CHECK: <<ArgValue:j\d+>> ParameterValue
@@ -516,19 +517,17 @@
/// CHECK-START: long Main.rol_long_reg_v_negv_add(long, int) instruction_simplifier (after)
/// CHECK: <<ArgValue:j\d+>> ParameterValue
/// CHECK: <<ArgDistance:i\d+>> ParameterValue
- /// CHECK: <<Neg:i\d+>> Neg [<<ArgDistance>>]
- /// CHECK: <<Ror:j\d+>> Ror [<<ArgValue>>,<<Neg>>]
- /// CHECK: Return [<<Ror>>]
-
- /// CHECK-START: long Main.rol_long_reg_v_negv_add(long, int) instruction_simplifier (after)
- /// CHECK-NOT: Add
- /// CHECK-NOT: Shl
- /// CHECK-NOT: UShr
+ /// CHECK-DAG: <<Neg:i\d+>> Neg [<<ArgDistance>>]
+ /// CHECK-DAG: <<UShr:j\d+>> UShr [<<ArgValue>>,<<Neg>>]
+ /// CHECK-DAG: <<Shl:j\d+>> Shl [<<ArgValue>>,<<ArgDistance>>]
+ /// CHECK: <<Add:j\d+>> Add [<<Shl>>,<<UShr>>]
+ /// CHECK: Return [<<Add>>]
public static long rol_long_reg_v_negv_add(long value, int distance) {
return (value << distance) + (value >>> -distance);
}
// (j << distance) ^ (j >>> -distance)
+ // We can't perform the optimization as distance might be `0`, resulting in the wrong value.
/// CHECK-START: long Main.rol_long_reg_v_negv_xor(long, int) instruction_simplifier (before)
/// CHECK: <<ArgValue:j\d+>> ParameterValue
@@ -542,18 +541,60 @@
/// CHECK-START: long Main.rol_long_reg_v_negv_xor(long, int) instruction_simplifier (after)
/// CHECK: <<ArgValue:j\d+>> ParameterValue
/// CHECK: <<ArgDistance:i\d+>> ParameterValue
- /// CHECK: <<Neg:i\d+>> Neg [<<ArgDistance>>]
- /// CHECK: <<Ror:j\d+>> Ror [<<ArgValue>>,<<Neg>>]
- /// CHECK: Return [<<Ror>>]
+ /// CHECK-DAG: <<Neg:i\d+>> Neg [<<ArgDistance>>]
+ /// CHECK-DAG: <<UShr:j\d+>> UShr [<<ArgValue>>,<<Neg>>]
+ /// CHECK-DAG: <<Shl:j\d+>> Shl [<<ArgValue>>,<<ArgDistance>>]
+ /// CHECK: <<Xor:j\d+>> Xor [<<Shl>>,<<UShr>>]
+ /// CHECK: Return [<<Xor>>]
- /// CHECK-START: long Main.rol_long_reg_v_negv_xor(long, int) instruction_simplifier (after)
- /// CHECK-NOT: Xor
- /// CHECK-NOT: Shl
- /// CHECK-NOT: UShr
public static long rol_long_reg_v_negv_xor(long value, int distance) {
return (value << distance) ^ (value >>> -distance);
}
+ /// CHECK-START: void Main.$noinline$testDontOptimizeAddIntoRotate_Int() disassembly (after)
+ /// CHECK-NOT: Ror
+ public static void $noinline$testDontOptimizeAddIntoRotate_Int() {
+ int distance = returnFalse() ? 1 : 0;
+ int value = -512667375;
+ int expected_result = 2 * value;
+ int result = (value >>> distance) + (value << -distance);
+ assertIntEquals(expected_result, result);
+ }
+
+ /// CHECK-START: void Main.$noinline$testDontOptimizeAddIntoRotate_Long() disassembly (after)
+ /// CHECK-NOT: Ror
+ public static void $noinline$testDontOptimizeAddIntoRotate_Long() {
+ int distance = returnFalse() ? 1 : 0;
+ long value = -512667375L;
+ long expected_result = 2L * value;
+ long result = (value >>> distance) + (value << -distance);
+ assertLongEquals(expected_result, result);
+ }
+
+ /// CHECK-START: void Main.$noinline$testDontOptimizeXorIntoRotate_Int() disassembly (after)
+ /// CHECK-NOT: Ror
+ public static void $noinline$testDontOptimizeXorIntoRotate_Int() {
+ int distance = returnFalse() ? 1 : 0;
+ int value = -512667375;
+ int expected_result = 0;
+ int result = (value >>> distance) ^ (value << -distance);
+ assertIntEquals(expected_result, result);
+ }
+
+ /// CHECK-START: void Main.$noinline$testDontOptimizeXorIntoRotate_Long() disassembly (after)
+ /// CHECK-NOT: Ror
+ public static void $noinline$testDontOptimizeXorIntoRotate_Long() {
+ int distance = returnFalse() ? 1 : 0;
+ long value = -512667375L;
+ long expected_result = 0;
+ long result = (value >>> distance) ^ (value << -distance);
+ assertLongEquals(expected_result, result);
+ }
+
+ static boolean returnFalse() {
+ return false;
+ }
+
public static void main(String[] args) {
assertIntEquals(2, ror_int_constant_c_c(8));
assertIntEquals(2, ror_int_constant_c_c_0(8));
@@ -581,5 +622,11 @@
assertLongEquals(32L, rol_long_reg_v_negv_add(8L, 2));
assertLongEquals(32L, rol_long_reg_v_negv_xor(8L, 2));
+
+ $noinline$testDontOptimizeAddIntoRotate_Int();
+ $noinline$testDontOptimizeAddIntoRotate_Long();
+
+ $noinline$testDontOptimizeXorIntoRotate_Int();
+ $noinline$testDontOptimizeXorIntoRotate_Long();
}
}