[CIR] Change AtomicFenceOp's syncscope to OptionalAttr (#1429)

FantasqueX · lanza · commit 0f231b13c590 · 2025-03-17T19:19:56.000-07:00
This PR make CIR's AtomicFenceOp similar to MLIR LLVMIR's FenceOp. MLIR LLVMIR FenceOp: https://github.com/llvm/clangir/blob/0bedc285dc6fbd8486877887939c742c2ddaecfa/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td#L1925-L1947
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -5706,17 +5706,17 @@ def AtomicFence : CIR_Op<"atomic.fence"> {
 
     Example:
     ```mlir
-      cir.atomic.fence system seq_cst
-      cir.atomic.fence single_thread seq_cst
+      cir.atomic.fence syncscope(system) seq_cst
+      cir.atomic.fence syncscope(single_thread) seq_cst
     ```
 
   }];
+  let arguments = (ins Arg<MemOrder, "memory order">:$ordering,
+                       OptionalAttr<MemScopeKind>:$syncscope);
   let results = (outs);
-  let arguments = (ins Arg<MemScopeKind, "sync scope">:$sync_scope,
-                       Arg<MemOrder, "memory order">:$ordering);
 
   let assemblyFormat = [{
-    $sync_scope $ordering attr-dict
+    (`syncscope` `(` $syncscope^ `)`)? $ordering attr-dict
   }];
 
   let hasVerifier = 0;
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
@@ -365,8 +365,9 @@ static mlir::Value makeAtomicFenceValue(CIRGenFunction &cgf,
     cir::MemOrder ordering =
         static_cast<cir::MemOrder>(constOrderingAttr.getUInt());
 
-    builder.create<cir::AtomicFence>(cgf.getLoc(expr->getSourceRange()),
-                                     syncScope, ordering);
+    builder.create<cir::AtomicFence>(
+        cgf.getLoc(expr->getSourceRange()), ordering,
+        MemScopeKindAttr::get(&cgf.getMLIRContext(), syncScope));
   }
 
   return mlir::Value();
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -405,6 +405,14 @@ static mlir::Value emitToMemory(mlir::ConversionPatternRewriter &rewriter,
   return value;
 }
 
+std::optional<llvm::StringRef>
+getLLVMSyncScope(std::optional<cir::MemScopeKind> syncScope) {
+  if (syncScope.has_value())
+    return syncScope.value() == cir::MemScopeKind::MemScope_SingleThread
+               ? "singlethread"
+               : "";
+  return std::nullopt;
+}
 } // namespace
 
 //===----------------------------------------------------------------------===//
@@ -3217,11 +3225,6 @@ mlir::LLVM::AtomicOrdering getLLVMAtomicOrder(cir::MemOrder memo) {
   llvm_unreachable("shouldn't get here");
 }
 
-llvm::StringRef getLLVMSyncScope(cir::MemScopeKind syncScope) {
-  return syncScope == cir::MemScopeKind::MemScope_SingleThread ? "singlethread"
-                                                               : "";
-}
-
 mlir::LogicalResult CIRToLLVMAtomicCmpXchgLowering::matchAndRewrite(
     cir::AtomicCmpXchg op, OpAdaptor adaptor,
     mlir::ConversionPatternRewriter &rewriter) const {
@@ -3232,8 +3235,7 @@ mlir::LogicalResult CIRToLLVMAtomicCmpXchgLowering::matchAndRewrite(
       op.getLoc(), adaptor.getPtr(), expected, desired,
       getLLVMAtomicOrder(adaptor.getSuccOrder()),
       getLLVMAtomicOrder(adaptor.getFailOrder()));
-  if (const auto ss = adaptor.getSyncscope(); ss.has_value())
-    cmpxchg.setSyncscope(getLLVMSyncScope(ss.value()));
+  cmpxchg.setSyncscope(getLLVMSyncScope(adaptor.getSyncscope()));
   cmpxchg.setAlignment(adaptor.getAlignment());
   cmpxchg.setWeak(adaptor.getWeak());
   cmpxchg.setVolatile_(adaptor.getIsVolatile());
@@ -3395,10 +3397,11 @@ mlir::LogicalResult CIRToLLVMAtomicFenceLowering::matchAndRewrite(
     cir::AtomicFence op, OpAdaptor adaptor,
     mlir::ConversionPatternRewriter &rewriter) const {
   auto llvmOrder = getLLVMAtomicOrder(adaptor.getOrdering());
-  auto llvmSyncScope = getLLVMSyncScope(adaptor.getSyncScope());
 
-  rewriter.replaceOpWithNewOp<mlir::LLVM::FenceOp>(op, llvmOrder,
-                                                   llvmSyncScope);
+  auto fence = rewriter.create<mlir::LLVM::FenceOp>(op.getLoc(), llvmOrder);
+  fence.setSyncscope(getLLVMSyncScope(adaptor.getSyncscope()));
+
+  rewriter.replaceOp(op, fence);
 
   return mlir::success();
 }
diff --git a/clang/test/CIR/CodeGen/atomic-thread-fence.c b/clang/test/CIR/CodeGen/atomic-thread-fence.c
@@ -16,7 +16,7 @@ void applyThreadFence() {
 }
 
 // CIR-LABEL: @applyThreadFence
-// CIR:   cir.atomic.fence system seq_cst
+// CIR:   cir.atomic.fence syncscope(system) seq_cst
 // CIR:   cir.return
 
 // LLVM-LABEL: @applyThreadFence
@@ -27,7 +27,7 @@ void applySignalFence() {
   __atomic_signal_fence(__ATOMIC_SEQ_CST);
 }
 // CIR-LABEL: @applySignalFence
-// CIR:    cir.atomic.fence single_thread seq_cst
+// CIR:    cir.atomic.fence syncscope(single_thread) seq_cst
 // CIR:    cir.return
 
 // LLVM-LABEL: @applySignalFence
@@ -40,7 +40,7 @@ void modifyWithThreadFence(DataPtr d) {
 }
 // CIR-LABEL: @modifyWithThreadFence
 // CIR:    %[[DATA:.*]] = cir.alloca !cir.ptr<!ty_Data>, !cir.ptr<!cir.ptr<!ty_Data>>, ["d", init] {alignment = 8 : i64}
-// CIR:    cir.atomic.fence system seq_cst
+// CIR:    cir.atomic.fence syncscope(system) seq_cst
 // CIR:    %[[VAL_42:.*]] = cir.const #cir.int<42> : !s32i
 // CIR:    %[[LOAD_DATA:.*]] = cir.load %[[DATA]] : !cir.ptr<!cir.ptr<!ty_Data>>, !cir.ptr<!ty_Data>
 // CIR:    %[[DATA_VALUE:.*]] = cir.get_member %[[LOAD_DATA]][0] {name = "value"} : !cir.ptr<!ty_Data> -> !cir.ptr<!s32i>
@@ -61,7 +61,7 @@ void modifyWithSignalFence(DataPtr d) {
 }
 // CIR-LABEL: @modifyWithSignalFence
 // CIR:    %[[DATA:.*]] = cir.alloca !cir.ptr<!ty_Data>, !cir.ptr<!cir.ptr<!ty_Data>>, ["d", init] {alignment = 8 : i64}
-// CIR:    cir.atomic.fence single_thread seq_cst
+// CIR:    cir.atomic.fence syncscope(single_thread) seq_cst
 // CIR:    %[[VAL_42:.*]] = cir.const #cir.int<24> : !s32i
 // CIR:    %[[LOAD_DATA:.*]] = cir.load %[[DATA]] : !cir.ptr<!cir.ptr<!ty_Data>>, !cir.ptr<!ty_Data>
 // CIR:    %[[DATA_VALUE:.*]] = cir.get_member %[[LOAD_DATA]][0] {name = "value"} : !cir.ptr<!ty_Data> -> !cir.ptr<!s32i>
@@ -83,7 +83,7 @@ void loadWithThreadFence(DataPtr d) {
 // CIR-LABEL: @loadWithThreadFence
 // CIR:    %[[DATA:.*]] = cir.alloca !cir.ptr<!ty_Data>, !cir.ptr<!cir.ptr<!ty_Data>>, ["d", init] {alignment = 8 : i64}
 // CIR:    %[[ATOMIC_TEMP:.*]] = cir.alloca !cir.ptr<!void>, !cir.ptr<!cir.ptr<!void>>, ["atomic-temp"] {alignment = 8 : i64}
-// CIR:    cir.atomic.fence system seq_cst
+// CIR:    cir.atomic.fence syncscope(system) seq_cst
 // CIR:    %[[LOAD_DATA:.*]] = cir.load %[[DATA]] : !cir.ptr<!cir.ptr<!ty_Data>>, !cir.ptr<!ty_Data>
 // CIR:    %[[DATA_VALUE:.*]] = cir.get_member %[[LOAD_DATA]][1] {name = "ptr"} : !cir.ptr<!ty_Data> -> !cir.ptr<!cir.ptr<!void>>
 // CIR:    %[[CASTED_DATA_VALUE:.*]] = cir.cast(bitcast, %[[DATA_VALUE]] : !cir.ptr<!cir.ptr<!void>>), !cir.ptr<!u64i>
@@ -112,7 +112,7 @@ void loadWithSignalFence(DataPtr d) {
 // CIR-LABEL: @loadWithSignalFence
 // CIR:    %[[DATA:.*]] = cir.alloca !cir.ptr<!ty_Data>, !cir.ptr<!cir.ptr<!ty_Data>>, ["d", init] {alignment = 8 : i64}
 // CIR:    %[[ATOMIC_TEMP:.*]] = cir.alloca !cir.ptr<!void>, !cir.ptr<!cir.ptr<!void>>, ["atomic-temp"] {alignment = 8 : i64}
-// CIR:    cir.atomic.fence single_thread seq_cst
+// CIR:    cir.atomic.fence syncscope(single_thread) seq_cst
 // CIR:    %[[LOAD_DATA:.*]] = cir.load %[[DATA]] : !cir.ptr<!cir.ptr<!ty_Data>>, !cir.ptr<!ty_Data>
 // CIR:    %[[DATA_PTR:.*]] = cir.get_member %[[LOAD_DATA]][1] {name = "ptr"} : !cir.ptr<!ty_Data> -> !cir.ptr<!cir.ptr<!void>>
 // CIR:    %[[CASTED_DATA_PTR:.*]] = cir.cast(bitcast, %[[DATA_PTR]] : !cir.ptr<!cir.ptr<!void>>), !cir.ptr<!u64i>
