llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen
Author: Kiran (kiran-isaac)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/180200.diff
3 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+3)
- (modified) clang/lib/CodeGen/CGExprScalar.cpp (+6-1)
- (added) clang/test/CodeGen/compound-assign-atomic-bool.c (+31)
``````````diff
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 d21e017bd2b56..1f9389660e127 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4035,9 +4035,14 @@ 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();
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..fd99f09d640ad
--- /dev/null
+++ b/clang/test/CodeGen/compound-assign-atomic-bool.c
@@ -0,0 +1,31 @@
+// 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
+// with zero rather than truncating.
+
+// CHECK-LABEL: @compund_assign_add
+int compund_assign_add(void) {
+ _Atomic _Bool b;
+
+ b += 2;
+ // CHECK: add
+ // CHECK: icmp ne
+ // CHECK-NOT: trunc
+ // CHECK: {{cmpxchg|call.*__atomic_compare_exchange}}
+ return b;
+}
+
+// CHECK-LABEL: @compund_assign_minus
+int compund_assign_minus(void) {
+ _Atomic _Bool b;
+
+ b -= 2;
+ // CHECK: sub
+ // CHECK: icmp ne
+ // CHECK-NOT: trunc
+ // CHECK: {{cmpxchg|call.*__atomic_compare_exchange}}
+ return b;
+}
\ No newline at end of file
``````````
</details>
https://github.com/llvm/llvm-project/pull/180200
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
