RFR: AArch64: 8203699 java/lang/invoke/SpecialInterfaceCall fails with SIGILL on aarch64

[Andrew Dinn]

On 23/05/18 20:34, Andrew Dinn wrote:
> On 23/05/18 18:28, Aleksey Shipilev wrote:
>> On 05/23/2018 06:54 PM, Andrew Dinn wrote:
>>> JIRA:     http://bugs.openjdk.java.net/browse/JDK-8203699
>>>
>>> webrev:   http://cr.openjdk.java.net/~adinn/8203699/webrev.00/
>>
>> Looks good.
>>
>> It passes the test for me, on the platform where it originally failed.
> 
> Thanks for the review Aleksey. Anyone else?
Well, in the absence of anyone else to review this patch I guess I can
always provide my own :-)

I followed the logic of x86 in adding this conditional call to mov

   push(pushed_registers, sp);

+  if (super_klass != r0 || UseCompressedOops) {
+    mov(r0, super_klass);
+  }

Now, the x86 code munges these two cases together and plants a move when
UseCompressedOops is true because it does the move in the same
conditional branch as the push.

  if (super_klass != rax || UseCompressedOops) {
    if (!IS_A_TEMP(rax)) { push(rax); pushed_rax = true; }
    mov(rax, super_klass);
  }

A push /is/ needed for the UseCompressedOops case because r0/rax gets
modified by an uncompress. The move does no harm on x86 when super_klass
= rax because mov(rax, rax) gets elided. So, in that case calling mov is
merely wasted generation time not wasted execution time.

In the above AArch64 code it looks odd to include the move under the OR
with UseCompressedOops. It only makes sense to do the move either when
super_klass != r0 or unconditionally -- you either rely on the mov
getting elided or you only call mov when the registers are different.

So, I propose correcting patch to

   push(pushed_registers, sp);

+  if (super_klass != r0) {
+    mov(r0, super_klass);
+  }

Revised webrev: http://cr.openjdk.java.net/~adinn/8203699/webrev.01/

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander