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

Hamlin Li mli at openjdk.org
Fri Dec 6 12:48:38 UTC 2024


On Thu, 5 Dec 2024 11:49:47 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

Nice tests, Thanks!
The code looks good, just some minor comments about the name.

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

> 277:   int num_threads = 4;
> 278:   CmpxchgTester<TESTSIZE, ASMSIZE> cmpxchg(0, false); // variant 0, not bool ret
> 279:   auto incThread = [&](Thread* _current, int _id) {

By checking the test framework code, seems passed-in `_id` starts from `0` for the first thread in thread group and increment `1` for every other thread?
It might be good have some comment here to state it explicitly to help read the code in the future.

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

PR Review: https://git.openjdk.org/jdk/pull/22574#pullrequestreview-2484741825
PR Review Comment: https://git.openjdk.org/jdk/pull/22574#discussion_r1873277720


More information about the hotspot-dev mailing list