[master] RFR: Load Klass from header, C1/x86 implementation

[Roman Kennke]

On Mon, 18 Oct 2021 07:12:45 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> This implements loading the Klass* from object header (instead of dedicated Klass* field) in C1 generated code. It introduces a new C1 LIR opcode (LoadKlass) and related CodeStub for the slow-path runtime-call.
>> 
>> The original implementation was brittle in that it emits the decode_klass() whenever it encounters a load that is T_ADDRESS and from klass_offset_in_bytes() (e.g. 4 or 8). Lucky that we did not seem to emit any unrelated such loads. This new implementation uses a dedicated C1 op instead, and expands this to the corresponding load of the header word, and call into slow-path upon fast-path-failure, in the LIR assembler.
>> 
>> Testing:
>>  - [x] tier1 (x86_64,x86_32)
>>  - [x] tier2 (x86_64,x86_32)
>>  - [ ] tier3 (x86_64,x86_32)
>>  - [ ] tier4 (x86_64,x86_32)
>
> src/hotspot/cpu/x86/c1_CodeStubs_x86.cpp line 311:
> 
>> 309:     // without messing with the stack.
>> 310:     __ xchgptr(rax, res);
>> 311:   }
> 
> I am trying to wrap my head around this in early Monday :) Is `res` guaranteed to be callee-save? I.e. does this stashing work if runtime call clobbers `res`?

The actual save/restore of registers and the runtime call happens in the stub in c1_Runtime1_x86.cpp

> src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp line 1260:
> 
>> 1258: 
>> 1259:     case T_ADDRESS:
>> 1260:       __ movptr(dest->as_register(), from_addr);
> 
> Can we / should we `assert(addr->disp() != oopDesc::klass_offset_in_bytes(), "sanity")` here and later? So that the non-`LoadKlass` loads would break early?

Not sure, tbh. The current implementation is quite brittle, and we are lucky that we have apparently never hit (emit) a load with offset 8 (or 4 on x86_32) which happens to be klass_offset_in_bytes().

> src/hotspot/cpu/x86/c1_Runtime1_x86.cpp line 1124:
> 
>> 1122:       }
>> 1123: #else
>> 1124:       __ should_not_reach_here();
> 
> So the intent here to generate x86_32 stub, but check it is not called ever, right? I wonder if we still need to do `StubFrame f(sasm, "load_klass", dont_gc_arguments);` even for a simple `__ should_not_reach_here();`...

Perhaps not. Let me see if I can avoid this altogether.

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

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