RFR: 8310939: [c1] The visibility of write-volatile requires membar_volatile instead of membar
SUN Guoyun
duke at openjdk.org
Thu Jul 20 07:16:43 UTC 2023
On Wed, 19 Jul 2023 11:26:12 GMT, Andrew Haley <aph at openjdk.org> wrote:
> > Maybe I should write a performance test case for this PR.
>
> Seriously? You never measured the speed problem before writing a patch for it? How did you know it was a problem?
I've tested specjbb2015 and some related JMH tests, really haven't found a performance improvement.
But for the LoongArch architecture, StoreLoad is not equal to full barrier. And compared to C2's volatile write `MembarRelease-StoreNode-MembarVolatile`implementation, it is argued that c1 here membar() should be membar_storeload().
I previously thought that PPC, RISC-V's StoreLoad was not equivalent to full barrier, so I submitted the PR. Now it seems that only LoongArch has this problem, so should I make the following changes or abandon this PR?
<pre><code class="shell">
--- a/src/hotspot/share/gc/shared/c1/barrierSetC1.cpp
+++ b/src/hotspot/share/gc/shared/c1/barrierSetC1.cpp
@@ -161,7 +161,7 @@ void BarrierSetC1::store_at_resolved(LIRAccess& access, LIR_Opr value) {
}
if (is_volatile && !support_IRIW_for_not_multiple_copy_atomic_cpu) {
- __ membar();
+ __ membar_storeload();
}
}
</code></pre>
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14677#issuecomment-1643400051
More information about the hotspot-gc-dev
mailing list