RFR: 8267098: AArch64: C1 StubFrames end confusingly

Andrew Haley aph at openjdk.java.net
Fri May 14 13:53:48 UTC 2021


On Fri, 14 May 2021 13:15:45 GMT, Alan Hayward <github.com+4146708+a74nh at openjdk.org> wrote:

>> 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.

This code is a legacy carried over from x86. It's not necessary to "match". Changing others would be unnecessary churn, and risk breakage.

There's an urge from some contributors: when I suggest doing something in an easy-to-understand and clean way, people want to change everything else to match. This urge can be resisted, and IMVHO should be in this case. Churn is, in itself, bad.

And this case, is special, I think, because `does_not_return` uses the "don't do" anti-pattern, where the `true` case was `does_not_return`.

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

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


More information about the hotspot-compiler-dev mailing list