Skip to content

Commit 58d5f0b

Browse files
authored
[CIR] Fix for __atomic_compare_exchange weak arg (#721)
ClangIR was failing on ``` __atomic_compare_exchange_n(&a, &old, 42, true, 5, 5); ``` The `true` was the problem. It would work with a literal `0` or `1`, but not with a literal `true` or `false`. The bug was in `isCstWeak` in CIRGenAtomic.cpp, which was only looking for an integral constant. It didn't recognize a boolean constant and was falling back on the non-constant path, which isn't implemented yet. Rewrite `isCstWeak` to check for both intergral and boolean constants.
1 parent 5792eb0 commit 58d5f0b

File tree

2 files changed

+23
-10
lines changed

2 files changed

+23
-10
lines changed

clang/lib/CIR/CodeGen/CIRGenAtomic.cpp

+20-7
Original file line numberDiff line numberDiff line change
@@ -348,12 +348,25 @@ static mlir::cir::IntAttr getConstOpIntAttr(mlir::Value v) {
348348
return constVal;
349349
}
350350

351-
static bool isCstWeak(mlir::Value weakVal, uint64_t &val) {
352-
auto intAttr = getConstOpIntAttr(weakVal);
353-
if (!intAttr)
354-
return false;
355-
val = intAttr.getUInt();
356-
return true;
351+
// Inspect a value that is the strong/weak flag for a compare-exchange. If it
352+
// is a constant of intergral or boolean type, set `val` to the constant's
353+
// boolean value and return true. Otherwise leave `val` unchanged and return
354+
// false.
355+
static bool isCstWeak(mlir::Value weakVal, bool &val) {
356+
mlir::Operation *op = weakVal.getDefiningOp();
357+
while (auto c = dyn_cast<mlir::cir::CastOp>(op)) {
358+
op = c.getOperand().getDefiningOp();
359+
}
360+
if (auto c = dyn_cast<mlir::cir::ConstantOp>(op)) {
361+
if (mlir::isa<mlir::cir::IntType>(c.getType())) {
362+
val = mlir::cast<mlir::cir::IntAttr>(c.getValue()).getUInt() != 0;
363+
return true;
364+
} else if (mlir::isa<mlir::cir::BoolType>(c.getType())) {
365+
val = mlir::cast<mlir::cir::BoolAttr>(c.getValue()).getValue();
366+
return true;
367+
}
368+
}
369+
return false;
357370
}
358371

359372
static void buildAtomicCmpXchg(CIRGenFunction &CGF, AtomicExpr *E, bool IsWeak,
@@ -470,7 +483,7 @@ static void buildAtomicOp(CIRGenFunction &CGF, AtomicExpr *E, Address Dest,
470483
case AtomicExpr::AO__atomic_compare_exchange_n:
471484
case AtomicExpr::AO__scoped_atomic_compare_exchange:
472485
case AtomicExpr::AO__scoped_atomic_compare_exchange_n: {
473-
uint64_t weakVal;
486+
bool weakVal;
474487
if (isCstWeak(IsWeak, weakVal)) {
475488
buildAtomicCmpXchgFailureSet(CGF, E, weakVal, Dest, Ptr, Val1, Val2,
476489
FailureOrder, Size, Order, Scope);

clang/test/CIR/CodeGen/atomic.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ bool fd4(struct S *a, struct S *b, struct S *c) {
260260
bool fi4a(int *i) {
261261
int cmp = 0;
262262
int desired = 1;
263-
return __atomic_compare_exchange(i, &cmp, &desired, 0, memory_order_acquire, memory_order_acquire);
263+
return __atomic_compare_exchange(i, &cmp, &desired, false, memory_order_acquire, memory_order_acquire);
264264
}
265265

266266
// CHECK-LABEL: @_Z4fi4aPi
@@ -273,7 +273,7 @@ bool fi4a(int *i) {
273273

274274
bool fi4b(int *i) {
275275
int cmp = 0;
276-
return __atomic_compare_exchange_n(i, &cmp, 1, 1, memory_order_acquire, memory_order_acquire);
276+
return __atomic_compare_exchange_n(i, &cmp, 1, true, memory_order_acquire, memory_order_acquire);
277277
}
278278

279279
// CHECK-LABEL: @_Z4fi4bPi
@@ -470,4 +470,4 @@ void cmp_val_short(short* p, short x, short u) {
470470
// LLVM: cmpxchg ptr {{.*}}, i8 {{.*}}, i8 {{.*}} seq_cst seq_cst
471471
void cmp_val_byte(char* p, char x, char u) {
472472
char r = __sync_val_compare_and_swap(p, x, u);
473-
}
473+
}

0 commit comments

Comments
 (0)