RFR: 8322630: Remove ICStubs and related safepoints [v3]
Aleksei Voitylov
avoitylov at openjdk.org
Tue Jan 30 08:49:50 UTC 2024
On Fri, 26 Jan 2024 15:12:17 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:
>> Erik Österlund has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Whitespace fix
>>
>> Co-authored-by: Thomas Schatzl <59967451+tschatzl at users.noreply.github.com>
>
> src/hotspot/cpu/arm/compiledIC_arm.cpp line 107:
>
>> 105: address stub = find_stub();
>> 106: guarantee(stub != nullptr, "stub not found");
>> 107:
>
> The other platforms removed the trace logging here. If the ARM porters still want this in at least update to log the correct class name. `s/CompiledDirectStaticCall/CompiledDirectCall/`
fixed by removing the trace logging.
> src/hotspot/cpu/arm/sharedRuntime_arm.cpp line 631:
>
>> 629:
>> 630: __ ic_check(1 /* end_alignment */);
>> 631: __ ldr(Rmethod, Address(receiver_klass, CompiledICData::speculated_method_offset()));
>
> Maybe I am missing something here but this looks very wrong. The speculated `Klass*` gets loaded into `R4` (which `receiver_klass` alias) in `ic_check` this load would result in loading a `InstanceKlass*` c++ vtable pointer.
> `Ricklass` (`R8` alias) contains the `CompiledICData*` .
>
> I would think the correct diff would be
>
> - const Register receiver_klass = R4;
> -
> - __ load_klass(receiver_klass, receiver);
> - __ ldr(holder_klass, Address(Ricklass, CompiledICHolder::holder_klass_offset()));
> - __ ldr(Rmethod, Address(Ricklass, CompiledICHolder::holder_metadata_offset()));
> - __ cmp(receiver_klass, holder_klass);
> + __ ic_check(1 /* end_alignment */);
> + __ ldr(Rmethod, Address(Ricklass, CompiledICData::speculated_method_offset()));
>
>
> The fact that you say ARM32 tests are passing makes me doubt my understanding of the inline cache.
hotspot jtreg tests pass in both variants, but you finding is correct. Updated the code with your suggestion.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17495#discussion_r1470784084
PR Review Comment: https://git.openjdk.org/jdk/pull/17495#discussion_r1470785164
More information about the shenandoah-dev
mailing list