RFR: 8344232: [PPC64] secondary_super_cache does not scale well: C1 and interpreter
Richard Reingruber
rrich at openjdk.org
Wed Jan 22 12:07:37 UTC 2025
On Tue, 21 Jan 2025 17:20:10 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:
>> src/hotspot/cpu/ppc/c1_Runtime1_ppc.cpp line 607:
>>
>>> 605: super_klass = R4,
>>> 606: temp1_reg = R6;
>>> 607: __ check_klass_subtype_slow_path(sub_klass, super_klass, temp1_reg, noreg); // may return with CR0.eq if successful
>>
>> The comment is unclear to me. Where is the result of the subtype check? Can it also return with CR0.ne if successful?
>> I noticed you added the `crandc` to `check_klass_subtype_slow_path_linear()` but if we reach there calling from this location then the `crandc` is not emitted because `L_success == nullptr`. Is this ok?
>> I'd appreciate comments on the masm methods explaining how the result of the subtype check is conveyed.
>
> The correct result is always in CR0 with this PR (unless a label or result GP reg is provided).
> "return" means "blr", here. That can optionally be used in case of success. In this case, CR0 is always "eq".
> I've moved the `crandc` instruction into `check_klass_subtype_slow_path_linear` which contains such a "blr" for a success case. This way, the linear version works exactly as before.
> The new code `check_klass_subtype_slow_path_table` doesn't use "blr". That's why I added "may" to the comment.
This is extremely hard to see.
L2154 with the "blr" in `check_klass_subtype_slow_path_linear` looks redundant to me. It should be removed if you agree.
The comment here should be adapted then too.
Also the comment at macroAssembler_ppc.cpp:2258 needs to be adapted because fallthrough from `check_klass_subtype_slow_path` does not mean "not successful". `L_failure` could be renamed to `L_fast_path_failure`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22881#discussion_r1925207385
More information about the hotspot-dev
mailing list