RFR: 8345322: RISC-V: Add concurrent gtests for cmpxchg variants [v2]

[Fei Yang]

On Tue, 10 Dec 2024 13:28:56 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> Hi, please consider these additional concurrent tests.
>> 
>> (this will not go into 24)
>> 
>> There are two concurrent counter versions:
>> - Each thread is exclusively responsible for an certain increment steps
>> - Each thread plainly tries to CAS increment by one
>> 
>> I refactored the code, so these concurrent versions can reuse the generated CAS functions.
>> 
>> 
>> [ RUN      ] RiscV.cmpxchg_int64_concurrent_lr_sc_vm
>> [       OK ] RiscV.cmpxchg_int64_concurrent_lr_sc_vm (24 ms)
>> [ RUN      ] RiscV.cmpxchg_int64_concurrent_maybe_zacas_vm
>> [       OK ] RiscV.cmpxchg_int64_concurrent_maybe_zacas_vm (12 ms)
>> [ RUN      ] RiscV.cmpxchg_int32_concurrent_lr_sc_vm
>> [       OK ] RiscV.cmpxchg_int32_concurrent_lr_sc_vm (14 ms)
>> [ RUN      ] RiscV.cmpxchg_int32_concurrent_maybe_zacas_vm
>> [       OK ] RiscV.cmpxchg_int32_concurrent_maybe_zacas_vm (14 ms)
>> [ RUN      ] RiscV.cmpxchg_int16_concurrent_lr_sc_vm
>> [       OK ] RiscV.cmpxchg_int16_concurrent_lr_sc_vm (15 ms)
>> [ RUN      ] RiscV.cmpxchg_int16_concurrent_maybe_zacas_vm
>> [       OK ] RiscV.cmpxchg_int16_concurrent_maybe_zacas_vm (14 ms)
>> [ RUN      ] RiscV.cmpxchg_int8_concurrent_lr_sc_vm
>> [       OK ] RiscV.cmpxchg_int8_concurrent_lr_sc_vm (14 ms)
>> [ RUN      ] RiscV.cmpxchg_int8_concurrent_maybe_zacas_vm
>> [       OK ] RiscV.cmpxchg_int8_concurrent_maybe_zacas_vm (14 ms)
>> [ RUN      ] RiscV.weak_cmpxchg_int64_concurrent_lr_sc_vm
>> [       OK ] RiscV.weak_cmpxchg_int64_concurrent_lr_sc_vm (15 ms)
>> [ RUN      ] RiscV.weak_cmpxchg_int64_concurrent_maybe_zacas_vm
>> [       OK ] RiscV.weak_cmpxchg_int64_concurrent_maybe_zacas_vm (11 ms)
>> [ RUN      ] RiscV.weak_cmpxchg_int32_concurrent_lr_sc_vm
>> [       OK ] RiscV.weak_cmpxchg_int32_concurrent_lr_sc_vm (15 ms)
>> [ RUN      ] RiscV.weak_cmpxchg_int32_concurrent_maybe_zacas_vm
>> [       OK ] RiscV.weak_cmpxchg_int32_concurrent_maybe_zacas_vm (12 ms)
>> [ RUN      ] RiscV.weak_cmpxchg_int16_concurrent_lr_sc_vm
>> [       OK ] RiscV.weak_cmpxchg_int16_concurrent_lr_sc_vm (13 ms)
>> [ RUN      ] RiscV.weak_cmpxchg_int16_concurrent_maybe_zacas_vm
>> [       OK ] RiscV.weak_cmpxchg_int16_concurrent_maybe_zacas_vm (14 ms)
>> [ RUN      ] RiscV.weak_cmpxchg_int8_concurrent_lr_sc_vm
>> [       OK ] RiscV.weak_cmpxchg_int8_concurrent_lr_sc_vm (13 ms)
>> [ RUN      ] RiscV.weak_cmpxchg_int8_concurrent_maybe_zacas_vm
>> [       OK ] RiscV.weak_cmpxchg_int8_concurrent_maybe_zacas_vm (15 ms)
>> 
>> 
>> Execute with +UseZacas, and without on BPI-F3.
>> 
>> Thanks, Robbin
>
> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Code share
>  - Overflow
>  - Merge branch 'master' into 8345322
>  - Concurrent

Thanks for the update. Two comments remain.

test/hotspot/gtest/riscv/test_assembler_riscv.cpp line 355:

> 353:   TESTSIZE diff = std::numeric_limits<TESTSIZE>::max() - now;
> 354:   add -= diff;
> 355:   return std::numeric_limits<TESTSIZE>::min() + add;

`next_count(127, 1)` returns -127. Is this expected? Shouldn't we get -128 after the wrap around?

test/hotspot/gtest/riscv/test_assembler_riscv.cpp line 375:

> 373: template<>
> 374: constexpr int64_t result_count<int64_t>() {
> 375:     return std::numeric_limits<int64_t>::min() + TOTAL_ITERATIONS;

There is an indentation issue for this line. Maybe we can specialize `int32_t` as well?

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

PR Review: https://git.openjdk.org/jdk/pull/22574#pullrequestreview-2494194962
PR Review Comment: https://git.openjdk.org/jdk/pull/22574#discussion_r1879261321
PR Review Comment: https://git.openjdk.org/jdk/pull/22574#discussion_r1879269786