RFR: 8339983: [s390x] secondary_super_cache does not scale well: C1 and interpreter [v2]

[Lutz Schmidt]

On Mon, 25 Nov 2024 12:59:10 GMT, Amit Kumar <amitkumar at openjdk.org> wrote:

>> s390x Port for : [JDK-8331341](https://bugs.openjdk.org/browse/JDK-8331341)
>> 
>> Tier1 test: 
>> 1. `-XX:+UseSecondarySupersTable  -XX:+VerifySecondarySupers -XX:+VerifySecondarySupers -XX:-UseSecondarySupersCache`
>> 2.  No flag turn on. i.e. used `UseSecondarySupersCache` by default. 
>> 3. `-XX:+UseSecondarySupersTable  -XX:+VerifySecondarySupers -XX:+VerifySecondarySupers -XX:-UseSecondarySupersCache` with C1 compiler
>
> Amit Kumar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   suggestion from Andrew

I had a few minor remarks. Code seems OK (I didn't test).

src/hotspot/cpu/s390/macroAssembler_s390.cpp line 3170:

> 3168:   RegSetIterator<Register> available_regs
> 3169:   // Z_R0 will be used to hold Z_R15(Z_SP) while pushing a new frame, So don't use that here.
> 3170:   // Z_R1 is will be used to hold r_bitmap in lookup_secondary_supers_table_var, so can't be used

"is" or "will be"?

src/hotspot/cpu/s390/macroAssembler_s390.cpp line 3200:

> 3198:                                     temp_reg, temp2_reg, temp3_reg, temp4_reg, result_reg);
> 3199: 
> 3200:   z_cghi(result_reg, 0);

There is a significant (and varying) distance between here and where the CC is evaluated. I would expect at least a comment that the code in between must not alter the CC.

src/hotspot/cpu/s390/macroAssembler_s390.cpp line 3423:

> 3421:                                                        Register result) {
> 3422:   assert_different_registers(r_sub_klass, r_super_klass, temp1, temp2, temp3, temp4, result, Z_R1_scratch, Z_R0_scratch);
> 3423:   // Z_R0 will be only used in debug builds for slot value varification.

Typo. Type vErification, please.

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

Changes requested by lucy (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22341#pullrequestreview-2458761205
PR Review Comment: https://git.openjdk.org/jdk/pull/22341#discussion_r1858536074
PR Review Comment: https://git.openjdk.org/jdk/pull/22341#discussion_r1856808440
PR Review Comment: https://git.openjdk.org/jdk/pull/22341#discussion_r1856810391