[lworld] RFR: 8266890: [lworld] [AArch64] add support for InlineTypePassFieldsAsArgs [v2]

[Tobias Hartmann]

On Thu, 27 May 2021 10:19:41 GMT, Nick Gasson <ngasson at openjdk.org> wrote:

>> This patch implements InlineTypePassFieldsAsArgs on AArch64 and the
>> associated stack extension/repair mechanism. It mostly follows the x86
>> implementation closely except for how the stack increment is stored in
>> the callee frame. On x86 the sp_inc stack slot stores the total size of
>> the frame which is the sum of the extension space, return address copy,
>> and the original method frame size (i.e. the total bytes needed to pop
>> the frame). On AArch64 we just store the size of the extension space. I
>> did it this way because it simplifies the stack repair code in
>> MacroAssembler::remove_frame (I've added some notes there to document
>> this). I don't think this should cause any problem because only the
>> MacroAssembler and platform-dependant frame walking code need to be
>> aware of this.
>> 
>> This patch includes JDK-8266609 which is a small refactoring in mainline
>> JDK to simplify how the frame size is passed around in the AArch64 C1
>> backend.
>> 
>> There was some X86 specific code in unpack_inline_args() in
>> macroAssembler_common.cpp. I've split this out into an arch-dependant
>> MacroAssembler::extend_stack_for_inline_args().
>> 
>> MacroAssembler::pack_inline_helper() and shuffle_inline_args() now take
>> a new Register val_array argument which is the register holding the
>> buffered oop array for packing. Previously it assumed this was already
>> loaded in RAX (x86) or x20 (AArch64) but IMO passing it in explicitly
>> makes the code easier to understand.
>> 
>> There is one new test failure: TestNullableInlineTypes.java. The test
>> seems functionally correct but there is a failure verifying the C2 IR
>> graph (see below). I haven't investigated this but the test enables
>> InlineTypePassFieldsAsArgs which we don't support yet on AArch64 so that
>> might be causing the failure.
>> 
>> 
>> Exception in thread "main" java.lang.RuntimeException: Graph for 'TestNullableInlineTypes::test26' contains forbidden node:
>> 83  StoreL  ===  176  177  84  14  [[ 89 ]]  @compiler/valhalla/inlinetypes/MyValue3:exact+16 *, name=l, idx=6;  Memory: @rawptr:BotPTR, idx=Raw; !orig=82 !jvms: TestNullableInlineTypes::test26 @ bci:13 (line 704): expected false, was true
>> 
>> 
>> Tested tier1, runtime/valhalla, and compiler/valhalla on x86 and AArch64. There are some "bad AD file" failures on x86 but don't seem to be related to this patch.
>
> Nick Gasson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Store total frame size in sp_inc

Looks good to me.

> Sure. Do you have any ideas about this one? https://bugs.openjdk.java.net/browse/JDK-8264340

That doesn't ring a bell. Is it reproducible?

-------------

Marked as reviewed by thartmann (Committer).

PR: https://git.openjdk.java.net/valhalla/pull/420