RFR: 8339771: RISC-V: Reduce icache flushes [v2]

[Robbin Ehn]

On Thu, 19 Sep 2024 06:42:29 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Sorry, now I realize what you are asking.
>
> The purpose is to make a store to instruction memory visible to all RISC-V harts.
> Check this code in file icache_riscv.cpp:
> 
> static int icache_flush(address addr, int lines, int magic) {
>   // To make a store to instruction memory visible to all RISC-V harts,
>   // the writing hart has to execute a data FENCE before requesting that
>   // all remote RISC-V harts execute a FENCE.I.
> 
>   // We need to make sure stores happens before the I/D cache synchronization.
>   __asm__ volatile("fence rw, rw" : : : "memory");                           <==============
> 
>   RiscvFlushIcache::flush((uintptr_t)addr, ((uintptr_t)lines) << ICache::log2_line_size);
> 
>   return magic;
> }
> 
> 
> PS: Here is what the spec says:
> 
> `FENCE.I does not ensure that other RISC-V harts’ instruction fetches will
> observe the local hart’s stores in a multiprocessor system. To make a store to instruction memory
> visible to all RISC-V harts, the writing hart also has to execute a data FENCE before requesting
> that all remote RISC-V harts execute a FENCE.I.`

Yes. The comment is a bit misleading.
A fence wr, wr is just for locally, on a hart, order instructions.
A fence wr wr do not flush a store from the store buffer, it just means that the hart must act such that the store appears to have happened before the syscall. Meaning the store can not happen after the syscall.

As we are not using the syscall:
Instead the other threads/harts must emit the fence.i them self, either by leaving a safepoint, hitting the patch_epoch_barrier and if they are moved to another hart.

Which means the store must happen before we leave the safepoint and disarming the nmethod barrier.
These already have store fences as we have a bunch of other stores which must be ordered.
For the nmethod barrier disarming is a full fence as the new epoch must happen before the disarm:


0: (nmethod barriers armed, implies storestore)
1: stores to instruction stream
2: store new patching epoch
3: storestore // the store to instruction and patching epoch must happen before disarm
4: disarm


If the stores to instruction stream and storing the new patching epoch happens in another order that is fine, as the critical thing is the disarm.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20913#discussion_r1766290017