RFR: 8340453: C2: Improve encoding of LoadNKlass for compact headers [v2]

[Roberto Castañeda Lozano]

On Wed, 13 Nov 2024 19:45:36 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> We currently use the offset 4 as a placeholder in LoadNKlass, when running with compact headers. In reality, we are loading from offset 0, but we want to keep LoadNKlass on a separate memory slice from other mark-word-accesses, because LoadNKlass is essentially immutable memory. The consequence is that we need to figure out the address of the mark-word in the backend, and this is ugly.
>> 
>> However, we can do better. We can just as well load 4 bytes from offset 4, and shift by a 32 smaller shift. This has previously not been possible because we needed to check for the monitor bit in the markWord, but this is no longer necessary. This simplifies the code and even makes the instructions encoding a bit smaller.
>> 
>> Testing:
>>  - [x] tier1 aarch64 +UCOH 
>>  - [x] tier1 x86_64 +UCOH
>
> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Improve opto asm of LoadNKlass

While this does not address IMO the conceptual problem of bending the meaning of `oopDesc::klass_offset_in_bytes()` through the C2 code base, it is a clear improvement over the existing model, thanks.

An advantage of this model over the one proposed in the JBS issue is that it exposes to C2 the actual address that will be loaded. This improves the confidence that there will not be any issue when matching complex addressing modes, etc. due to a mismatch between what C2 sees and what finally gets emitted.

It would be good to document the overloaded semantics of LoadNKlass for compact headers. Here is a suggestion: https://github.com/openjdk/jdk/commit/042317434d4644ac8f3591c8b1021e5651b5ed6d. If you agree, feel free to incorporate it as-is or edit it to your liking.

src/hotspot/cpu/aarch64/aarch64.ad line 6702:

> 6700:   format %{
> 6701:     "ldrw  $dst, $mem\t# compressed class ptr, shifted"
> 6702:     "lsrw  $dst, markWord::klass_shift_at_offset"

Suggestion:

    "lsrw  $dst, $dst, markWord::klass_shift_at_offset"

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

Changes requested by rcastanedalo (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22078#pullrequestreview-2435462969
PR Review Comment: https://git.openjdk.org/jdk/pull/22078#discussion_r1841832635