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