ART: Release inputs in Long.reverse intrinsic in x86

In the worst case we are using two temps each for input and output.
Then we do not have a temp left over for the swap operations. The
input is dead, however, after the first swap. So try to release it
(a no-op if it isn't actually a temp).

Bug: 22324327

(cherry picked from commit 575422fa5be7389bdaff5e2d25dd87b1d2d4de85)

Change-Id: I1fc50159afdad14160e34abeaf4670958171d6b2

2 files changed, 54 insertions, 0 deletions

diff --git a/compiler/dex/quick/x86/int_x86.cc b/compiler/dex/quick/x86/int_x86.cc
index d993d93..d1fe167 100755
--- a/compiler/dex/quick/x86/int_x86.cc
+++ b/compiler/dex/quick/x86/int_x86.cc
@@ -1336,9 +1336,24 @@ bool X86Mir2Lir::GenInlinedReverseBits(CallInfo* info, OpSize size) {
     }
     OpRegReg(kOpRev, rl_result.reg.GetLow(), rl_i.reg.GetHigh());
     OpRegReg(kOpRev, rl_result.reg.GetHigh(), r_i_low);
+    // Free up at least one input register if it was a temp. Otherwise we may be in the bad
+    // situation of not having a temp available for SwapBits. Make sure it's not overlapping
+    // with the output, though.
     if (rl_i.reg.GetLowReg() == rl_result.reg.GetLowReg()) {
+      // There's definitely a free temp after this.
       FreeTemp(r_i_low);
+    } else {
+      // We opportunistically release both here. That saves duplication of the register state
+      // lookup (to see if it's actually a temp).
+      if (rl_i.reg.GetLowReg() != rl_result.reg.GetHighReg()) {
+        FreeTemp(rl_i.reg.GetLow());
+      }
+      if (rl_i.reg.GetHighReg() != rl_result.reg.GetLowReg() &&
+          rl_i.reg.GetHighReg() != rl_result.reg.GetHighReg()) {
+        FreeTemp(rl_i.reg.GetHigh());
+      }
     }
+
     SwapBits(rl_result.reg.GetLow(), 1, 0x55555555);
     SwapBits(rl_result.reg.GetLow(), 2, 0x33333333);
     SwapBits(rl_result.reg.GetLow(), 4, 0x0f0f0f0f);
diff --git a/test/082-inline-execute/src/Main.java b/test/082-inline-execute/src/Main.java
index 4dfa73c..177c5a4 100644
--- a/test/082-inline-execute/src/Main.java
+++ b/test/082-inline-execute/src/Main.java
@@ -1000,6 +1000,45 @@ public class Main {
     Assert.assertEquals(Long.reverse(0x8765432187654321L), 0x84c2a6e184c2a6e1L);
     Assert.assertEquals(Long.reverse(Long.MAX_VALUE), 0xfffffffffffffffeL);
     Assert.assertEquals(Long.reverse(Long.MIN_VALUE), 1L);
+
+    Assert.assertEquals(test_Long_reverse_b22324327(0xaaaaaaaaaaaaaaaaL, 0x5555555555555555L),
+            157472205507277347L);
+  }
+
+  // A bit more complicated than the above. Use local variables to stress register allocation.
+  private static long test_Long_reverse_b22324327(long l1, long l2) {
+    // A couple of local integers. Use them in a loop, so they get promoted.
+    int i1 = 0, i2 = 1, i3 = 2, i4 = 3, i5 = 4, i6 = 5, i7 = 6, i8 = 7;
+    for (int k = 0; k < 10; k++) {
+      i1 += 1;
+      i2 += 2;
+      i3 += 3;
+      i4 += 4;
+      i5 += 5;
+      i6 += 6;
+      i7 += 7;
+      i8 += 8;
+    }
+
+    // Do the Long.reverse() calls, save the results.
+    long r1 = Long.reverse(l1);
+    long r2 = Long.reverse(l2);
+
+    // Some more looping with the ints.
+    for (int k = 0; k < 10; k++) {
+      i1 += 1;
+      i2 += 2;
+      i3 += 3;
+      i4 += 4;
+      i5 += 5;
+      i6 += 6;
+      i7 += 7;
+      i8 += 8;
+    }
+
+    // Include everything in the result, so things are kept live. Try to be a little bit clever to
+    // avoid things being folded somewhere.
+    return (r1 / i1) + (r2 / i2) + i3 + i4 + i5 + i6 + i7 + i8;
   }
 
   static Object runtime;