[foreign-abi] On invokers

Nick Gasson nick.gasson at arm.com
Mon Oct 28 09:45:13 UTC 2019


Hi Jorn,

Updated webrev here:

http://cr.openjdk.java.net/~ngasson/panama/new-invoker/webrev.03/

I tested this with Maurizio's fix to CallGeneratorHelper.

> 
> universalNativeInvoker_aarch64.cpp
> - Why are there 15 bytes added to stack_arg_bytes when allocating space?
> I'd think that `__ sub(sp, sp, Rstack_size); __ andr(sp, sp, -16);`
> would be sufficient? Or is there a requirement to keep `sp` 16-byte
> aligned at _all_ times?

The architecture requires that SP be 16-byte aligned when it's used as 
the base for an address calculation. So in theory it should be fine to 
subtract Rstack_size and then AND with -16. But on AArch64 the encoding 
for the SP register is a bit special: register 31 can refer to either SP 
or the zero register (ZR) depending on the particular instruction. For 
the AND-immediate instruction, R31 as a source operand always denotes ZR 
so it's impossible to encode `__ andr(sp, sp, -16);`. We have to do the 
calculation in another register and then move it back into SP.

> - I see you've also added support for shadow space. But, since it's not
> used by any currently supported AArch64 ABI, I think it's better to
> remove the support for now to reduce clutter. If you want to keep it
> though; shadow space (at least on Windows) should not mess with the
> stack alignment, so there's no need to reallign the stack. Could add an
> assert like `abi._shadow_space_bytes % abi._stack_alignment_bytes == 0`
> to make sure. You will also, in that case, need to account for the
> shadow space when computing stack arguments offset for upcalls `__
> add(rscratch1, rfp, 16);` -> `__ add(rscratch1, rfp, 16 +
> abi._shadow_space_byte)`. (Looks like I forgot to change the hard-coded
> value in the x86 impl for that as well).

Removed the shadow space stuff as it not testable and added an assert 
that it's zero. Also replaced the hard-coded constant as suggested.

> 
> universalNativeInvoker_aarch64.cpp/universalUpcallHandler_aarch64.cpp
> - You should use abi._stack_alignment_bytes to align the stack instead
> of the hard-coded '16' literal. This is more important to have, since
> the current supported ABI actually uses stack alignment.
> 

Done.

Thanks,
Nick



More information about the panama-dev mailing list