[aarch64-port-dev ] RFD: AOT for AArch64

[Andrew Dinn]

On 27/03/18 16:08, Andrew Dinn wrote:
> I'll try to get final comments plus a yea or nay (well, ok I
> guess it's going to be a yea) posted by late tomorrow morning.
Well, this is all very interesting.

I used a Java debugger to step through code under jdk.tools.jaotc.Main
and gdb to step through an AOT-compiled Test.main([Ljava/lang/String;)V
and that made a lot of stuff clearer about how this all ties together.
It still doesn't really put me in any better position to critique the
patch further but it certainly makes me feel like a lot happier as to
what the patch is doing and why.

Since this works to AOT-compile and then run simple programs plus
java.base, I guess it caters for most (maybe all?) of the cases that are
likely to turn up in generation and subsequent execution. I think
whatever may be missing will be found by i) committing this and then
testing it on larger apps ii) letting you, me and others play with it
until we get to the point where we can spot any rare omissions that
testing does not uncover.

I have only one further comment on the Graal code:

1)
compiler/src/org.graalvm.compiler.lir.aarch64/src/org/graalvm/compiler/lir/aarch64/AArch64Call.java

@@ public class AArch64Call {
     public static void directJmp(CompilationResultBuilder crb,
AArch64MacroAssembler masm, InvokeTarget callTarget) {
         try (AArch64MacroAssembler.ScratchRegister scratch =
masm.getScratchRegister()) {
             int before = masm.position();
-            masm.movNativeAddress(scratch.getRegister(), 0L);
-            masm.jmp(scratch.getRegister());
+            if (GeneratePIC.getValue(crb.getOptions())) {
+                masm.jmp();
+            } else {
+                masm.movNativeAddress(scratch.getRegister(), 0L);
+                masm.jmp(scratch.getRegister());
+            }
             int after = masm.position();
             crb.recordDirectCall(before, after, callTarget, null);
             masm.ensureUniquePC();

The else branch here omits the helpful comment provided in earlier
method directCall which it might actually be useful to include. However,
in both cases it might also help to add a comment in the if branch to
explain that the call is guaranteed to fit into 28 bits because in the
PIC case far jumps/calls indirect through a PLT i.e. for directJmp:

@@ public class AArch64Call {
     public static void directJmp(CompilationResultBuilder crb,
AArch64MacroAssembler masm, InvokeTarget callTarget) {
         try (AArch64MacroAssembler.ScratchRegister scratch =
masm.getScratchRegister()) {
             int before = masm.position();
-            masm.movNativeAddress(scratch.getRegister(), 0L);
-            masm.jmp(scratch.getRegister());
+            if (GeneratePIC.getValue(crb.getOptions())) {
+                /*
+                 * Offset must fit into a 28-bit immediate as far jumps
+                 * require indirection through a PLT.
+                 * generate a PC-relative jump fixed up by the linker
+                 */
+                masm.jmp();
+            } else {
+                /*
+                 * Offset might not fit into a 28-bit immediate,
generate an indirect call with a 64-bit
+                 * immediate address which is fixed up by HotSpot.
+                 */
+                masm.movNativeAddress(scratch.getRegister(), 0L);
+                masm.jmp(scratch.getRegister());
+            }
             int after = masm.position();
             crb.recordDirectCall(before, after, callTarget, null);
             masm.ensureUniquePC();

and ditto for directCall.

Otherwise I'm happy to see a patch with all updates accumulated so far
pushed to Graal and hs.

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