[clang] Handle constant "pointers" for `__atomic_always_lock_free`/`__atomic_is_lock_free`. (PR #99340)
James Y Knight via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 22 11:18:00 PDT 2024
https://github.com/jyknight updated https://github.com/llvm/llvm-project/pull/99340
>From 2cb327b0377d4196982842455e2aaf4e6a21f667 Mon Sep 17 00:00:00 2001
From: James Y Knight <jyknight at google.com>
Date: Wed, 17 Jul 2024 10:43:45 -0400
Subject: [PATCH 1/4] Handle constant "pointers" for
`__atomic_always_lock_free`/`__atomic_is_lock_free`.
The second argument passed to these builtins is used to validate
whether the object's alignment is sufficient for atomic operations of
the given size.
Currently, the builtins can be folded at compile time only when the
argument is 0/nullptr, or if the _type_ of the pointer guarantees
appropriate alignment.
This change allows the compiler to also evaluate non-null constant
pointers, which enables callers to check a specified alignment,
instead of only the type or an exact object. E.g.:
`__atomic_is_lock_free(sizeof(T), (void*)4)`
can be potentially evaluated to true at compile time, instead of
generating a libcall. This is also supported by GCC, and used by
libstdc++.
Related to (but doesn't fix) issue #75081.
---
clang/lib/AST/ExprConstant.cpp | 18 +++++++++++++-----
clang/test/Sema/atomic-ops.c | 18 ++++++++++++++++++
2 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 0aeac9d03eed3..6eb6d27cc9588 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12949,13 +12949,21 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
Info.Ctx.getTargetInfo().getMaxAtomicInlineWidth();
if (Size <= Info.Ctx.toCharUnitsFromBits(InlineWidthBits)) {
if (BuiltinOp == Builtin::BI__c11_atomic_is_lock_free ||
- Size == CharUnits::One() ||
- E->getArg(1)->isNullPointerConstant(Info.Ctx,
- Expr::NPC_NeverValueDependent))
- // OK, we will inline appropriately-aligned operations of this size,
- // and _Atomic(T) is appropriately-aligned.
+ Size == CharUnits::One())
return Success(1, E);
+ // If the pointer argument can be evaluated to a compile-time constant
+ // integer (or nullptr), check if that value is appropriately aligned.
+ const Expr *PtrArg = E->getArg(1);
+ Expr::EvalResult ExprResult;
+ APSInt IntResult;
+ if (PtrArg->EvaluateAsRValue(ExprResult, Info.Ctx) &&
+ ExprResult.Val.toIntegralConstant(IntResult, PtrArg->getType(),
+ Info.Ctx) &&
+ IntResult.isAligned(Size.getAsAlign()))
+ return Success(1, E);
+
+ // Otherwise, check if the type's alignment against Size.
QualType PointeeType = E->getArg(1)->IgnoreImpCasts()->getType()->
castAs<PointerType>()->getPointeeType();
if (!PointeeType->isIncompleteType() &&
diff --git a/clang/test/Sema/atomic-ops.c b/clang/test/Sema/atomic-ops.c
index 9b82d82ff8269..86b6ec6ba7a25 100644
--- a/clang/test/Sema/atomic-ops.c
+++ b/clang/test/Sema/atomic-ops.c
@@ -124,6 +124,24 @@ _Static_assert(__atomic_always_lock_free(4, &i64), "");
_Static_assert(!__atomic_always_lock_free(8, &i32), "");
_Static_assert(__atomic_always_lock_free(8, &i64), "");
+// Validate use with fake pointers constants. This mechanism is used to allow
+// validating atomicity of a given size and alignment.
+_Static_assert(__atomic_is_lock_free(1, (void*)1), "");
+_Static_assert(__atomic_is_lock_free(1, (void*)-1), "");
+_Static_assert(__atomic_is_lock_free(4, (void*)2), ""); // expected-error {{not an integral constant expression}}
+_Static_assert(__atomic_is_lock_free(4, (void*)-2), ""); // expected-error {{not an integral constant expression}}
+_Static_assert(__atomic_is_lock_free(4, (void*)4), "");
+_Static_assert(__atomic_is_lock_free(4, (void*)-4), "");
+
+_Static_assert(__atomic_always_lock_free(1, (void*)1), "");
+_Static_assert(__atomic_always_lock_free(1, (void*)-1), "");
+_Static_assert(!__atomic_always_lock_free(4, (void*)2), "");
+_Static_assert(!__atomic_always_lock_free(4, (void*)-2), "");
+_Static_assert(__atomic_always_lock_free(4, (void*)4), "");
+_Static_assert(__atomic_always_lock_free(4, (void*)-4), "");
+
+
+
#define _AS1 __attribute__((address_space(1)))
#define _AS2 __attribute__((address_space(2)))
>From 3e6440c8faf5a15dc9fe8e0bcbe9c45533373c17 Mon Sep 17 00:00:00 2001
From: James Y Knight <jyknight at google.com>
Date: Fri, 19 Jul 2024 13:15:18 -0400
Subject: [PATCH 2/4] Fix bug with implicit casts from non-pointer types.
---
clang/lib/AST/ExprConstant.cpp | 19 +++++++++++++------
clang/test/Sema/atomic-ops.c | 7 +++++++
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6eb6d27cc9588..5cef336b739dc 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12964,12 +12964,19 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
return Success(1, E);
// Otherwise, check if the type's alignment against Size.
- QualType PointeeType = E->getArg(1)->IgnoreImpCasts()->getType()->
- castAs<PointerType>()->getPointeeType();
- if (!PointeeType->isIncompleteType() &&
- Info.Ctx.getTypeAlignInChars(PointeeType) >= Size) {
- // OK, we will inline operations on this object.
- return Success(1, E);
+ if (auto *ICE = dyn_cast<ImplicitCastExpr>(PtrArg)) {
+ // Drop the potential implicit-cast to 'const volatile void*', getting the underlying type.
+ if (ICE->getCastKind() == CK_BitCast)
+ PtrArg = ICE->getSubExpr();
+ }
+
+ if (auto PtrTy = PtrArg->getType()->getAs<PointerType>()) {
+ QualType PointeeType = PtrTy->getPointeeType();
+ if (!PointeeType->isIncompleteType() &&
+ Info.Ctx.getTypeAlignInChars(PointeeType) >= Size) {
+ // OK, we will inline operations on this object.
+ return Success(1, E);
+ }
}
}
}
diff --git a/clang/test/Sema/atomic-ops.c b/clang/test/Sema/atomic-ops.c
index 86b6ec6ba7a25..2405f804d0da5 100644
--- a/clang/test/Sema/atomic-ops.c
+++ b/clang/test/Sema/atomic-ops.c
@@ -140,6 +140,13 @@ _Static_assert(!__atomic_always_lock_free(4, (void*)-2), "");
_Static_assert(__atomic_always_lock_free(4, (void*)4), "");
_Static_assert(__atomic_always_lock_free(4, (void*)-4), "");
+// Ensure that "weird" constants don't cause trouble.
+_Static_assert(__atomic_always_lock_free(1, "string"), "");
+_Static_assert(!__atomic_always_lock_free(2, "string"), "");
+_Static_assert(__atomic_always_lock_free(2, (int[2]){}), "");
+void dummyfn();
+_Static_assert(__atomic_always_lock_free(2, dummyfn) || 1, "");
+
#define _AS1 __attribute__((address_space(1)))
>From 69e4fa45aabc6445d503be0bc6d459984da1bed2 Mon Sep 17 00:00:00 2001
From: James Y Knight <jyknight at google.com>
Date: Fri, 19 Jul 2024 13:37:17 -0400
Subject: [PATCH 3/4] fix clang-format issue.
---
clang/lib/AST/ExprConstant.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 5cef336b739dc..f3a185ef96597 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12965,7 +12965,8 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
// Otherwise, check if the type's alignment against Size.
if (auto *ICE = dyn_cast<ImplicitCastExpr>(PtrArg)) {
- // Drop the potential implicit-cast to 'const volatile void*', getting the underlying type.
+ // Drop the potential implicit-cast to 'const volatile void*', getting
+ // the underlying type.
if (ICE->getCastKind() == CK_BitCast)
PtrArg = ICE->getSubExpr();
}
>From 4e3c491a8aea990516adcceac0c2b379fca17235 Mon Sep 17 00:00:00 2001
From: James Y Knight <jyknight at google.com>
Date: Mon, 22 Jul 2024 14:17:49 -0400
Subject: [PATCH 4/4] Add release note
---
clang/docs/ReleaseNotes.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8e24087b3dcdb..9a07ad42b8f6f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -440,6 +440,13 @@ Non-comprehensive list of changes in this release
pointers, enabling more powerful alias analysis when accessing pointer types.
The new behavior can be enabled using ``-fpointer-tbaa``.
+- The ``__atomic_always_lock_free`` and ``__atomic_is_lock_free``
+ builtins may now return true if the pointer argument is a
+ compile-time constant (e.g. ``(void*)4``), and constant pointer is
+ sufficiently-aligned for the access requested. Previously, only the
+ type of the pointer was taken into account. This improves
+ compatibility with GCC's libstdc++.
+
New Compiler Flags
------------------
- ``-fsanitize=implicit-bitfield-conversion`` checks implicit truncation and
More information about the cfe-commits
mailing list