[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