Skip to content

[CIR] Add scoped atomic load operation #171134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Lancern merged 1 commit into from
Dec 10, 2025
+152 −49
Merged

[CIR] Add scoped atomic load operation #171134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
bool isVolatile = false, uint64_t alignment = 0) {
mlir::IntegerAttr alignmentAttr = getAlignmentAttr(alignment);
return cir::LoadOp::create(*this, loc, ptr, /*isDeref=*/false, isVolatile,
alignmentAttr, cir::MemOrderAttr{});
alignmentAttr, cir::SyncScopeKindAttr{},
cir::MemOrderAttr{});
}

mlir::Value createAlignedLoad(mlir::Location loc, mlir::Value ptr,
Expand Down Expand Up @@ -366,7 +367,7 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
auto addr = createAlloca(loc, getPointerTo(type), type, {}, alignmentAttr);
return cir::LoadOp::create(*this, loc, addr, /*isDeref=*/false,
/*isVolatile=*/false, alignmentAttr,
/*mem_order=*/{});
/*sync_scope=*/{}, /*mem_order=*/{});
}

cir::PtrStrideOp createPtrStride(mlir::Location loc, mlir::Value base,
Expand Down
16 changes: 11 additions & 5 deletions clang/include/clang/CIR/Dialect/IR/CIROps.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,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
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -586,13 +595,15 @@ def CIR_LoadOp : CIR_Op<"load", [
UnitAttr:$isDeref,
UnitAttr:$is_volatile,
OptionalAttr<I64Attr>:$alignment,
OptionalAttr<CIR_SyncScopeKind>:$sync_scope,
OptionalAttr<CIR_MemOrder>:$mem_order);
let results = (outs CIR_AnyType:$result);

let assemblyFormat = [{
(`deref` $isDeref^)?
(`volatile` $is_volatile^)?
(`align` `(` $alignment^ `)`)?
(`syncscope` `(` $sync_scope^ `)`)?
(`atomic` `(` $mem_order^ `)`)?
$addr `:` qualified(type($addr)) `,` type($result) attr-dict
}];
Expand Down Expand Up @@ -5265,11 +5276,6 @@ def CIR_AtomicFetchKind : CIR_I32EnumAttr<
I32EnumAttrCase<"Min", 7, "min">
]>;

def CIR_SyncScopeKind : CIR_I32EnumAttr<"SyncScopeKind", "sync scope kind", [
I32EnumAttrCase<"SingleThread", 0, "single_thread">,
I32EnumAttrCase<"System", 1, "system">
]>;

def CIR_AtomicFetchOp : CIR_Op<"atomic.fetch", [
AllTypesMatch<["result", "val"]>,
TypesMatchWith<"type of 'val' must match the pointee type of 'ptr'",
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/CIR/MissingFeatures.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ struct MissingFeatures {
static bool atomicInfoGetAtomicAddress() { return false; }
static bool atomicScope() { return false; }
static bool atomicSyncScopeID() { return false; }
static bool atomicMapTargetSyncScope() { return false; }
static bool atomicTypes() { return false; }
static bool atomicUseLibCall() { return false; }
static bool atomicMicrosoftVolatile() { return false; }
Expand Down
96 changes: 73 additions & 23 deletions clang/lib/CIR/CodeGen/CIRGenAtomic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,20 +399,14 @@ static void emitAtomicCmpXchgFailureSet(CIRGenFunction &cgf, AtomicExpr *e,
static void emitAtomicOp(CIRGenFunction &cgf, AtomicExpr *expr, Address dest,
Address ptr, Address val1, Address val2,
Expr *isWeakExpr, Expr *failureOrderExpr, int64_t size,
cir::MemOrder order) {
std::unique_ptr<AtomicScopeModel> scopeModel = expr->getScopeModel();
if (scopeModel) {
Copy link
Contributor

@andykaylor andykaylor Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This divergence from the structure of classic codegen worries me. It looks like we need for the handling to be done here to cover the non-constant scope value case properly.

Copy link
Member Author

@Lancern Lancern Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit worried that the changes to align the code structure may not fit in this PR. Should this be a separate PR?

Copy link
Contributor

@andykaylor andykaylor Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The divergence I was talking about is introduced by this PR. That is, handling the scope model in emitAtomicExpr as opposed to handling it here.

Copy link
Member Author

@Lancern Lancern Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The divergence I was talking about is introduced by this PR. That is, handling the scope model in emitAtomicExpr as opposed to handling it here.

I'll address this in the next revision of this PR.

Copy link
Member Author

@Lancern Lancern Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Aligned CIRGen code structure with the classic CG.

assert(!cir::MissingFeatures::atomicScope());
cgf.cgm.errorNYI(expr->getSourceRange(), "emitAtomicOp: atomic scope");
return;
}

cir::MemOrder order, cir::SyncScopeKind scope) {
assert(!cir::MissingFeatures::atomicSyncScopeID());
llvm::StringRef opName;

CIRGenBuilderTy &builder = cgf.getBuilder();
mlir::Location loc = cgf.getLoc(expr->getSourceRange());
auto orderAttr = cir::MemOrderAttr::get(builder.getContext(), order);
auto scopeAttr = cir::SyncScopeKindAttr::get(builder.getContext(), scope);
cir::AtomicFetchKindAttr fetchAttr;
bool fetchFirst = true;

Expand Down Expand Up @@ -446,13 +440,14 @@ static void emitAtomicOp(CIRGenFunction &cgf, AtomicExpr *expr, Address dest,

case AtomicExpr::AO__c11_atomic_load:
case AtomicExpr::AO__atomic_load_n:
case AtomicExpr::AO__atomic_load: {
case AtomicExpr::AO__atomic_load:
case AtomicExpr::AO__scoped_atomic_load_n:
case AtomicExpr::AO__scoped_atomic_load: {
cir::LoadOp load =
builder.createLoad(loc, ptr, /*isVolatile=*/expr->isVolatile());

assert(!cir::MissingFeatures::atomicSyncScopeID());

load->setAttr("mem_order", orderAttr);
load->setAttr("sync_scope", scopeAttr);

builder.createStore(loc, load->getResult(0), dest);
return;
Expand Down Expand Up @@ -586,8 +581,6 @@ static void emitAtomicOp(CIRGenFunction &cgf, AtomicExpr *expr, Address dest,

case AtomicExpr::AO__opencl_atomic_load:
case AtomicExpr::AO__hip_atomic_load:
case AtomicExpr::AO__scoped_atomic_load_n:
case AtomicExpr::AO__scoped_atomic_load:

case AtomicExpr::AO__opencl_atomic_store:
case AtomicExpr::AO__hip_atomic_store:
Expand Down Expand Up @@ -671,6 +664,51 @@ static void emitAtomicOp(CIRGenFunction &cgf, AtomicExpr *expr, Address dest,
builder.createStore(loc, result, dest);
}

// Map clang sync scope to CIR sync scope.
static cir::SyncScopeKind convertSyncScopeToCIR(CIRGenFunction &cgf,
SourceRange range,
clang::SyncScope scope) {
switch (scope) {
default: {
assert(!cir::MissingFeatures::atomicSyncScopeID());
cgf.cgm.errorNYI(range, "convertSyncScopeToCIR: unhandled sync scope");
return cir::SyncScopeKind::System;
}

case clang::SyncScope::SingleScope:
return cir::SyncScopeKind::SingleThread;
case clang::SyncScope::SystemScope:
return cir::SyncScopeKind::System;
}
}

static void emitAtomicOp(CIRGenFunction &cgf, AtomicExpr *expr, Address dest,
Address ptr, Address val1, Address val2,
Expr *isWeakExpr, Expr *failureOrderExpr, int64_t size,
cir::MemOrder order,
const std::optional<Expr::EvalResult> &scopeConst,
mlir::Value scopeValue) {
std::unique_ptr<AtomicScopeModel> scopeModel = expr->getScopeModel();

if (!scopeModel) {
emitAtomicOp(cgf, expr, dest, ptr, val1, val2, isWeakExpr, failureOrderExpr,
size, order, cir::SyncScopeKind::System);
return;
}

if (scopeConst.has_value()) {
cir::SyncScopeKind mappedScope = convertSyncScopeToCIR(
cgf, expr->getScope()->getSourceRange(),
scopeModel->map(scopeConst->Val.getInt().getZExtValue()));
emitAtomicOp(cgf, expr, dest, ptr, val1, val2, isWeakExpr, failureOrderExpr,
size, order, mappedScope);
return;
}

assert(!cir::MissingFeatures::atomicSyncScopeID());
cgf.cgm.errorNYI(expr->getSourceRange(), "emitAtomicOp: dynamic sync scope");
}

static bool isMemOrderValid(uint64_t order, bool isStore, bool isLoad) {
if (!cir::isValidCIRAtomicOrderingCABI(order))
return false;
Expand All @@ -688,7 +726,8 @@ static bool isMemOrderValid(uint64_t order, bool isStore, bool isLoad) {
static void emitAtomicExprWithDynamicMemOrder(
CIRGenFunction &cgf, mlir::Value order, AtomicExpr *e, Address dest,
Address ptr, Address val1, Address val2, Expr *isWeakExpr,
Expr *orderFailExpr, uint64_t size, bool isStore, bool isLoad) {
Expr *orderFailExpr, uint64_t size, bool isStore, bool isLoad,
const std::optional<Expr::EvalResult> &scopeConst, mlir::Value scopeValue) {
// The memory order is not known at compile-time. The atomic operations
// can't handle runtime memory orders; the memory order must be hard coded.
// Generate a "switch" statement that converts a runtime value into a
Expand All @@ -706,7 +745,7 @@ static void emitAtomicExprWithDynamicMemOrder(
else
emitMemOrderCaseLabel(builder, loc, order.getType(), caseOrders);
emitAtomicOp(cgf, e, dest, ptr, val1, val2, isWeakExpr, orderFailExpr,
size, actualOrder);
size, actualOrder, scopeConst, scopeValue);
builder.createBreak(loc);
builder.setInsertionPointToEnd(switchBlock);
};
Expand Down Expand Up @@ -773,10 +812,19 @@ RValue CIRGenFunction::emitAtomicExpr(AtomicExpr *e) {
TypeInfoChars typeInfo = getContext().getTypeInfoInChars(atomicTy);
uint64_t size = typeInfo.Width.getQuantity();

Expr::EvalResult orderConst;
mlir::Value order;
if (!e->getOrder()->EvaluateAsInt(orderConst, getContext()))
order = emitScalarExpr(e->getOrder());
// Emit the memory order operand, and try to evaluate it as a constant.
mlir::Value order = emitScalarExpr(e->getOrder());
std::optional<Expr::EvalResult> orderConst;
if (Expr::EvalResult eval; e->getOrder()->EvaluateAsInt(eval, getContext()))
orderConst.emplace(std::move(eval));

// Emit the sync scope operand, and try to evaluate it as a constant.
mlir::Value scope =
e->getScopeModel() ? emitScalarExpr(e->getScope()) : nullptr;
std::optional<Expr::EvalResult> scopeConst;
if (Expr::EvalResult eval;
e->getScopeModel() && e->getScope()->EvaluateAsInt(eval, getContext()))
scopeConst.emplace(std::move(eval));

bool shouldCastToIntPtrTy = true;

Expand All @@ -789,12 +837,14 @@ RValue CIRGenFunction::emitAtomicExpr(AtomicExpr *e) {
llvm_unreachable("already handled above with emitAtomicInit");

case AtomicExpr::AO__atomic_load_n:
case AtomicExpr::AO__scoped_atomic_load_n:
case AtomicExpr::AO__c11_atomic_load:
case AtomicExpr::AO__atomic_test_and_set:
case AtomicExpr::AO__atomic_clear:
break;

case AtomicExpr::AO__atomic_load:
case AtomicExpr::AO__scoped_atomic_load:
dest = emitPointerWithAlignment(e->getVal1());
break;

Expand Down Expand Up @@ -927,18 +977,18 @@ RValue CIRGenFunction::emitAtomicExpr(AtomicExpr *e) {
e->getOp() == AtomicExpr::AO__scoped_atomic_load ||
e->getOp() == AtomicExpr::AO__scoped_atomic_load_n;

if (!order) {
if (orderConst.has_value()) {
// We have evaluated the memory order as an integer constant in orderConst.
// We should not ever get to a case where the ordering isn't a valid CABI
// value, but it's hard to enforce that in general.
uint64_t ord = orderConst.Val.getInt().getZExtValue();
uint64_t ord = orderConst->Val.getInt().getZExtValue();
if (isMemOrderValid(ord, isStore, isLoad))
emitAtomicOp(*this, e, dest, ptr, val1, val2, isWeakExpr, orderFailExpr,
size, static_cast<cir::MemOrder>(ord));
size, static_cast<cir::MemOrder>(ord), scopeConst, scope);
} else {
emitAtomicExprWithDynamicMemOrder(*this, order, e, dest, ptr, val1, val2,
isWeakExpr, orderFailExpr, size, isStore,
isLoad);
isLoad, scopeConst, scope);
}

if (resultTy->isVoidType())
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
mlir::IntegerAttr align = getAlignmentAttr(addr.getAlignment());
return cir::LoadOp::create(*this, loc, addr.getPointer(), /*isDeref=*/false,
isVolatile, /*alignment=*/align,
/*sync_scope=*/cir::SyncScopeKindAttr{},
/*mem_order=*/cir::MemOrderAttr{});
}

Expand All @@ -473,6 +474,7 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
mlir::IntegerAttr alignAttr = getAlignmentAttr(alignment);
return cir::LoadOp::create(*this, loc, ptr, /*isDeref=*/false,
/*isVolatile=*/false, alignAttr,
/*sync_scope=*/cir::SyncScopeKindAttr{},
/*mem_order=*/cir::MemOrderAttr{});
}

Expand Down
7 changes: 5 additions & 2 deletions clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1643,12 +1643,15 @@ mlir::LogicalResult CIRToLLVMLoadOpLowering::matchAndRewrite(

assert(!cir::MissingFeatures::lowerModeOptLevel());

// TODO: nontemporal, syncscope.
// TODO: nontemporal.
assert(!cir::MissingFeatures::opLoadStoreNontemporal());
std::optional<llvm::StringRef> syncScope =
getLLVMSyncScope(op.getSyncScope());
mlir::LLVM::LoadOp newLoad = mlir::LLVM::LoadOp::create(
rewriter, op->getLoc(), llvmTy, adaptor.getAddr(), alignment,
op.getIsVolatile(), /*isNonTemporal=*/false,
/*isInvariant=*/false, /*isInvariantGroup=*/false, ordering);
/*isInvariant=*/false, /*isInvariantGroup=*/false, ordering,
syncScope.value_or(llvm::StringRef()));

// Convert adapted result to its original type if needed.
mlir::Value result =
Expand Down
40 changes: 40 additions & 0 deletions clang/test/CIR/CodeGen/atomic-scoped.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-cir %s -o %t.cir
// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-llvm %s -o %t-cir.ll
// RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -Wno-unused-value -emit-llvm %s -o %t.ll
// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG

void scoped_atomic_load(int *ptr) {
// CIR-LABEL: @scoped_atomic_load
// LLVM-LABEL: @scoped_atomic_load
// OGCG-LABEL: @scoped_atomic_load

int x;
__scoped_atomic_load(ptr, &x, __ATOMIC_RELAXED, __MEMORY_SCOPE_SINGLE);
// CIR: %{{.+}} = cir.load align(4) syncscope(single_thread) atomic(relaxed) %{{.+}} : !cir.ptr<!s32i>, !s32i
// LLVM: %{{.+}} = load atomic i32, ptr %{{.+}} syncscope("singlethread") monotonic, align 4
Copy link
Contributor

@andykaylor andykaylor Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have this difference from OGCG?

Copy link
Member Author

@Lancern Lancern Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is due to a difference between the incubator and classic codegen. I think we could address this in a later PR.

Copy link
Contributor

@andykaylor andykaylor Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer not to introduce the problem in the first place if possible. It looks like a difference between getLLVMSyncScopeID (which has target-specific behavior) and convertSyncScopeToCIR (which is returning cir::SyncScopeKind::SingleThread for clang::SyncScope::SingleScope).

Copy link
Member Author

@Lancern Lancern Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem that stops me from including this directly in this PR is that this could be placed either in CIRGen or in LLVMLowering. I'm not sure it's a good idea to put this in CIRGen.

Copy link
Contributor

@andykaylor andykaylor Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point.

// OGCG: %{{.+}} = load atomic i32, ptr %{{.+}} monotonic, align 4

__scoped_atomic_load(ptr, &x, __ATOMIC_RELAXED, __MEMORY_SCOPE_SYSTEM);
// CIR: %{{.+}} = cir.load align(4) syncscope(system) atomic(relaxed) %{{.+}} : !cir.ptr<!s32i>, !s32i
// LLVM: %{{.+}} = load atomic i32, ptr %{{.+}} monotonic, align 4
// OGCG: %{{.+}} = load atomic i32, ptr %{{.+}} monotonic, align 4
}

void scoped_atomic_load_n(int *ptr) {
// CIR-LABEL: @scoped_atomic_load_n
// LLVM-LABEL: @scoped_atomic_load_n
// OGCG-LABEL: @scoped_atomic_load_n

int x;
x = __scoped_atomic_load_n(ptr, __ATOMIC_RELAXED, __MEMORY_SCOPE_SINGLE);
// CIR: %{{.+}} = cir.load align(4) syncscope(single_thread) atomic(relaxed) %{{.+}} : !cir.ptr<!s32i>, !s32i
// LLVM: %{{.+}} = load atomic i32, ptr %{{.+}} syncscope("singlethread") monotonic, align 4
// OGCG: %{{.+}} = load atomic i32, ptr %{{.+}} monotonic, align 4

x = __scoped_atomic_load_n(ptr, __ATOMIC_RELAXED, __MEMORY_SCOPE_SYSTEM);
// CIR: %{{.+}} = cir.load align(4) syncscope(system) atomic(relaxed) %{{.+}} : !cir.ptr<!s32i>, !s32i
// LLVM: %{{.+}} = load atomic i32, ptr %{{.+}} monotonic, align 4
// OGCG: %{{.+}} = load atomic i32, ptr %{{.+}} monotonic, align 4
}
Loading