[RISCV] Avoid emitting hardware fences for singlethread fences

[reames]

Summary

singlethread fences only synchronize with code running on the same hardware thread (i.e. signal handlers). Because of this, we need to prevent instruction reordering, but do not need to emit hardware fence instructions.

The implementation strategy here matches many other backends. The main motivation of this patch is to introduce the MEMBARRIER node and get some test coverage for it.

[jrtc27]

If every backend is doing it shouldn't this be a generic thing rather than copy/pasted to every backend?

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1849

This isn't standard naming for pseudos, but maybe other backends do it this way too?...

[reames]

In D141311#4037290, @jrtc27 wrote:

If every backend is doing it shouldn't this be a generic thing rather than copy/pasted to every backend?

Agreed. I looked briefly at doing this. I think we could probably share the SDAG pieces; the pseudos look less obvious. Happy to take a pass at this, but I'd prefer that be non-blocking for this change. You okay with that?

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1849

I honestly copied this from X86 without trying to understand it closely. If you have suggestions, happy to apply them.

[craig.topper]

There was an attempt to make a target independent compiler barrier here. https://reviews.llvm.org/D92842 I haven't re-read through the discussion yet.

[jrtc27]

In D141311#4037302, @reames wrote:

In D141311#4037290, @jrtc27 wrote:

If every backend is doing it shouldn't this be a generic thing rather than copy/pasted to every backend?

Agreed. I looked briefly at doing this. I think we could probably share the SDAG pieces; the pseudos look less obvious. Happy to take a pass at this, but I'd prefer that be non-blocking for this change. You okay with that?

Yeah, I just wish it were done that way from the start :( busywork as a result

llvm/lib/Target/RISCV/RISCVInstrInfo.td

1849

Normally it'd be riscv_membarrier defined at the top of the file and PseudoMEMBARRIER defined here (or in the instructions section)

[reames]

In D141311#4037335, @jrtc27 wrote:

In D141311#4037302, @reames wrote:

In D141311#4037290, @jrtc27 wrote:

If every backend is doing it shouldn't this be a generic thing rather than copy/pasted to every backend?

Agreed. I looked briefly at doing this. I think we could probably share the SDAG pieces; the pseudos look less obvious. Happy to take a pass at this, but I'd prefer that be non-blocking for this change. You okay with that?

Yeah, I just wish it were done that way from the start :( busywork as a result

The SDAG part turned out to be easier than I'd realized, and is now posted: https://reviews.llvm.org/D141317

I'm going to rebase this patch once that one lands.

[reames]

Rebase

[asb]

LGTM - two minor nits inline.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

3677

handles => handlers

3687

Accidental extra whitespace?

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

if (emitPseudoExpansionLowering(*OutStreamer, MI))
     return;
 
   MCInst TmpInst;
 
   if (MI->getOpcode() == RISCV::HWASAN_CHECK_MEMACCESS_SHORTGRANULES) {
     LowerHWASAN_CHECK_MEMACCESS(*MI);
     return;
   }
+  if (MI->getOpcode() == RISCV::Int_MemBarrier) {
+    OutStreamer->emitRawComment("MEMBARRIER");
+    return;
+  }
 
   if (!lowerRISCVMachineInstrToMCInst(MI, TmpInst, *this))
     EmitToStreamer(*OutStreamer, TmpInst);
 }
 
 bool RISCVAsmPrinter::PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
                                       const char *ExtraCode, raw_ostream &OS) {
   // First try the generic code, which knows about modifiers like 'c' and 'n'.

llvm/lib/Target/RISCV/RISCVISelLowering.h

enum NodeType : unsigned {
   // chain pointer.
   WRITE_CSR,
   // Read and write value of CSR.
   // The first operand is a chain pointer, the second specifies address of the
   // required CSR and the third is the value to write. Two results are produced,
   // the value read before the modification and the new chain pointer.
   SWAP_CSR,
 
+  // Compiler barrier only; generate a no-op.
+  MEMBARRIER,
+
   // FP to 32 bit int conversions for RV64. These are used to keep track of the
   // result being sign extended to 64 bit. These saturate out of range inputs.
   STRICT_FCVT_W_RV64 = ISD::FIRST_TARGET_STRICTFP_OPCODE,
   STRICT_FCVT_WU_RV64,
 
   // WARNING: Do not add anything in the end unless you want the node to
   // have memop! In fact, starting from FIRST_TARGET_MEMORY_OPCODE all
   // opcodes will be thought as target memory ops!

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

if (Subtarget.hasStdExtA()) {
     setMaxAtomicSizeInBitsSupported(Subtarget.getXLen());
     setMinCmpXchgSizeInBits(32);
   } else if (Subtarget.hasForcedAtomics()) {
     setMaxAtomicSizeInBitsSupported(Subtarget.getXLen());
   } else {
     setMaxAtomicSizeInBitsSupported(0);
   }
 
+  setOperationAction(ISD::ATOMIC_FENCE, MVT::Other, Custom);
+
   setBooleanContents(ZeroOrOneBooleanContent);
 
   if (Subtarget.hasVInstructions()) {
     setBooleanVectorContents(ZeroOrOneBooleanContent);
 
     setOperationAction(ISD::VSCALE, XLenVT, Custom);
 
     // RVV intrinsics may have illegal operands.
RISCVMatInt::InstSeq Seq =
       RISCVMatInt::generateInstSeq(Imm, Subtarget.getFeatureBits());
   if (Seq.size() <= Subtarget.getMaxBuildIntsCost())
     return Op;
 
   // Expand to a constant pool using the default expansion code.
   return SDValue();
 }
 
+static SDValue LowerATOMIC_FENCE(SDValue Op, SelectionDAG &DAG) {
+  SDLoc dl(Op);
+  SyncScope::ID FenceSSID =
+      static_cast<SyncScope::ID>(Op.getConstantOperandVal(2));
+
+  // singlethread fences only synchronize with signal handles on the same
+  // thread and thus only need to preserve instruction order, not actually
+  // enforce memory ordering.
+  if (FenceSSID == SyncScope::SingleThread)
+    // MEMBARRIER is a compiler barrier; it codegens to a no-op.
+    return DAG.getNode(RISCVISD::MEMBARRIER, dl, MVT::Other, Op.getOperand(0));
+
+  return Op;
+}
+
+
 SDValue RISCVTargetLowering::LowerOperation(SDValue Op,
                                             SelectionDAG &DAG) const {
   switch (Op.getOpcode()) {
   default:
     report_fatal_error("unimplemented operand");
+  case ISD::ATOMIC_FENCE:
+    return LowerATOMIC_FENCE(Op, DAG);
   case ISD::GlobalAddress:
     return lowerGlobalAddress(Op, DAG);
   case ISD::BlockAddress:
     return lowerBlockAddress(Op, DAG);
   case ISD::ConstantPool:
     return lowerConstantPool(Op, DAG);
   case ISD::JumpTable:
     return lowerJumpTable(Op, DAG);
#define NODE_NAME_CASE(NODE)                                                   \
   NODE_NAME_CASE(VRGATHEREI16_VV_VL)
   NODE_NAME_CASE(VSEXT_VL)
   NODE_NAME_CASE(VZEXT_VL)
   NODE_NAME_CASE(VCPOP_VL)
   NODE_NAME_CASE(VFIRST_VL)
   NODE_NAME_CASE(READ_CSR)
   NODE_NAME_CASE(WRITE_CSR)
   NODE_NAME_CASE(SWAP_CSR)
+  NODE_NAME_CASE(MEMBARRIER)
   }
   // clang-format on
   return nullptr;
 #undef NODE_NAME_CASE
 }
 
 /// getConstraintType - Given a constraint letter, return the type of
 /// constraint it is for this target.

llvm/lib/Target/RISCV/RISCVInstrInfo.td

 
 let Predicates = [IsRV64] in {
 // Select W instructions if only the lower 32-bits of the result are used.
 def : Pat<(binop_allwusers<add> GPR:$rs1, (AddiPair:$rs2)),
           (ADDIW (ADDIW GPR:$rs1, (AddiPairImmLarge AddiPair:$rs2)),
                  (AddiPairImmSmall AddiPair:$rs2))>;
 }
 
+def SDT_RISCVMEMBARRIER : SDTypeProfile<0, 0, []>;
+def RISCVMemBarrier : SDNode<"RISCVISD::MEMBARRIER", SDT_RISCVMEMBARRIER,
+                             [SDNPHasChain,SDNPSideEffect]>;
+
+let hasSideEffects = 1, isMeta = 1 in
+def Int_MemBarrier : Pseudo<(outs), (ins),
+                     [(RISCVMemBarrier)]>;
+
 //===----------------------------------------------------------------------===//
 // Standard extensions
 //===----------------------------------------------------------------------===//
 
 include "RISCVInstrInfoM.td"
 include "RISCVInstrInfoA.td"
 include "RISCVInstrInfoF.td"
 include "RISCVInstrInfoD.td"

llvm/test/CodeGen/RISCV/atomic-fence.ll

 ; CHECK-NEXT:    ret
   fence seq_cst
   ret void
 }
 
 define void @fence_singlethread_acquire() nounwind {
 ; CHECK-LABEL: fence_singlethread_acquire:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    fence r, rw
+; CHECK-NEXT:    #MEMBARRIER
 ; CHECK-NEXT:    ret
   fence syncscope("singlethread") acquire
   ret void
 }
 
 define void @fence_singlethread_release() nounwind {
 ; CHECK-LABEL: fence_singlethread_release:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    fence rw, w
+; CHECK-NEXT:    #MEMBARRIER
 ; CHECK-NEXT:    ret
   fence syncscope("singlethread") release
   ret void
 }
 
 define void @fence_singlethread_acq_rel() nounwind {
 ; CHECK-LABEL: fence_singlethread_acq_rel:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    fence.tso
+; CHECK-NEXT:    #MEMBARRIER
 ; CHECK-NEXT:    ret
   fence syncscope("singlethread") acq_rel
   ret void
 }
 
 define void @fence_singlethread_seq_cst() nounwind {
 ; CHECK-LABEL: fence_singlethread_seq_cst:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    fence rw, rw
+; CHECK-NEXT:    #MEMBARRIER
 ; CHECK-NEXT:    ret
   fence syncscope("singlethread") seq_cst
   ret void
 }