[master] RFR: Load narrowKlass from header, AArch64 assembler implementation [v2]

[Roman Kennke]

On Thu, 27 Jan 2022 14:27:57 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add asserts
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 3722:
> 
>> 3720:   // Fast-path: shift and decode Klass*.
>> 3721:   mov(dst, tmp);
>> 3722:   lsr(dst, dst, markWord::klass_shift);
> 
> Suggestion:
> 
>   lsr(dst, tmp, markWord::klass_shift);

Done.

> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 3733:
> 
>> 3731:   // We don't need to preserve r0 here, but we need to preserve rscratch1 and rescratch2,
>> 3732:   // because some users of load_klass() use them around the call.
>> 3733:   push(RegSet::of(rscratch1, rscratch2), sp);
> 
> This is all very weird, and looks confusing. I think it might be safer to force the callers of `load_klass()` to provide a temp.

I checked again, and it actually looks like we don't need to preserve rscratch1 and rscratch2. I added asserts to verify that src register isn't one of the rscratch registers and would risk to get clobbered. It doesn't matter if dst is rscratch register.

> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 3737:
> 
>> 3735:   assert(StubRoutines::load_nklass() != NULL, "Must have stub");
>> 3736:   mov(rscratch1, StubRoutines::load_nklass());
>> 3737:   blr(rscratch1);
> 
> Suggestion:
> 
>   far_call(RuntimeAddress(StubRoutines::load_nklass()), rscratch1);

Done (without the rscratch1 arg, which is the default anyway).

> src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 6619:
> 
>> 6617:     __ set_last_Java_frame(sp, rfp, lr, rscratch1);
>> 6618:     __ enter();
>> 6619:     __ push_call_clobbered_registers_except(RegSet::of(r0));
> 
> Suggestion:
> 
>     __ push_call_clobbered_registers_except(r0);
> 
> ... everywhere.

Done.

> src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 6621:
> 
>> 6619:     __ push_call_clobbered_registers_except(RegSet::of(r0));
>> 6620:     __ call_VM_leaf(CAST_FROM_FN_PTR(address, oopDesc::load_nklass_runtime), 1);
>> 6621:     __ pop_call_clobbered_registers_except(RegSet::of(r0));
> 
> Suggestion:
> 
>     __ pop_call_clobbered_registers_except(r0);

Done.

> src/hotspot/share/oops/oop.hpp line 312:
> 
>> 310: #ifdef _LP64
>> 311:   static int nklass_offset_in_bytes()    {
>> 312:     assert(markWord::klass_shift % 8 == 0, "klass_shift must be multiple of 8");
> 
> Suggestion:
> 
>     STATIC_ASSERT(markWord::klass_shift % 8 == 0, "klass_shift must be multiple of 8");

Done.

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

PR: https://git.openjdk.java.net/lilliput/pull/36