[master] RFR: Load Klass* from header in interpreter (x86) [v5]

[Roman Kennke]

On Tue, 12 Oct 2021 12:34:56 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Roman Kennke has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>> 
>>  - Merge branch 'master' into klass-from-header-interpreter
>>  - Emit null-checks before load_klass with correct offset
>>  - Use constant instead of literal in xor-test-sequence
>>  - Update comment about xorb/xorq encoding
>>  - Load Klass* from header in interpreter (x86)
>
> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 4554:
> 
>> 4552:   // NOTE: While it would seem nice to use xorb instead (for which we don't have an encoding in our assembler),
>> 4553:   // the encoding for xorq uses the signed version (0x81/6) of xor, which encodes as compact as xorb would,
>> 4554:   // and does't make a difference performance-wise.
> 
> I can give you `xorb`, if you want, in a subsequent PR.

I actually have it, locally, but as comment says, it doesn't look any better in terms of code size or performance, so why bother?

> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 4597:
> 
>> 4595:     null_check(src, oopDesc::klass_offset_in_bytes());
>> 4596:   }
>> 4597:   movptr(dst, Address(src, oopDesc::klass_offset_in_bytes()));
> 
> Do we not need to do any fixups for x86_32?

No because layout-wise, the Klass* remains 4 bytes from object start, and would never get disturbed by anything that happens in the 32-bit-header. Hopefully, when we find a way to avoid displaced headers altogether, we can unify the code with x86_64 and avoid the mess that we make here.

> src/hotspot/cpu/x86/templateTable_x86.cpp line 4185:
> 
>> 4183:   Register tmp_load_klass = LP64_ONLY(rscratch1) NOT_LP64(noreg);
>> 4184:   __ load_klass(rdx, rdx, tmp_load_klass, false);
>> 4185:   __ jmp(resolved);
> 
> Have you run into troubles with `jmpb` short branch here?

Yes. The (subsequent) load_klass() increases the distance to resolved.

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

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