RFR: 8267098: AArch64: C1 StubFrames end confusingly

[Alan Hayward]

On Fri, 14 May 2021 12:56:16 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> For many of the stub frames, a leave/ret is generated after the stub has
>> already branched or returned. This is confusing. For these cases, replace
>> the superfluous code with a should_not_reach_here
>> 
>> For handle excception, instead of storing return from the exception
>> handler on the stack, it can be moved directly into lr, replacing a store and
>> load with a single move. (If/when PAC support is implemented, then this store
>> would also have to be signed).
>
> src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp line 442:
> 
>> 440:   //      handler regardless of whether handler existed in the nmethod.
>> 441:   //      Move it out of the way to the return register.
>> 442:   __ mov(lr, r0);
> 
> I don't think that leaving LR live here is a good idea. Storing into the stack frame is fine.

My motivation here was then if/when PAC is enabled, that store will have to sign the value before storing, then auth the value on loading it again. That won't be the fastest, and seemed a waste. Agreed that it makes the code slightly more awkward.

> src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp line 607:
> 
>> 605:   const bool must_gc_arguments = true;
>> 606:   const bool dont_gc_arguments = false;
>> 607:   const bool does_not_return = true;
> 
> I wonder if an `enum` would read better here.
> 
> Something like
> 
> 
> enum may_return_t {
>   does_not_return, may_return
> };
> 
> class StubFrame {
> 
>   StubFrame(StubAssembler* sasm, const char* name, bool must_gc_arguments, may_return_t may_return);
> 
> };

Ok. I was keeping to the existing style of dont_gc_arguments/must_gc_arguments - but I could change those too so they match.

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

PR: https://git.openjdk.java.net/jdk/pull/4030