[RISCV] Implement a proposed mapping for Ztso

preames · preames · commit be6fe9567362 · 2023-02-09T12:00:05.000-08:00
This change implements a proposed lowering from LLVM's memory model to the TSO memory model defined by the Ztso extension. Selecting the proposed mapping turns out to be an involved conversation that really didn't fit within a review description, so let me refer you to https://github.com/preames/public-notes/blob/master/riscv-tso-mappings.rst. This review implements the WMO compatible variant (the proposed one in that document). Ztso is currently accepted as an experimental extension in LLVM. Despite the fact the extension was recently ratified, I think we need to leave it as experimental until we have wide agreement on the chosen mapping for ABI purposes. I need to note that the current in-tree implementation defaults to generating WMO compatible fences. This is entirely compatible with the proposed mapping in this patch, but is unfortunately not compatible with the major alternative. The in tree implementation is explicitly experimental so the impact of this is limited, but it is worth calling out that if settle on the alternative we will have a minor ABI break. My apologies for not calling this out in the original patch; I had not realized at the time that one of our realistic choices for mappings wouldn't be WMO compatible. This patch only contains the changes for load/store and fence. That is, it does not change the lowering for atomicrmw operations. This is a sound thing to do under the proposed mapping since the existing WMO mappings remain compatible. I do plan to change these; I'm just working incrementally. Differential Revision: https://reviews.llvm.org/D143076
diff --git a/llvm/docs/RISCVUsage.rst b/llvm/docs/RISCVUsage.rst
@@ -151,7 +151,7 @@ The primary goal of experimental support is to assist in the process of ratifica
   LLVM implements the `0.2 draft specification <https://github.com/riscv/riscv-isa-manual/releases/tag/draft-20220831-bf5a151>`_.
 
 ``experimental-ztso``
-  LLVM implements the `v0.1 proposed specification <https://github.com/riscv/riscv-isa-manual/releases/download/draft-20220723-10eea63/riscv-spec.pdf>`_ (see Chapter 25).  Using will set appropriate ELF flags and attributes, but does not yet change code generation.
+  LLVM implements the `v0.1 proposed specification <https://github.com/riscv/riscv-isa-manual/releases/download/draft-20220723-10eea63/riscv-spec.pdf>`_ (see Chapter 25).  The mapping from the C/C++ memory model to Ztso has not yet been ratified in any standards document.  There are multiple possible mappings, and they are *not* mutually ABI compatible.  The mapping LLVM implements is ABI compatible with the default WMO mapping.  This mapping may change and there is *explicitly* no ABI stability offered while the extension remains in experimental status.  User beware.
 
 ``experimental-zvfh``
   LLVM implements `this draft text <https://github.com/riscv/riscv-v-spec/pull/780>`_.
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -3721,11 +3721,25 @@ static SDValue lowerConstant(SDValue Op, SelectionDAG &DAG,
   return SDValue();
 }
 
-static SDValue LowerATOMIC_FENCE(SDValue Op, SelectionDAG &DAG) {
+static SDValue LowerATOMIC_FENCE(SDValue Op, SelectionDAG &DAG,
+                                 const RISCVSubtarget &Subtarget) {
   SDLoc dl(Op);
+  AtomicOrdering FenceOrdering =
+      static_cast<AtomicOrdering>(Op.getConstantOperandVal(1));
   SyncScope::ID FenceSSID =
       static_cast<SyncScope::ID>(Op.getConstantOperandVal(2));
 
+  if (Subtarget.hasStdExtZtso()) {
+    // The only fence that needs an instruction is a sequentially-consistent
+    // cross-thread fence.
+    if (FenceOrdering == AtomicOrdering::SequentiallyConsistent &&
+        FenceSSID == SyncScope::System)
+      return Op;
+
+    // MEMBARRIER is a compiler barrier; it codegens to a no-op.
+    return DAG.getNode(ISD::MEMBARRIER, dl, MVT::Other, Op.getOperand(0));
+  }
+
   // singlethread fences only synchronize with signal handlers on the same
   // thread and thus only need to preserve instruction order, not actually
   // enforce memory ordering.
@@ -3742,7 +3756,7 @@ SDValue RISCVTargetLowering::LowerOperation(SDValue Op,
   default:
     report_fatal_error("unimplemented operand");
   case ISD::ATOMIC_FENCE:
-    return LowerATOMIC_FENCE(Op, DAG);
+    return LowerATOMIC_FENCE(Op, DAG, Subtarget);
   case ISD::GlobalAddress:
     return lowerGlobalAddress(Op, DAG);
   case ISD::BlockAddress:
@@ -13759,6 +13773,12 @@ void RISCVTargetLowering::LowerAsmOperandForConstraint(
 Instruction *RISCVTargetLowering::emitLeadingFence(IRBuilderBase &Builder,
                                                    Instruction *Inst,
                                                    AtomicOrdering Ord) const {
+  if (Subtarget.hasStdExtZtso()) {
+    if (isa<LoadInst>(Inst) && Ord == AtomicOrdering::SequentiallyConsistent)
+      return Builder.CreateFence(Ord);
+    return nullptr;
+  }
+
   if (isa<LoadInst>(Inst) && Ord == AtomicOrdering::SequentiallyConsistent)
     return Builder.CreateFence(Ord);
   if (isa<StoreInst>(Inst) && isReleaseOrStronger(Ord))
@@ -13769,6 +13789,9 @@ Instruction *RISCVTargetLowering::emitLeadingFence(IRBuilderBase &Builder,
 Instruction *RISCVTargetLowering::emitTrailingFence(IRBuilderBase &Builder,
                                                     Instruction *Inst,
                                                     AtomicOrdering Ord) const {
+  if (Subtarget.hasStdExtZtso())
+    return nullptr;
+
   if (isa<LoadInst>(Inst) && isAcquireOrStronger(Ord))
     return Builder.CreateFence(AtomicOrdering::Acquire);
   return nullptr;
diff --git a/llvm/test/CodeGen/RISCV/atomic-fence.ll b/llvm/test/CodeGen/RISCV/atomic-fence.ll
@@ -1,40 +1,55 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
-; RUN:   | FileCheck %s
+; RUN:   | FileCheck --check-prefixes=CHECK,WMO %s
 ; RUN: llc -mtriple=riscv32 -mattr=+a -verify-machineinstrs < %s \
-; RUN:   | FileCheck %s
+; RUN:   | FileCheck --check-prefixes=CHECK,WMO %s
 ; RUN: llc -mtriple=riscv32 -mattr=+a,+experimental-ztso -verify-machineinstrs < %s \
-; RUN:   | FileCheck %s
+; RUN:   | FileCheck --check-prefixes=CHECK,TSO %s
 ; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
-; RUN:   | FileCheck %s
+; RUN:   | FileCheck --check-prefixes=CHECK,WMO %s
 ; RUN: llc -mtriple=riscv64 -mattr=+a -verify-machineinstrs < %s \
-; RUN:   | FileCheck %s
+; RUN:   | FileCheck --check-prefixes=CHECK,WMO %s
 ; RUN: llc -mtriple=riscv64 -mattr=+a,+experimental-ztso -verify-machineinstrs < %s \
-; RUN:   | FileCheck %s
+; RUN:   | FileCheck --check-prefixes=CHECK,TSO %s
 
 define void @fence_acquire() nounwind {
-; CHECK-LABEL: fence_acquire:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    fence r, rw
-; CHECK-NEXT:    ret
+; WMO-LABEL: fence_acquire:
+; WMO:       # %bb.0:
+; WMO-NEXT:    fence r, rw
+; WMO-NEXT:    ret
+;
+; TSO-LABEL: fence_acquire:
+; TSO:       # %bb.0:
+; TSO-NEXT:    #MEMBARRIER
+; TSO-NEXT:    ret
   fence acquire
   ret void
 }
 
 define void @fence_release() nounwind {
-; CHECK-LABEL: fence_release:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    fence rw, w
-; CHECK-NEXT:    ret
+; WMO-LABEL: fence_release:
+; WMO:       # %bb.0:
+; WMO-NEXT:    fence rw, w
+; WMO-NEXT:    ret
+;
+; TSO-LABEL: fence_release:
+; TSO:       # %bb.0:
+; TSO-NEXT:    #MEMBARRIER
+; TSO-NEXT:    ret
   fence release
   ret void
 }
 
 define void @fence_acq_rel() nounwind {
-; CHECK-LABEL: fence_acq_rel:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    fence.tso
-; CHECK-NEXT:    ret
+; WMO-LABEL: fence_acq_rel:
+; WMO:       # %bb.0:
+; WMO-NEXT:    fence.tso
+; WMO-NEXT:    ret
+;
+; TSO-LABEL: fence_acq_rel:
+; TSO:       # %bb.0:
+; TSO-NEXT:    #MEMBARRIER
+; TSO-NEXT:    ret
   fence acq_rel
   ret void
 }
diff --git a/llvm/test/CodeGen/RISCV/atomic-load-store.ll b/llvm/test/CodeGen/RISCV/atomic-load-store.ll
