[master] RFR: Load Klass from header, C1 [v11]

Aleksey Shipilev shade at openjdk.java.net
Tue Jan 11 16:57:15 UTC 2022


On Tue, 11 Jan 2022 12:37:22 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> This implements loading the Klass* from object header (instead of dedicated Klass* field) in C1 generated code.
>> 
>> Testing:
>>  - [x] tier1 (x86_64,x86_32, aarch64)
>>  - [x] tier2 (x86_64,x86_32, aarch64)
>>  - [x] tier3 (x86_64,x86_32, aarch64)
>>  - [ ] tier4 (x86_64,x86_32, aarch64)
>
> Roman Kennke has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 39 commits:
> 
>  - Merge branch 'master' into klass-from-header-c1
>  - Merge branch 'master' into klass-from-header-c1
>  - AArch64 implementation
>  - Merge remote-tracking branch 'origin/klass-from-header-c1' into klass-from-header-c1
>  - Merge remote-tracking branch 'upstream/master' into klass-from-header-c1
>  - Increase code size estimate
>  - Merge branch 'master' into klass-from-header-c1
>  - Merge remote-tracking branch 'jdk-upstream/master' into klass-from-header-c1
>  - Merge remote-tracking branch 'upstream/master' into klass-from-header-c1
>  - Basic arm implementation
>  - ... and 29 more: https://git.openjdk.java.net/lilliput/compare/af5b0d92...bc5b45e9

Looks fine for experimental code, some nits below.

src/hotspot/cpu/arm/c1_CodeStubs_arm.cpp line 247:

> 245: 
> 246: void LoadKlassStub::emit_code(LIR_Assembler* ce) {
> 247:   // Currently not needed.

Suggestion:

  // Currently not needed.
  Unimplemented();

src/hotspot/cpu/ppc/c1_CodeStubs_ppc.cpp line 323:

> 321: 
> 322: void LoadKlassStub::emit_code(LIR_Assembler* ce) {
> 323:   // Currently not needed.

Suggestion:

  // Currently not needed.
  Unimplemented();

src/hotspot/cpu/s390/c1_CodeStubs_s390.cpp line 274:

> 272: 
> 273: void LoadKlassStub::emit_code(LIR_Assembler* ce) {
> 274:   // Currently not needed.

Suggestion:

  // Currently not needed.
  Unimplemented();

src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp line 3531:

> 3529:   if (info != NULL) {
> 3530:     add_debug_info_for_null_check_here(info);
> 3531:   }

Wait, so this thing would not be executed for `!_LP64` anymore?

src/hotspot/cpu/x86/c1_Runtime1_x86.cpp line 1118:

> 1116:       {
> 1117:         StubFrame f(sasm, "load_klass", dont_gc_arguments);
> 1118:         sasm->save_live_registers_no_oop_map(true);

I presume calling `*_no_oop_map` here is intentional, and valid for `call_VM_leaf`? I.e. no safepoint expected here?

src/hotspot/share/oops/oop.cpp line 183:

> 181:   assert(oopDesc::is_oop(obj), "need a valid oop here: " PTR_FORMAT, p2i(o));
> 182:   Klass* klass = obj->klass();
> 183:   return klass;

Suggestion:

  return obj->klass();

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

Marked as reviewed by shade (Committer).

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


More information about the lilliput-dev mailing list