[CIR] Add syncscope support for atomic load operations (#1958)

Fix #1868

Author: jiang1997

clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

@@ -175,8 +175,8 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
     return cir::LoadOp::create(*this, loc, ptr, /*isDeref=*/false, isVolatile,
                                isNontemporal,
                                /*alignment=*/alignmentAttr,
-                               /*mem_order=*/
-                               cir::MemOrderAttr{},
+                               /*sync_scope=*/cir::SyncScopeKindAttr{},
+                               /*mem_order=*/cir::MemOrderAttr{},
                                /*tbaa=*/cir::TBAAAttr{});
   }
 

clang/include/clang/CIR/Dialect/IR/CIROps.td

@@ -538,6 +538,15 @@ def CIR_MemOrder : CIR_I32EnumAttr<
     I32EnumAttrCase<"SequentiallyConsistent", 5, "seq_cst">
 ]>;
 
+//===----------------------------------------------------------------------===//
+// C/C++ sync scope definitions
+//===----------------------------------------------------------------------===//
+
+def CIR_SyncScopeKind : CIR_I32EnumAttr<"SyncScopeKind", "sync scope kind", [
+  I32EnumAttrCase<"SingleThread", 0, "single_thread">,
+  I32EnumAttrCase<"System", 1, "system">
+]>;
+
 //===----------------------------------------------------------------------===//
 // AllocaOp
 //===----------------------------------------------------------------------===//
@@ -675,7 +684,8 @@ def CIR_LoadOp : CIR_Op<"load", [
     %4 = cir.load volatile %0 : !cir.ptr<i32>, i32
 
     // Others
-    %x = cir.load align(16) atomic(seq_cst) %0 : !cir.ptr<i32>, i32
+    %x = cir.load align(16) syncscope(single_thread) atomic(seq_cst)
+      %0 : !cir.ptr<i32>, i32
     ```
   }];
 
@@ -684,6 +694,7 @@ def CIR_LoadOp : CIR_Op<"load", [
                        UnitAttr:$is_volatile,
                        UnitAttr:$is_nontemporal,
                        OptionalAttr<I64Attr>:$alignment,
+                       OptionalAttr<CIR_SyncScopeKind>:$sync_scope,
                        OptionalAttr<CIR_MemOrder>:$mem_order,
                        OptionalAttr<CIR_AnyTBAAAttr>:$tbaa
                        );
@@ -694,6 +705,7 @@ def CIR_LoadOp : CIR_Op<"load", [
     (`volatile` $is_volatile^)?
     (`nontemporal` $is_nontemporal^)?
     (`align` `(` $alignment^ `)`)?
+    (`syncscope` `(` $sync_scope^ `)`)?
     (`atomic` `(` $mem_order^ `)`)?
     $addr `:` qualified(type($addr)) `,` type($result) attr-dict
     (`tbaa` `(` $tbaa^ `)`)?
@@ -703,7 +715,8 @@ def CIR_LoadOp : CIR_Op<"load", [
     // TODO(CIR): The final interface here should include an argument for the
     // SyncScope::ID.
     // This should be used over the ODS generated setMemOrder.
-    void setAtomic(cir::MemOrder order);
+    void setAtomic(cir::MemOrder order,
+                   cir::SyncScopeKind scope);
   }];
 
   // FIXME: add verifier.
@@ -6091,11 +6104,6 @@ def CIR_AtomicXchg : CIR_Op<"atomic.xchg", [
   let hasVerifier = 1;
 }
 
-def CIR_MemScopeKind : CIR_I32EnumAttr<"MemScopeKind", "memory scope kind", [
-  I32EnumAttrCase<"SingleThread", 0, "single_thread">,
-  I32EnumAttrCase<"System", 1, "system">
-]>;
-
 def CIR_AtomicCmpXchg : CIR_Op<"atomic.cmp_xchg", [
   AllTypesMatch<["old", "expected", "desired"]>
 ]> {
@@ -6119,7 +6127,7 @@ def CIR_AtomicCmpXchg : CIR_Op<"atomic.cmp_xchg", [
                        CIR_AnyType:$desired,
                        Arg<CIR_MemOrder, "success memory order">:$succ_order,
                        Arg<CIR_MemOrder, "failure memory order">:$fail_order,
-                       OptionalAttr<CIR_MemScopeKind>:$syncscope,
+                       OptionalAttr<CIR_SyncScopeKind>:$sync_scope,
                        OptionalAttr<I64Attr>:$alignment,
                        UnitAttr:$weak,
                        UnitAttr:$is_volatile);
@@ -6132,7 +6140,7 @@ def CIR_AtomicCmpXchg : CIR_Op<"atomic.cmp_xchg", [
     `success` `=`  $succ_order `,`
     `failure` `=`  $fail_order
     `)`
-    (`syncscope` `(` $syncscope^ `)`)?
+    (`syncscope` `(` $sync_scope^ `)`)?
     (`align` `(` $alignment^ `)`)?
     (`weak` $weak^)?
     (`volatile` $is_volatile^)?
@@ -6162,7 +6170,7 @@ def CIR_AtomicTestAndSetOp : CIR_Op<"atomic.test_and_set"> {
   let arguments = (ins
     Arg<CIR_PtrToType<CIR_SInt8>, "", [MemRead, MemWrite]>:$ptr,
     Arg<CIR_MemOrder, "memory order">:$mem_order,
-    OptionalAttr<CIR_MemScopeKind>:$syncscope,
+    OptionalAttr<CIR_SyncScopeKind>:$sync_scope,
     OptionalAttr<I64Attr>:$alignment,
     UnitAttr:$is_volatile);
 
@@ -6193,7 +6201,7 @@ def CIR_AtomicClearOp : CIR_Op<"atomic.clear"> {
   let arguments = (ins
     Arg<CIR_PtrToType<CIR_SInt8>, "", [MemRead, MemWrite]>:$ptr,
     Arg<CIR_MemOrder, "memory order">:$mem_order,
-    OptionalAttr<CIR_MemScopeKind>:$syncscope,
+    OptionalAttr<CIR_SyncScopeKind>:$sync_scope,
     OptionalAttr<I64Attr>:$alignment,
     UnitAttr:$is_volatile);
 
@@ -6227,11 +6235,11 @@ def CIR_AtomicFence : CIR_Op<"atomic.fence"> {
 
   let arguments = (ins
     Arg<CIR_MemOrder, "memory order">:$ordering,
-    OptionalAttr<CIR_MemScopeKind>:$syncscope
+    OptionalAttr<CIR_SyncScopeKind>:$sync_scope
   );
 
   let assemblyFormat = [{
-    (`syncscope` `(` $syncscope^ `)`)? $ordering attr-dict
+    (`syncscope` `(` $sync_scope^ `)`)? $ordering attr-dict
   }];
 }
 

clang/include/clang/CIR/MissingFeatures.h

@@ -189,7 +189,6 @@ struct MissingFeatures {
   // Type qualifiers.
   static bool atomicTypes() { return false; }
   static bool volatileTypes() { return false; }
-  static bool syncScopeID() { return false; }
 
   // ABIInfo queries.
   static bool useTargetLoweringABIInfo() { return false; }

clang/lib/CIR/CodeGen/CIRGenAtomic.cpp

@@ -17,6 +17,7 @@
 #include "CIRGenOpenMPRuntime.h"
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/SyncScope.h"
 #include "clang/CIR/Dialect/IR/CIRAttrs.h"
 #include "clang/CIR/Dialect/IR/CIRDataLayout.h"
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
@@ -350,6 +351,20 @@ static cir::IntAttr extractIntAttr(mlir::Value v) {
   return {};
 }
 
+// Maps SyncScope::SingleScope to SyncScopeKind::SingleThread,
+// SyncScope::SystemScope to SyncScopeKind::System,
+// and asserts (llvm_unreachable) for anything else.
+static cir::SyncScopeKind convertSyncScopeToCIR(clang::SyncScope scope) {
+  switch (scope) {
+  case clang::SyncScope::SingleScope:
+    return cir::SyncScopeKind::SingleThread;
+  case clang::SyncScope::SystemScope:
+    return cir::SyncScopeKind::System;
+  default:
+    llvm_unreachable("NYI");
+  }
+}
+
 // Inspect a value that is the strong/weak flag for a compare-exchange.  If it
 // is a constant of intergral or boolean type, set `val` to the constant's
 // boolean value and return true.  Otherwise leave `val` unchanged and return
@@ -418,7 +433,7 @@ static void emitAtomicCmpXchg(CIRGenFunction &CGF, AtomicExpr *E, bool IsWeak,
                               Address Val2, uint64_t Size,
                               cir::MemOrder SuccessOrder,
                               cir::MemOrder FailureOrder,
-                              cir::MemScopeKind Scope) {
+                              cir::SyncScopeKind Scope) {
   auto &builder = CGF.getBuilder();
   auto loc = CGF.getLoc(E->getSourceRange());
   auto Expected = builder.createLoad(loc, Val1);
@@ -428,7 +443,7 @@ static void emitAtomicCmpXchg(CIRGenFunction &CGF, AtomicExpr *E, bool IsWeak,
       builder, loc, Expected.getType(), boolTy, Ptr.getPointer(), Expected,
       Desired, cir::MemOrderAttr::get(&CGF.getMLIRContext(), SuccessOrder),
       cir::MemOrderAttr::get(&CGF.getMLIRContext(), FailureOrder),
-      cir::MemScopeKindAttr::get(&CGF.getMLIRContext(), Scope),
+      cir::SyncScopeKindAttr::get(&CGF.getMLIRContext(), Scope),
       builder.getI64IntegerAttr(Ptr.getAlignment().getAsAlign().value()));
   cmpxchg.setIsVolatile(E->isVolatile());
   cmpxchg.setWeak(IsWeak);
@@ -456,7 +471,7 @@ static void emitAtomicCmpXchg(CIRGenFunction &CGF, AtomicExpr *E, bool IsWeak,
 static void emitAtomicCmpXchgFailureSet(
     CIRGenFunction &CGF, AtomicExpr *E, bool IsWeak, Address Dest, Address Ptr,
     Address Val1, Address Val2, mlir::Value FailureOrderVal, uint64_t Size,
-    cir::MemOrder SuccessOrder, cir::MemScopeKind Scope) {
+    cir::MemOrder SuccessOrder, cir::SyncScopeKind Scope) {
 
   cir::MemOrder FailureOrder;
   if (auto ordAttr = extractIntAttr(FailureOrderVal)) {
@@ -546,8 +561,7 @@ static void emitAtomicOp(CIRGenFunction &CGF, AtomicExpr *E, Address Dest,
                          Address Ptr, Address Val1, Address Val2,
                          mlir::Value IsWeak, mlir::Value FailureOrder,
                          uint64_t Size, cir::MemOrder Order,
-                         cir::MemScopeKind Scope) {
-  assert(!cir::MissingFeatures::syncScopeID());
+                         cir::SyncScopeKind Scope) {
   StringRef Op;
 
   auto &builder = CGF.getBuilder();
@@ -594,9 +608,7 @@ static void emitAtomicOp(CIRGenFunction &CGF, AtomicExpr *E, Address Dest,
   case AtomicExpr::AO__scoped_atomic_load_n:
   case AtomicExpr::AO__scoped_atomic_load: {
     auto load = builder.createLoad(loc, Ptr);
-    // FIXME(cir): add scope information.
-    assert(!cir::MissingFeatures::syncScopeID());
-    load.setMemOrder(Order);
+    load.setAtomic(Order, Scope);
     load.setIsVolatile(E->isVolatile());
 
     // TODO(cir): this logic should be part of createStore, but doing so
@@ -619,7 +631,6 @@ static void emitAtomicOp(CIRGenFunction &CGF, AtomicExpr *E, Address Dest,
   case AtomicExpr::AO__scoped_atomic_store_n: {
     auto loadVal1 = builder.createLoad(loc, Val1);
     // FIXME(cir): add scope information.
-    assert(!cir::MissingFeatures::syncScopeID());
     builder.createStore(loc, loadVal1, Ptr, E->isVolatile(),
                         /*isNontemporal=*/false,
                         /*alignment=*/mlir::IntegerAttr{}, orderAttr);
@@ -748,7 +759,7 @@ static void emitAtomicOp(CIRGenFunction &CGF, AtomicExpr *E, Address Dest,
   case AtomicExpr::AO__atomic_test_and_set: {
     auto op = cir::AtomicTestAndSetOp::create(
         builder, loc, Ptr.getPointer(), Order,
-        cir::MemScopeKindAttr::get(&CGF.getMLIRContext(), Scope),
+        cir::SyncScopeKindAttr::get(&CGF.getMLIRContext(), Scope),
         builder.getI64IntegerAttr(Ptr.getAlignment().getQuantity()),
         E->isVolatile());
     builder.createStore(loc, op, Dest);
@@ -758,7 +769,7 @@ static void emitAtomicOp(CIRGenFunction &CGF, AtomicExpr *E, Address Dest,
   case AtomicExpr::AO__atomic_clear: {
     cir::AtomicClearOp::create(
         builder, loc, Ptr.getPointer(), Order,
-        cir::MemScopeKindAttr::get(&CGF.getMLIRContext(), Scope),
+        cir::SyncScopeKindAttr::get(&CGF.getMLIRContext(), Scope),
         builder.getI64IntegerAttr(Ptr.getAlignment().getQuantity()),
         E->isVolatile());
     return;
@@ -811,16 +822,17 @@ static void emitAtomicOp(CIRGenFunction &CGF, AtomicExpr *Expr, Address Dest,
   // LLVM atomic instructions always have synch scope. If clang atomic
   // expression has no scope operand, use default LLVM synch scope.
   if (!ScopeModel) {
-    assert(!cir::MissingFeatures::syncScopeID());
     emitAtomicOp(CGF, Expr, Dest, Ptr, Val1, Val2, IsWeak, FailureOrder, Size,
-                 Order, cir::MemScopeKind::System);
+                 Order, cir::SyncScopeKind::System);
     return;
   }
 
   // Handle constant scope.
-  if (extractIntAttr(Scope)) {
-    assert(!cir::MissingFeatures::syncScopeID());
-    llvm_unreachable("NYI");
+  if (auto scopeAttr = extractIntAttr(Scope)) {
+    auto mappedScope =
+        convertSyncScopeToCIR(ScopeModel->map(scopeAttr.getUInt()));
+    emitAtomicOp(CGF, Expr, Dest, Ptr, Val1, Val2, IsWeak, FailureOrder, Size,
+                 Order, mappedScope);
     return;
   }
 

clang/lib/CIR/CodeGen/CIRGenBuilder.h

@@ -870,6 +870,7 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
     return cir::LoadOp::create(
         *this, loc, addr.getElementType(), addr.getPointer(), /*isDeref=*/false,
         /*is_volatile=*/isVolatile, /*is_nontemporal=*/isNontemporal, align,
+        /*sync_scope=*/cir::SyncScopeKindAttr{},
         /*mem_order=*/cir::MemOrderAttr{}, /*tbaa=*/cir::TBAAAttr{});
   }
 

clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp

@@ -370,15 +370,15 @@ static mlir::Value makeAtomicCmpXchgValue(CIRGenFunction &cgf,
                         cir::MemOrder::SequentiallyConsistent),
       MemOrderAttr::get(&cgf.getMLIRContext(),
                         cir::MemOrder::SequentiallyConsistent),
-      MemScopeKindAttr::get(&cgf.getMLIRContext(), cir::MemScopeKind::System),
+      SyncScopeKindAttr::get(&cgf.getMLIRContext(), cir::SyncScopeKind::System),
       builder.getI64IntegerAttr(destAddr.getAlignment().getAsAlign().value()));
 
   return returnBool ? op.getResult(1) : op.getResult(0);
 }
 
 static mlir::Value makeAtomicFenceValue(CIRGenFunction &cgf,
                                         const CallExpr *expr,
-                                        cir::MemScopeKind syncScope) {
+                                        cir::SyncScopeKind syncScope) {
   auto &builder = cgf.getBuilder();
   mlir::Value orderingVal = cgf.emitScalarExpr(expr->getArg(0));
 
@@ -392,7 +392,7 @@ static mlir::Value makeAtomicFenceValue(CIRGenFunction &cgf,
 
     cir::AtomicFence::create(
         builder, cgf.getLoc(expr->getSourceRange()), ordering,
-        MemScopeKindAttr::get(&cgf.getMLIRContext(), syncScope));
+        SyncScopeKindAttr::get(&cgf.getMLIRContext(), syncScope));
   }
 
   return mlir::Value();
@@ -2155,10 +2155,10 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
 
   case Builtin::BI__atomic_thread_fence:
     return RValue::get(
-        makeAtomicFenceValue(*this, E, cir::MemScopeKind::System));
+        makeAtomicFenceValue(*this, E, cir::SyncScopeKind::System));
   case Builtin::BI__atomic_signal_fence:
     return RValue::get(
-        makeAtomicFenceValue(*this, E, cir::MemScopeKind::SingleThread));
+        makeAtomicFenceValue(*this, E, cir::SyncScopeKind::SingleThread));
   case Builtin::BI__c11_atomic_thread_fence:
   case Builtin::BI__c11_atomic_signal_fence:
     llvm_unreachable("BI__c11_atomic_thread_fence like NYI");

clang/lib/CIR/CodeGen/CIRGenBuiltinAArch64.cpp

@@ -18,6 +18,7 @@
 #include "CIRGenFunction.h"
 #include "CIRGenModule.h"
 #include "TargetInfo.h"
+#include "clang/CIR/Dialect/IR/CIROpsEnums.h"
 #include "clang/CIR/MissingFeatures.h"
 
 // TODO(cir): once all builtins are covered, decide whether we still
@@ -4489,7 +4490,7 @@ CIRGenFunction::emitAArch64BuiltinExpr(unsigned BuiltinID, const CallExpr *E,
   case NEON::BI__builtin_neon_vldap1q_lane_s64: {
     cir::LoadOp Load = builder.createAlignedLoad(
         Ops[0].getLoc(), vTy.getElementType(), Ops[0], PtrOp0.getAlignment());
-    Load.setAtomic(cir::MemOrder::Acquire);
+    Load.setAtomic(cir::MemOrder::Acquire, cir::SyncScopeKind::System);
     return cir::VecInsertOp::create(builder, getLoc(E->getExprLoc()),
                                     builder.createBitcast(Ops[1], vTy), Load,
                                     Ops[2]);

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -1013,12 +1013,9 @@ LogicalResult cir::ComplexImagPtrOp::verify() {
 // LoadOp
 //===----------------------------------------------------------------------===//
 
-// TODO(CIR): The final interface here should include an argument for the
-// SyncScope::ID.
-void cir::LoadOp::setAtomic(cir::MemOrder order) {
+void cir::LoadOp::setAtomic(cir::MemOrder order, cir::SyncScopeKind scope) {
   setMemOrder(order);
-  if (cir::MissingFeatures::syncScopeID())
-    llvm_unreachable("NYI");
+  setSyncScope(scope);
 }
 
 //===----------------------------------------------------------------------===//
@@ -1027,11 +1024,7 @@ void cir::LoadOp::setAtomic(cir::MemOrder order) {
 
 // TODO(CIR): The final interface here should include an argument for the
 // SyncScope::ID.
-void cir::StoreOp::setAtomic(cir::MemOrder order) {
-  setMemOrder(order);
-  if (cir::MissingFeatures::syncScopeID())
-    llvm_unreachable("NYI");
-}
+void cir::StoreOp::setAtomic(cir::MemOrder order) { setMemOrder(order); }
 
 //===----------------------------------------------------------------------===//
 // VecCreateOp

clang/lib/CIR/Dialect/Transforms/TargetLowering/ItaniumCXXABI.cpp

@@ -386,7 +386,9 @@ void ItaniumCXXABI::lowerGetMethod(
         rewriter, op.getLoc(), vtablePtrPtr, /*isDeref=*/false,
         /*isVolatile=*/false,
         /*isNontemporal=*/false,
-        /*alignment=*/mlir::IntegerAttr(), /*mem_order=*/cir::MemOrderAttr(),
+        /*alignment=*/mlir::IntegerAttr(),
+        /*sync_scope=*/cir::SyncScopeKindAttr{},
+        /*mem_order=*/cir::MemOrderAttr(),
         /*tbaa=*/mlir::ArrayAttr());
 
     // Get the vtable offset.
@@ -421,6 +423,7 @@ void ItaniumCXXABI::lowerGetMethod(
                                       /*isDeref=*/false, /*isVolatile=*/false,
                                       /*isNontemporal=*/false,
                                       /*alignment=*/mlir::IntegerAttr(),
+                                      /*sync_scope=*/cir::SyncScopeKindAttr{},
                                       /*mem_order=*/cir::MemOrderAttr(),
                                       /*tbaa=*/mlir::ArrayAttr());
       }