[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
Fri Jul 19 10:37:37 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/3] 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/3] 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/3] 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();
}
More information about the cfe-commits
mailing list