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