RFR: 8342014: RISC-V: ZStoreBarrierStubC2 clobbers rflags

Hamlin Li mli at openjdk.org
Mon Oct 14 08:01:11 UTC 2024


On Mon, 14 Oct 2024 02:45:35 GMT, Fei Yang <fyang 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

found another one, not sure if it's safe to make the change either.

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.

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

PR Review: https://git.openjdk.org/jdk/pull/21485#pullrequestreview-2365685832
PR Review Comment: https://git.openjdk.org/jdk/pull/21485#discussion_r1798939069


More information about the hotspot-dev mailing list