RFR: 8342014: RISC-V: ZStoreBarrierStubC2 clobbers rflags

[Fei Yang]

On Mon, 14 Oct 2024 07:57:14 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> `ZStoreBarrierStubC2` (`ZBarriersetAssembler::generate_c2_store_barrier_stub`) clobbers rflags (the `t1` register) on riscv [1].
>> And `ZStoreBarrierStubC2` is used by `z_store_barrier` in file gc/z/z_riscv.ad. But the calling instructs in the same ad file
>> didn't list the rflags as being killed. As the call chain is not simple, this kind of problem could go silently unnoticed.
>> I would suggest we add clobbering of rflags for all gc-related C2 instructs. No obvious impact witnessed on performance.
>> This would help reduce the risk of another PR: https://github.com/openjdk/jdk/pull/21406 which touches g1/x/z prefering `t1` for performance reasons. Tagging @robehn 
>> 
>> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/gc/z/zBarrierSetAssembler_riscv.cpp#L746
>> 
>> Testing on linux-riscv64:
>> - [x] hs-tier1 - hs-tier3
>
> src/hotspot/cpu/riscv/gc/g1/g1_riscv.ad line 169:
> 
>> 167:   predicate(UseG1GC && n->as_LoadStore()->barrier_data() != 0);
>> 168:   match(Set res (CompareAndExchangeP mem (Binary oldval newval)));
>> 169:   effect(TEMP res, TEMP tmp1, TEMP tmp2);
> 
> should the effect of `res` be changed from `TEMP` to `DEF_TEMP`?
> And other similar usage places.

Ah, that looks like a refactoring change to me. Maybe go with a separate change if it's necessary? Seems not strongly related to this PR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21485#discussion_r1799023251