https://github.com/kiran-isaac created
https://github.com/llvm/llvm-project/pull/180200
This is my second attempt to merge the #33210 fix. My original patch #178220
caused test failures on risc-v, as I committed a new test file that was too
target-specific. I reverted the PR (#180183) to avoid causing disruption, as I
did not know how long the fix would take.
I have now fixed the test so that it behaves correctly on risc-v. I have also
changed the run lines so it is tested on aarch64, amd64 and risc-v. Below is a
summary of the issue, and the changes I made to the check lines to fix it
IR generated on risc-v:
```
%add = add nsw i32 %conv, 2
%tobool = icmp ne i32 %add, 0
store i8 %1, ptr %atomic-temp1, align 1
%storedv3 = zext i1 %tobool to i8
store i8 %storedv3, ptr %atomic-temp2, align 1
%call = call zeroext i1 @__atomic_compare_exchange(..)
```
Old check:
```
// CHECK: add
// CHECK-NEXT: icmp ne
// CHECK-NEXT: zext
// CHECK-NEXT: cmpxchg
```
Failed on the `zext`, as there is a `store` in between the `icmp ne` and the
`zext`.
New checks:
```
// CHECK: add
// CHECK: icmp ne
// CHECK-NOT: trunc
// CHECK: {{cmpxchg|call.*__atomic_compare_exchange}}
```
These tests do not rely on the exact position of the `zext`, as it is not
really related to this issue.
Lesson learned. I will use one/multiple target-specific run lines, rather than
relying on native target, for future codegen tests.
>From e9a48062a4207736cd3da24bd442ccee710c5edf Mon Sep 17 00:00:00 2001
From: Kiran Sturt <[email protected]>
Date: Tue, 30 Dec 2025 18:05:53 +0000
Subject: [PATCH 1/6] [Clang] Fix atomic boolean compound assignment (#33210)
---
clang/lib/CodeGen/CGExprScalar.cpp | 7 ++++
.../CodeGen/compound-assign-atomic-bool.c | 33 +++++++++++++++++++
2 files changed, 40 insertions(+)
create mode 100644 clang/test/CodeGen/compound-assign-atomic-bool.c
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp
b/clang/lib/CodeGen/CGExprScalar.cpp
index d21e017bd2b56..984215e563894 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1591,6 +1591,13 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value
*Src, QualType SrcType,
if (DstType->isBooleanType())
return EmitConversionToBool(Src, SrcType);
+ // Also handle conversions to atomic bools
+ if (const AtomicType *atomicType = DstType->getAs<AtomicType>()) {
+ QualType ValueType = atomicType->getValueType();
+ if (ValueType->isBooleanType())
+ return EmitConversionToBool(Src, ValueType);
+ }
+
llvm::Type *DstTy = ConvertType(DstType);
// Cast from half through float if half isn't a native type.
diff --git a/clang/test/CodeGen/compound-assign-atomic-bool.c
b/clang/test/CodeGen/compound-assign-atomic-bool.c
new file mode 100644
index 0000000000000..8a39a76788e66
--- /dev/null
+++ b/clang/test/CodeGen/compound-assign-atomic-bool.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+// When performing compound assignment on atomic_bool, ensure that we
+// correctly handle the conversion from integer to boolean, by comparing
+// with zero rather than truncating.
+
+#include <stdatomic.h>
+#include <stdbool.h>
+
+
+// CHECK: @compund_assign_add
+int compund_assign_add(void) {
+ atomic_bool b;
+
+ b += 2;
+ // CHECK: add
+ // CHECK-NEXT: icmp ne
+ // CHECK-NEXT: zext
+ // CHECK-NEXT: cmpxchg
+ return b;
+}
+
+// CHECK: @compund_assign_minus
+int compund_assign_minus(void) {
+ atomic_bool b;
+
+ b -= 2;
+ // CHECK: sub
+ // CHECK-NEXT: icmp ne
+ // CHECK-NEXT: zext
+ // CHECK-NEXT: cmpxchg
+ return b;
+}
>From 851fd8224198a25aec9250c9979e939505fb24cd Mon Sep 17 00:00:00 2001
From: Kiran Sturt <[email protected]>
Date: Thu, 5 Feb 2026 15:54:56 +0000
Subject: [PATCH 2/6] Style changes, add release notes
Change-Id: I98669a9543f0c93a08fa939655ebd12a2f321ae5
---
clang/docs/ReleaseNotes.rst | 3 +++
clang/lib/CodeGen/CGExprScalar.cpp | 4 ++--
clang/test/CodeGen/compound-assign-atomic-bool.c | 8 ++------
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 24d4e07ca68b3..1f925419432bb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -223,6 +223,9 @@ Improvements to Coverage Mapping
Bug Fixes in This Version
-------------------------
+
+- Fixed atomic boolean compound assignment; the conversion back to atomic bool
would be miscompiled. (#GH33210)
+
- Fixed a failed assertion in the preprocessor when ``__has_embed`` parameters
are missing parentheses. (#GH175088)
- Fix lifetime extension of temporaries in for-range-initializers in
templates. (#GH165182)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp
b/clang/lib/CodeGen/CGExprScalar.cpp
index 984215e563894..32fc394a5d959 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1592,8 +1592,8 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value
*Src, QualType SrcType,
return EmitConversionToBool(Src, SrcType);
// Also handle conversions to atomic bools
- if (const AtomicType *atomicType = DstType->getAs<AtomicType>()) {
- QualType ValueType = atomicType->getValueType();
+ if (const auto *DstAsAtomic = DstType->getAs<AtomicType>()) {
+ QualType ValueType = DstAsAtomic->getValueType();
if (ValueType->isBooleanType())
return EmitConversionToBool(Src, ValueType);
}
diff --git a/clang/test/CodeGen/compound-assign-atomic-bool.c
b/clang/test/CodeGen/compound-assign-atomic-bool.c
index 8a39a76788e66..4e8f351b49b81 100644
--- a/clang/test/CodeGen/compound-assign-atomic-bool.c
+++ b/clang/test/CodeGen/compound-assign-atomic-bool.c
@@ -4,13 +4,9 @@
// correctly handle the conversion from integer to boolean, by comparing
// with zero rather than truncating.
-#include <stdatomic.h>
-#include <stdbool.h>
-
-
// CHECK: @compund_assign_add
int compund_assign_add(void) {
- atomic_bool b;
+ _Atomic _Bool b;
b += 2;
// CHECK: add
@@ -22,7 +18,7 @@ int compund_assign_add(void) {
// CHECK: @compund_assign_minus
int compund_assign_minus(void) {
- atomic_bool b;
+ _Atomic _Bool b;
b -= 2;
// CHECK: sub
>From cc6cf37b9e53070c3359606548b945260d1d62c7 Mon Sep 17 00:00:00 2001
From: Kiran Sturt <[email protected]>
Date: Thu, 5 Feb 2026 16:15:52 +0000
Subject: [PATCH 3/6] Lift the conversion from EmitScalerConversion to
EmitCompoundAssignLValue.
Atomics aren't really scaler, are they.
Change-Id: Ida3cc76eecde6b426c0f29ac52034b89b45f6816
---
clang/lib/CodeGen/CGExprScalar.cpp | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp
b/clang/lib/CodeGen/CGExprScalar.cpp
index 32fc394a5d959..5ffff5ee588c2 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1591,13 +1591,6 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value
*Src, QualType SrcType,
if (DstType->isBooleanType())
return EmitConversionToBool(Src, SrcType);
- // Also handle conversions to atomic bools
- if (const auto *DstAsAtomic = DstType->getAs<AtomicType>()) {
- QualType ValueType = DstAsAtomic->getValueType();
- if (ValueType->isBooleanType())
- return EmitConversionToBool(Src, ValueType);
- }
-
llvm::Type *DstTy = ConvertType(DstType);
// Cast from half through float if half isn't a native type.
@@ -4042,10 +4035,15 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue(
if (LHSLV.isBitField()) {
Previous = Result;
Result = EmitScalarConversion(Result, PromotionTypeCR, LHSTy, Loc);
- } else
+ } else if (const auto *atomicTy = LHSTy->getAs<AtomicType>()) {
+ Result = EmitScalarConversion(Result, PromotionTypeCR,
+ atomicTy->getValueType(), Loc,
+ ScalarConversionOpts(CGF.SanOpts));
+ } else {
Result = EmitScalarConversion(Result, PromotionTypeCR, LHSTy, Loc,
ScalarConversionOpts(CGF.SanOpts));
-
+ }
+
if (atomicPHI) {
llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
>From a3d4218a954a1b58441b14f2fd6f71003bfaf13d Mon Sep 17 00:00:00 2001
From: Kiran Sturt <[email protected]>
Date: Thu, 5 Feb 2026 16:36:53 +0000
Subject: [PATCH 4/6] Reformat
Change-Id: I561a5ac983f5abdc5dfeca1d179e7df2d9b6a49c
---
clang/lib/CodeGen/CGExprScalar.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp
b/clang/lib/CodeGen/CGExprScalar.cpp
index 5ffff5ee588c2..1f9389660e127 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4036,14 +4036,14 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue(
Previous = Result;
Result = EmitScalarConversion(Result, PromotionTypeCR, LHSTy, Loc);
} else if (const auto *atomicTy = LHSTy->getAs<AtomicType>()) {
- Result = EmitScalarConversion(Result, PromotionTypeCR,
- atomicTy->getValueType(), Loc,
- ScalarConversionOpts(CGF.SanOpts));
+ Result =
+ EmitScalarConversion(Result, PromotionTypeCR, atomicTy->getValueType(),
+ Loc, ScalarConversionOpts(CGF.SanOpts));
} else {
Result = EmitScalarConversion(Result, PromotionTypeCR, LHSTy, Loc,
ScalarConversionOpts(CGF.SanOpts));
}
-
+
if (atomicPHI) {
llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
>From 5dc7278c4c4de4eecf3c7d9f023b474b48657bb4 Mon Sep 17 00:00:00 2001
From: Kiran Sturt <[email protected]>
Date: Fri, 6 Feb 2026 13:34:16 +0000
Subject: [PATCH 5/6] Make the test more general; avoid test failures on
targets that handle atomics differently
Change-Id: I9cde53604b42d2def3e4d5c726c6499d1f8c4a1c
---
.../test/CodeGen/compound-assign-atomic-bool.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/clang/test/CodeGen/compound-assign-atomic-bool.c
b/clang/test/CodeGen/compound-assign-atomic-bool.c
index 4e8f351b49b81..b4cbfac123070 100644
--- a/clang/test/CodeGen/compound-assign-atomic-bool.c
+++ b/clang/test/CodeGen/compound-assign-atomic-bool.c
@@ -4,26 +4,26 @@
// correctly handle the conversion from integer to boolean, by comparing
// with zero rather than truncating.
-// CHECK: @compund_assign_add
+// CHECK-LABEL: @compund_assign_add
int compund_assign_add(void) {
_Atomic _Bool b;
b += 2;
// CHECK: add
- // CHECK-NEXT: icmp ne
- // CHECK-NEXT: zext
- // CHECK-NEXT: cmpxchg
+ // CHECK: icmp ne
+ // CHECK-NOT: trunc
+ // CHECK: {{cmpxchg|call.*__atomic_compare_exchange}}
return b;
}
-// CHECK: @compund_assign_minus
+// CHECK-LABEL: @compund_assign_minus
int compund_assign_minus(void) {
_Atomic _Bool b;
b -= 2;
// CHECK: sub
- // CHECK-NEXT: icmp ne
- // CHECK-NEXT: zext
- // CHECK-NEXT: cmpxchg
+ // CHECK: icmp ne
+ // CHECK-NOT: trunc
+ // CHECK: {{cmpxchg|call.*__atomic_compare_exchange}}
return b;
-}
+}
\ No newline at end of file
>From 25a261c7b8d9a6fd5f91bd44519b5ddf873dc297 Mon Sep 17 00:00:00 2001
From: Kiran Sturt <[email protected]>
Date: Fri, 6 Feb 2026 14:03:49 +0000
Subject: [PATCH 6/6] Multiple targets
Change-Id: Ic6d66f208dde1ff27ce0b4918c5269837eeb57df
---
clang/test/CodeGen/compound-assign-atomic-bool.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/test/CodeGen/compound-assign-atomic-bool.c
b/clang/test/CodeGen/compound-assign-atomic-bool.c
index b4cbfac123070..fd99f09d640ad 100644
--- a/clang/test/CodeGen/compound-assign-atomic-bool.c
+++ b/clang/test/CodeGen/compound-assign-atomic-bool.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -o - %s |
FileCheck %s
+// RUN: %clang_cc1 -triple riscv64-unknown-linux-gnu -emit-llvm -o - %s |
FileCheck %s
+// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -emit-llvm -o - %s |
FileCheck %s
// When performing compound assignment on atomic_bool, ensure that we
// correctly handle the conversion from integer to boolean, by comparing
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
