Skip to content

Commit d6c34a5

Browse files
jyknightyuxuanchen1997
authored andcommitted
Handle constant "pointers" for __atomic_always_lock_free/__atomic_is_lock_free. (#99340)
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++, and is also useful for libc++'s atomic_ref. Also helps with (but doesn't fix) issue #75081. This also fixes a crash bug, when the second argument was a non-pointer implicitly convertible to a pointer (such as an array, or a function).
1 parent 5fa5a84 commit d6c34a5

File tree

3 files changed

+58
-10
lines changed

3 files changed

+58
-10
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,13 @@ Non-comprehensive list of changes in this release
440440
pointers, enabling more powerful alias analysis when accessing pointer types.
441441
The new behavior can be enabled using ``-fpointer-tbaa``.
442442

443+
- The ``__atomic_always_lock_free`` and ``__atomic_is_lock_free``
444+
builtins may now return true if the pointer argument is a
445+
compile-time constant (e.g. ``(void*)4``), and constant pointer is
446+
sufficiently-aligned for the access requested. Previously, only the
447+
type of the pointer was taken into account. This improves
448+
compatibility with GCC's libstdc++.
449+
443450
New Compiler Flags
444451
------------------
445452
- ``-fsanitize=implicit-bitfield-conversion`` checks implicit truncation and

clang/lib/AST/ExprConstant.cpp

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12949,19 +12949,35 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
1294912949
Info.Ctx.getTargetInfo().getMaxAtomicInlineWidth();
1295012950
if (Size <= Info.Ctx.toCharUnitsFromBits(InlineWidthBits)) {
1295112951
if (BuiltinOp == Builtin::BI__c11_atomic_is_lock_free ||
12952-
Size == CharUnits::One() ||
12953-
E->getArg(1)->isNullPointerConstant(Info.Ctx,
12954-
Expr::NPC_NeverValueDependent))
12955-
// OK, we will inline appropriately-aligned operations of this size,
12956-
// and _Atomic(T) is appropriately-aligned.
12952+
Size == CharUnits::One())
1295712953
return Success(1, E);
1295812954

12959-
QualType PointeeType = E->getArg(1)->IgnoreImpCasts()->getType()->
12960-
castAs<PointerType>()->getPointeeType();
12961-
if (!PointeeType->isIncompleteType() &&
12962-
Info.Ctx.getTypeAlignInChars(PointeeType) >= Size) {
12963-
// OK, we will inline operations on this object.
12955+
// If the pointer argument can be evaluated to a compile-time constant
12956+
// integer (or nullptr), check if that value is appropriately aligned.
12957+
const Expr *PtrArg = E->getArg(1);
12958+
Expr::EvalResult ExprResult;
12959+
APSInt IntResult;
12960+
if (PtrArg->EvaluateAsRValue(ExprResult, Info.Ctx) &&
12961+
ExprResult.Val.toIntegralConstant(IntResult, PtrArg->getType(),
12962+
Info.Ctx) &&
12963+
IntResult.isAligned(Size.getAsAlign()))
1296412964
return Success(1, E);
12965+
12966+
// Otherwise, check if the type's alignment against Size.
12967+
if (auto *ICE = dyn_cast<ImplicitCastExpr>(PtrArg)) {
12968+
// Drop the potential implicit-cast to 'const volatile void*', getting
12969+
// the underlying type.
12970+
if (ICE->getCastKind() == CK_BitCast)
12971+
PtrArg = ICE->getSubExpr();
12972+
}
12973+
12974+
if (auto PtrTy = PtrArg->getType()->getAs<PointerType>()) {
12975+
QualType PointeeType = PtrTy->getPointeeType();
12976+
if (!PointeeType->isIncompleteType() &&
12977+
Info.Ctx.getTypeAlignInChars(PointeeType) >= Size) {
12978+
// OK, we will inline operations on this object.
12979+
return Success(1, E);
12980+
}
1296512981
}
1296612982
}
1296712983
}

clang/test/Sema/atomic-ops.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,31 @@ _Static_assert(__atomic_always_lock_free(4, &i64), "");
124124
_Static_assert(!__atomic_always_lock_free(8, &i32), "");
125125
_Static_assert(__atomic_always_lock_free(8, &i64), "");
126126

127+
// Validate use with fake pointers constants. This mechanism is used to allow
128+
// validating atomicity of a given size and alignment.
129+
_Static_assert(__atomic_is_lock_free(1, (void*)1), "");
130+
_Static_assert(__atomic_is_lock_free(1, (void*)-1), "");
131+
_Static_assert(__atomic_is_lock_free(4, (void*)2), ""); // expected-error {{not an integral constant expression}}
132+
_Static_assert(__atomic_is_lock_free(4, (void*)-2), ""); // expected-error {{not an integral constant expression}}
133+
_Static_assert(__atomic_is_lock_free(4, (void*)4), "");
134+
_Static_assert(__atomic_is_lock_free(4, (void*)-4), "");
135+
136+
_Static_assert(__atomic_always_lock_free(1, (void*)1), "");
137+
_Static_assert(__atomic_always_lock_free(1, (void*)-1), "");
138+
_Static_assert(!__atomic_always_lock_free(4, (void*)2), "");
139+
_Static_assert(!__atomic_always_lock_free(4, (void*)-2), "");
140+
_Static_assert(__atomic_always_lock_free(4, (void*)4), "");
141+
_Static_assert(__atomic_always_lock_free(4, (void*)-4), "");
142+
143+
// Ensure that "weird" constants don't cause trouble.
144+
_Static_assert(__atomic_always_lock_free(1, "string"), "");
145+
_Static_assert(!__atomic_always_lock_free(2, "string"), "");
146+
_Static_assert(__atomic_always_lock_free(2, (int[2]){}), "");
147+
void dummyfn();
148+
_Static_assert(__atomic_always_lock_free(2, dummyfn) || 1, "");
149+
150+
151+
127152
#define _AS1 __attribute__((address_space(1)))
128153
#define _AS2 __attribute__((address_space(2)))
129154

0 commit comments

Comments
 (0)