RFR: 8356949: AArch64: Tighten up template interpreter method entry code [v2]
Andrew Haley
aph at openjdk.org
Thu May 22 09:22:50 UTC 2025
On Thu, 22 May 2025 08:41:19 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>>> I can give `r5` a name but what do you think about `r10`? I feel like naming it something like `r10_ConstantPool` implies that is what it will always hold which isn't the case
>>
>> I'd perhaps call it `r10_ConstantPool`, then switch to just `r10`. I don't think that will confuse anyone.
>> You could simply use another register: at interpreter entry most are free, so something between `r11` and `r17`.
>
> Honestly, `r11_ConstantPool` confuses me in relation to `rcpool`. It is a problem with `rcpool` as the name, not worth renaming here. But, out of style considerations: `r11_constants` and `r5_const_method`?
>
> I also sometimes do things like `const Register r11 = noreg;`, so that this local shadow makes the rest of the method barf if someone uses `r11` without thinking twice.
>
> Plus, comments like `// Use stored ConstMethod* to avoid loading again` are self-obvious when we are loading `r5_const_method` :) I made the comments in the x86 version to track which register carries the cached value, but extended register names make this obvious. (I wish I thought about this back then.)
At some risk of bikeshedding...
> Honestly, `r11_ConstantPool` confuses me in relation to `rcpool`. It is a problem with `rcpool` as the name, not worth renaming here. But, out of style considerations: `r11_constants` and `r5_const_method`?
Good.
> I also sometimes do things like `const Register r11 = noreg;`, so that this local shadow makes the rest of the method barf if someone uses `r11` without thinking twice.
>
> Plus, comments like `// Use stored ConstMethod* to avoid loading again` are self-obvious when we are loading `r5_const_method` :) I made the comments in the x86 version to track which register carries the cached value, but extended register names make this obvious. (I wish I thought about this back then.)
I agree.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25312#discussion_r2102066125
More information about the hotspot-dev
mailing list