8351949: RISC-V: Cleanup and enable store-load peephole for membars

Reviewed-by: fyang, fjiang, mli

Author: robehn

src/hotspot/cpu/riscv/assembler_riscv.hpp

@@ -815,7 +815,7 @@ class Assembler : public AbstractAssembler {
     emit(insn);
   }
 
- public:
+ protected:
 
   enum barrier {
     i = 0b1000, o = 0b0100, r = 0b0010, w = 0b0001,
@@ -846,6 +846,8 @@ class Assembler : public AbstractAssembler {
     emit(insn);
   }
 
+ public:
+
 #define INSN(NAME, op, funct3, funct7)                      \
   void NAME() {                                             \
     unsigned insn = 0;                                      \

src/hotspot/cpu/riscv/macroAssembler_riscv.cpp

@@ -183,7 +183,6 @@ void MacroAssembler::set_membar_kind(address addr, uint32_t order_kind) {
   Assembler::sd_instr(membar, insn);
 }
 
-
 static void pass_arg0(MacroAssembler* masm, Register arg) {
   if (c_rarg0 != arg) {
     masm->mv(c_rarg0, arg);
@@ -3556,6 +3555,14 @@ void MacroAssembler::lookup_virtual_method(Register recv_klass,
 }
 
 void MacroAssembler::membar(uint32_t order_constraint) {
+  if (UseZtso && ((order_constraint & StoreLoad) != StoreLoad)) {
+    // TSO allows for stores to be reordered after loads. When the compiler
+    // generates a fence to disallow that, we are required to generate the
+    // fence for correctness.
+    BLOCK_COMMENT("elided tso membar");
+    return;
+  }
+
   address prev = pc() - MacroAssembler::instruction_size;
   address last = code()->last_insn();
 
@@ -3564,15 +3571,14 @@ void MacroAssembler::membar(uint32_t order_constraint) {
     // can do this simply by ORing them together.
     set_membar_kind(prev, get_membar_kind(prev) | order_constraint);
     BLOCK_COMMENT("merged membar");
-  } else {
-    code()->set_last_insn(pc());
-
-    uint32_t predecessor = 0;
-    uint32_t successor = 0;
-
-    membar_mask_to_pred_succ(order_constraint, predecessor, successor);
-    fence(predecessor, successor);
+    return;
   }
+
+  code()->set_last_insn(pc());
+  uint32_t predecessor = 0;
+  uint32_t successor = 0;
+  membar_mask_to_pred_succ(order_constraint, predecessor, successor);
+  fence(predecessor, successor);
 }
 
 void MacroAssembler::cmodx_fence() {

src/hotspot/cpu/riscv/macroAssembler_riscv.hpp

@@ -417,15 +417,17 @@ class MacroAssembler: public Assembler {
   // We used four bit to indicate the read and write bits in the predecessors and successors,
   // and extended i for r, o for w if UseConservativeFence enabled.
   enum Membar_mask_bits {
-    StoreStore = 0b0101,               // (pred = ow   + succ =   ow)
-    LoadStore  = 0b1001,               // (pred = ir   + succ =   ow)
-    StoreLoad  = 0b0110,               // (pred = ow   + succ =   ir)
-    LoadLoad   = 0b1010,               // (pred = ir   + succ =   ir)
-    AnyAny     = LoadStore | StoreLoad // (pred = iorw + succ = iorw)
+    StoreStore = 0b0101,               // (pred = w   + succ = w)
+    LoadStore  = 0b1001,               // (pred = r   + succ = w)
+    StoreLoad  = 0b0110,               // (pred = w   + succ = r)
+    LoadLoad   = 0b1010,               // (pred = r   + succ = r)
+    AnyAny     = LoadStore | StoreLoad // (pred = rw  + succ = rw)
   };
 
   void membar(uint32_t order_constraint);
 
+ private:
+
   static void membar_mask_to_pred_succ(uint32_t order_constraint,
                                        uint32_t& predecessor, uint32_t& successor) {
     predecessor = (order_constraint >> 2) & 0x3;
@@ -437,33 +439,21 @@ class MacroAssembler: public Assembler {
     // 11(rw)-> 1111(iorw)
     if (UseConservativeFence) {
       predecessor |= predecessor << 2;
-      successor |= successor << 2;
+      successor   |= successor << 2;
     }
   }
 
   static int pred_succ_to_membar_mask(uint32_t predecessor, uint32_t successor) {
     return ((predecessor & 0x3) << 2) | (successor & 0x3);
   }
 
-  void fence(uint32_t predecessor, uint32_t successor) {
-    if (UseZtso) {
-      if ((pred_succ_to_membar_mask(predecessor, successor) & StoreLoad) == StoreLoad) {
-        // TSO allows for stores to be reordered after loads. When the compiler
-        // generates a fence to disallow that, we are required to generate the
-        // fence for correctness.
-        Assembler::fence(predecessor, successor);
-      } else {
-        // TSO guarantees other fences already.
-      }
-    } else {
-      // always generate fence for RVWMO
-      Assembler::fence(predecessor, successor);
-    }
-  }
+ public:
 
   void cmodx_fence();
 
   void pause() {
+    // Zihintpause
+    // PAUSE is encoded as a FENCE instruction with pred=W, succ=0, fm=0, rd=x0, and rs1=x0.
     Assembler::fence(w, 0);
   }
 

src/hotspot/cpu/riscv/riscv.ad

@@ -4397,6 +4397,12 @@ pipe_class pipe_slow()
   LDST   : MEM;
 %}
 
+// The real do-nothing guy
+pipe_class real_empty()
+%{
+    instruction_count(0);
+%}
+
 // Empty pipeline class
 pipe_class pipe_class_empty()
 %{
@@ -7902,113 +7908,153 @@ instruct xorL_reg_imm(iRegLNoSp dst, iRegL src1, immLAdd src2) %{
 // ============================================================================
 // MemBar Instruction
 
-instruct load_fence() %{
+// RVTSO
+
+instruct unnecessary_membar_rvtso() %{
+  predicate(UseZtso);
   match(LoadFence);
-  ins_cost(ALU_COST);
+  match(StoreFence);
+  match(StoreStoreFence);
+  match(MemBarAcquire);
+  match(MemBarRelease);
+  match(MemBarStoreStore);
+  match(MemBarAcquireLock);
+  match(MemBarReleaseLock);
 
-  format %{ "#@load_fence" %}
+  ins_cost(0);
+
+  size(0);
 
+  format %{ "#@unnecessary_membar_rvtso elided/tso (empty encoding)" %}
   ins_encode %{
-    __ membar(MacroAssembler::LoadLoad | MacroAssembler::LoadStore);
+    __ block_comment("unnecessary_membar_rvtso");
   %}
-  ins_pipe(pipe_serial);
+  ins_pipe(real_empty);
 %}
 
-instruct membar_acquire() %{
-  match(MemBarAcquire);
-  ins_cost(ALU_COST);
+instruct membar_volatile_rvtso() %{
+  predicate(UseZtso);
+  match(MemBarVolatile);
+  ins_cost(VOLATILE_REF_COST);
 
-  format %{ "#@membar_acquire\n\t"
-            "fence ir iorw" %}
+  format %{ "#@membar_volatile_rvtso\n\t"
+            "fence w, r"%}
 
   ins_encode %{
-    __ block_comment("membar_acquire");
-    __ membar(MacroAssembler::LoadLoad | MacroAssembler::LoadStore);
+    __ block_comment("membar_volatile_rvtso");
+    __ membar(MacroAssembler::StoreLoad);
   %}
 
-  ins_pipe(pipe_serial);
+  ins_pipe(pipe_slow);
 %}
 
-instruct membar_acquire_lock() %{
-  match(MemBarAcquireLock);
+instruct unnecessary_membar_volatile_rvtso() %{
+  predicate(UseZtso && Matcher::post_store_load_barrier(n));
+  match(MemBarVolatile);
   ins_cost(0);
 
-  format %{ "#@membar_acquire_lock (elided)" %}
-
+  size(0);
+  
+  format %{ "#@unnecessary_membar_volatile_rvtso (unnecessary so empty encoding)" %}
   ins_encode %{
-    __ block_comment("membar_acquire_lock (elided)");
+    __ block_comment("unnecessary_membar_volatile_rvtso");
   %}
-
-  ins_pipe(pipe_serial);
+  ins_pipe(real_empty);
 %}
 
-instruct store_fence() %{
-  match(StoreFence);
-  ins_cost(ALU_COST);
+// RVWMO
+
+instruct membar_aqcuire_rvwmo() %{
+  predicate(!UseZtso);
+  match(LoadFence);
+  match(MemBarAcquire);
+  ins_cost(VOLATILE_REF_COST);
 
-  format %{ "#@store_fence" %}
+  format %{ "#@membar_aqcuire_rvwmo\n\t"
+            "fence r, rw" %}
 
   ins_encode %{
-    __ membar(MacroAssembler::LoadStore | MacroAssembler::StoreStore);
+    __ block_comment("membar_aqcuire_rvwmo");
+    __ membar(MacroAssembler::LoadLoad | MacroAssembler::LoadStore);
   %}
   ins_pipe(pipe_serial);
 %}
 
-instruct membar_release() %{
+instruct membar_release_rvwmo() %{
+  predicate(!UseZtso);
+  match(StoreFence);
   match(MemBarRelease);
-  ins_cost(ALU_COST);
+  ins_cost(VOLATILE_REF_COST);
 
-  format %{ "#@membar_release\n\t"
-            "fence iorw ow" %}
+  format %{ "#@membar_release_rvwmo\n\t"
+            "fence rw, w" %}
 
   ins_encode %{
-    __ block_comment("membar_release");
+    __ block_comment("membar_release_rvwmo");
     __ membar(MacroAssembler::LoadStore | MacroAssembler::StoreStore);
   %}
   ins_pipe(pipe_serial);
 %}
 
-instruct membar_storestore() %{
+instruct membar_storestore_rvwmo() %{
+  predicate(!UseZtso);
   match(MemBarStoreStore);
   match(StoreStoreFence);
-  ins_cost(ALU_COST);
+  ins_cost(VOLATILE_REF_COST);
 
-  format %{ "MEMBAR-store-store\t#@membar_storestore" %}
+  format %{ "#@membar_storestore_rvwmo\n\t"
+            "fence w, w" %}
 
   ins_encode %{
     __ membar(MacroAssembler::StoreStore);
   %}
   ins_pipe(pipe_serial);
 %}
 
-instruct membar_release_lock() %{
-  match(MemBarReleaseLock);
-  ins_cost(0);
+instruct membar_volatile_rvwmo() %{
+  predicate(!UseZtso);
+  match(MemBarVolatile);
+  ins_cost(VOLATILE_REF_COST);
 
-  format %{ "#@membar_release_lock (elided)" %}
+  format %{ "#@membar_volatile_rvwmo\n\t"
+            "fence w, r"%}
 
   ins_encode %{
-    __ block_comment("membar_release_lock (elided)");
+    __ block_comment("membar_volatile_rvwmo");
+    __ membar(MacroAssembler::StoreLoad);
   %}
 
   ins_pipe(pipe_serial);
 %}
 
-instruct membar_volatile() %{
-  match(MemBarVolatile);
-  ins_cost(ALU_COST);
+instruct membar_lock_rvwmo() %{
+  predicate(!UseZtso);
+  match(MemBarAcquireLock);
+  match(MemBarReleaseLock);
+  ins_cost(0);
 
-  format %{ "#@membar_volatile\n\t"
-             "fence iorw iorw"%}
+  format %{ "#@membar_lock_rvwmo (elided)" %}
 
   ins_encode %{
-    __ block_comment("membar_volatile");
-    __ membar(MacroAssembler::StoreLoad);
+    __ block_comment("membar_lock_rvwmo (elided)");
   %}
 
   ins_pipe(pipe_serial);
 %}
 
+instruct unnecessary_membar_volatile_rvwmo() %{
+  predicate(!UseZtso && Matcher::post_store_load_barrier(n));
+  match(MemBarVolatile);
+  ins_cost(0);
+
+  size(0);
+  format %{ "#@unnecessary_membar_volatile_rvwmo (unnecessary so empty encoding)" %}
+  ins_encode %{
+    __ block_comment("unnecessary_membar_volatile_rvwmo");
+  %}
+  ins_pipe(real_empty);
+%}
+
 instruct spin_wait() %{
   predicate(UseZihintpause);
   match(OnSpinWait);