Skip to content

Commit

Permalink
Handle constant "pointers" for __atomic_always_lock_free/`__atomic_…
Browse files Browse the repository at this point in the history
…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).
  • Loading branch information
jyknight authored Jul 22, 2024
1 parent 146fd7c commit 511e93b
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 10 deletions.
7 changes: 7 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 26 additions & 10 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12949,19 +12949,35 @@ 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);

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.
// 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.
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);
}
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions clang/test/Sema/atomic-ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,31 @@ _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), "");

// 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)))
#define _AS2 __attribute__((address_space(2)))

Expand Down

0 comments on commit 511e93b

Please sign in to comment.