RFR: 8351949: RISC-V: Cleanup and enable store-load peephole for membars [v2]

[Fei Yang]

On Fri, 14 Mar 2025 11:38:19 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> Hi please consider.
>> 
>> |RVWMO| Patched|
>> | ---------- | ---------- |
>> |fence iorw,iorw| fence iorw,ow|
>> |sw  t4,120(t2) | sw  t4,120(t2) |
>> |fence ow,ir | unnecessary_membar_volatile_rvwmo |
>> | sw  t6,128(t2) // Non-volatile | sw  t6,128(t2) // Non-volatile |
>> |fence iorw,ow | fence iorw,ow|
>> |sw  t5,124(t2) |sw  t5,124(t2) |
>> 
>> |TSO                                | Patched|
>> | ---------- | ---------- |
>> | lw	a4,120(t2) | lw	a6,120(t2) |
>> | sw	a0,124(t2) | sw	t6,124(t2) |
>> | fence	iorw,iorw | unnecessary_membar_volatile_tso |
>> | sw	t4,120(t2) | sw	t4,120(t2) |
>> | fence	ow,ir | unnecessary_membar_volatile_tso |
>> | sw	t6,128(t2) | sw	t5,128(t2) |
>> | sw	t5,124(t2) // Non-volatile| sw	a1,124(t2)  // Non-volatile |
>> | fence	iorw,iorw  | unnecessary_membar_volatile_tso |
>> |...  | ... |
>> | sw	a3,120(t2) | sw	a0,120(t2) |
>> | fence	ow,ir | fence	ow,ir |
>> | lw	a7,124(t2) | lw	a5,124(t2) |
>> 
>> For the specific rvwmo volatile store + store + volatile store is around 30% faster on VF2.
>> 
>> The patch do:
>> - Separate ztso and rvwmo in ad by using UseZtso predicate.
>> - Match all that requires the same membar.
>> - Make fence/fencei protected as they shouldn't be using directly.
>> - Increased cost of membars to VOLATILE_REF_COST.
>> - Added a real_empty pipe.
>> - Change to pipe_slow on TSO (as x86).
>> 
>> Note that C2-rv64 is now superior to gcc/clang regrading fencing:
>> https://godbolt.org/z/6E3YTP15j
>> 
>> Testing jcstress, tier1 and manually reading the generated assembly.
>> Doing additional testing, but RFR it now as it may need some consideration.
>> 
>> /Robbin
>
> Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments

src/hotspot/cpu/riscv/riscv.ad line 7888:

> 7886: // ZTSO
> 7887: 
> 7888: instruct no_membar_rvtso() %{

Can we rename this as `unnecessary_membar_rvtso` which is similar to other names like `unnecessary_membar_volatile_rvtso`? I personally don't really like the `no_` prefix.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24035#discussion_r1997834690