RFR: 8329258: TailCall should not use frame pointer register for jump target [v4]

[Andrew Haley]

On Thu, 11 Apr 2024 10:31:09 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Applying @danielogh's patch (see description of [JDK-8329258](https://bugs.openjdk.org/browse/JDK-8329258)) to enable `StressGCM` / `StressLCM` for stub compilations triggers a crash on AArch64. The problem is that the register allocator uses `R29` (`rfp`) which is usually used for the frame pointer to hold the `TailCall` `exc_target` when generating the `_slow_arraycopy_Java` stub:
>> https://github.com/openjdk/jdk/blob/6dfb8120c270a76fcba5a5c3c9ad91da3282d5fa/src/hotspot/share/opto/generateOptoStub.cpp#L258-L264
>> 
>> With `StressGCM` / `StressLCM` the register initialization is scheduled early and `R29` is corrupted by the `MachEpilogNode` which is opaque to the register allocator and inserted right before the `TailCall`:
>> https://github.com/openjdk/jdk/blob/b49ba426a721db5926ac1b45d573d468389d479c/src/hotspot/share/opto/output.cpp#L320-L326 
>> 
>> 
>> 028 mov R29, 0x0000ffff78cc0080 # ptr
>> 
>> [...]
>> 
>> 098     # pop frame 16
>>         ldp  lr, rfp, [sp,#0]              <- Epilog kills rfp (and lr + sp)
>>         add  sp, sp, #16
>> 
>> [...]
>> 
>> 0a0 br R29 # R12 holds method
>> 
>> 
>> As a result, we jump to a "garbage" location. See [bad code](https://bugs.openjdk.org/secure/attachment/108835/BAD_slow_arraycopy_Java.txt) vs. [good code](https://bugs.openjdk.org/secure/attachment/108834/GOOD_slow_arraycopy_Java.txt).
>> 
>> On x86, we use `no_rbp_RegP` instead:
>> https://github.com/openjdk/jdk/blob/b49ba426a721db5926ac1b45d573d468389d479c/src/hotspot/cpu/x86/x86_64.ad#L2564-L2566 https://github.com/openjdk/jdk/blob/b49ba426a721db5926ac1b45d573d468389d479c/src/hotspot/cpu/x86/x86_64.ad#L12470
>> 
>> I implemented the same on AArch64.
>> 
>> I think other platforms are affected as well but I don't have the hardware to test there. @offamitkumar (S390), @TheRealMDoerr (PPC), @RealFYang (RISC-V), @bulasevich (ARM32), could you please have a look?
>> 
>> I also wondered if `R29` shouldn't be a callee-save (SOE) register in the C calling convention?
>> https://github.com/openjdk/jdk/blob/b49ba426a721db5926ac1b45d573d468389d479c/src/hotspot/cpu/aarch64/aarch64.ad#L139-L140 On x86_64, `RBP` is SOE:
>> https://github.com/openjdk/jdk/blob/b49ba426a721db5926ac1b45d573d468389d479c/src/hotspot/cpu/x86/x86_64.ad#L86-L87
>> 
>> `TestTailCallInArrayCopyStub.java` will only work once [JDK-8330016](https://bugs.openjdk.org/browse/JDK-8330016) is integrated.
>> 
>> Thanks,
>> Tobias
>
> Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Comment adjustment

Thanks. Sorry for my slow reply.

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

Marked as reviewed by aph (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18716#pullrequestreview-2035101941