r359367 - Reinstate r359059, reverted in r359361, with a fix to properly prevent
Michael Spencer via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 9 19:12:06 PDT 2019
On Fri, Apr 26, 2019 at 7:56 PM Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: rsmith
> Date: Fri Apr 26 19:58:17 2019
> New Revision: 359367
>
> URL: http://llvm.org/viewvc/llvm-project?rev=359367&view=rev
> Log:
> Reinstate r359059, reverted in r359361, with a fix to properly prevent
> us emitting the operand of __builtin_constant_p if it has side-effects.
>
> Original commit message:
>
> Fix interactions between __builtin_constant_p and constexpr to match
> current trunk GCC.
>
> GCC permits information from outside the operand of
> __builtin_constant_p (but in the same constant evaluation context) to be
> used within that operand; clang now does so too. A few other minor
> deviations from GCC's behavior showed up in my testing and are also
> fixed (matching GCC):
> * Clang now supports nullptr_t as the argument type for
> __builtin_constant_p
> * Clang now returns true from __builtin_constant_p if called with a
> null pointer
> * Clang now returns true from __builtin_constant_p if called with an
> integer cast to pointer type
>
> Added:
> cfe/trunk/test/SemaCXX/builtin-constant-p.cpp
> Modified:
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/CodeGen/builtin-constant-p.c
> cfe/trunk/test/SemaCXX/enable_if.cpp
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359367&r1=359366&r2=359367&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Apr 26 19:58:17 2019
> @@ -7801,19 +7801,33 @@ EvaluateBuiltinClassifyType(const CallEx
> }
>
> /// EvaluateBuiltinConstantPForLValue - Determine the result of
> -/// __builtin_constant_p when applied to the given lvalue.
> +/// __builtin_constant_p when applied to the given pointer.
> ///
> -/// An lvalue is only "constant" if it is a pointer or reference to the
> first
> -/// character of a string literal.
> -template<typename LValue>
> -static bool EvaluateBuiltinConstantPForLValue(const LValue &LV) {
> - const Expr *E = LV.getLValueBase().template dyn_cast<const Expr*>();
> - return E && isa<StringLiteral>(E) && LV.getLValueOffset().isZero();
> +/// A pointer is only "constant" if it is null (or a pointer cast to
> integer)
> +/// or it points to the first character of a string literal.
> +static bool EvaluateBuiltinConstantPForLValue(const APValue &LV) {
> + APValue::LValueBase Base = LV.getLValueBase();
> + if (Base.isNull()) {
> + // A null base is acceptable.
> + return true;
> + } else if (const Expr *E = Base.dyn_cast<const Expr *>()) {
> + if (!isa<StringLiteral>(E))
> + return false;
> + return LV.getLValueOffset().isZero();
> + } else {
> + // Any other base is not constant enough for GCC.
> + return false;
> + }
> }
>
> /// EvaluateBuiltinConstantP - Evaluate __builtin_constant_p as similarly
> to
> /// GCC as we can manage.
> -static bool EvaluateBuiltinConstantP(ASTContext &Ctx, const Expr *Arg) {
> +static bool EvaluateBuiltinConstantP(EvalInfo &Info, const Expr *Arg) {
> + // Constant-folding is always enabled for the operand of
> __builtin_constant_p
> + // (even when the enclosing evaluation context otherwise requires a
> strict
> + // language-specific constant expression).
> + FoldConstant Fold(Info, true);
> +
> QualType ArgType = Arg->getType();
>
> // __builtin_constant_p always has one operand. The rules which gcc
> follows
> @@ -7821,34 +7835,30 @@ static bool EvaluateBuiltinConstantP(AST
> //
> // - If the operand is of integral, floating, complex or enumeration
> type,
> // and can be folded to a known value of that type, it returns 1.
> - // - If the operand and can be folded to a pointer to the first
> character
> - // of a string literal (or such a pointer cast to an integral type),
> it
> - // returns 1.
> + // - If the operand can be folded to a pointer to the first character
> + // of a string literal (or such a pointer cast to an integral type)
> + // or to a null pointer or an integer cast to a pointer, it returns
> 1.
> //
> // Otherwise, it returns 0.
> //
> // FIXME: GCC also intends to return 1 for literals of aggregate types,
> but
> - // its support for this does not currently work.
> - if (ArgType->isIntegralOrEnumerationType()) {
> - Expr::EvalResult Result;
> - if (!Arg->EvaluateAsRValue(Result, Ctx) || Result.HasSideEffects)
> + // its support for this did not work prior to GCC 9 and is not yet well
> + // understood.
> + if (ArgType->isIntegralOrEnumerationType() || ArgType->isFloatingType()
> ||
> + ArgType->isAnyComplexType() || ArgType->isPointerType() ||
> + ArgType->isNullPtrType()) {
> + APValue V;
> + if (!::EvaluateAsRValue(Info, Arg, V)) {
> + Fold.keepDiagnostics();
> return false;
> + }
>
> - APValue &V = Result.Val;
> - if (V.getKind() == APValue::Int)
> - return true;
> + // For a pointer (possibly cast to integer), there are special rules.
> if (V.getKind() == APValue::LValue)
> return EvaluateBuiltinConstantPForLValue(V);
> - } else if (ArgType->isFloatingType() || ArgType->isAnyComplexType()) {
> - return Arg->isEvaluatable(Ctx);
> - } else if (ArgType->isPointerType() || Arg->isGLValue()) {
> - LValue LV;
> - Expr::EvalStatus Status;
> - EvalInfo Info(Ctx, Status, EvalInfo::EM_ConstantFold);
> - if ((Arg->isGLValue() ? EvaluateLValue(Arg, LV, Info)
> - : EvaluatePointer(Arg, LV, Info)) &&
> - !Status.HasSideEffects)
> - return EvaluateBuiltinConstantPForLValue(LV);
> +
> + // Otherwise, any constant value is good enough.
> + return V.getKind() != APValue::Uninitialized;
> }
>
> // Anything else isn't considered to be sufficiently constant.
> @@ -8258,18 +8268,15 @@ bool IntExprEvaluator::VisitBuiltinCallE
> }
>
> case Builtin::BI__builtin_constant_p: {
> - auto Arg = E->getArg(0);
> - if (EvaluateBuiltinConstantP(Info.Ctx, Arg))
> + const Expr *Arg = E->getArg(0);
> + if (EvaluateBuiltinConstantP(Info, Arg))
> return Success(true, E);
> - auto ArgTy = Arg->IgnoreImplicit()->getType();
> - if (!Info.InConstantContext && !Arg->HasSideEffects(Info.Ctx) &&
> - !ArgTy->isAggregateType() && !ArgTy->isPointerType()) {
> - // We can delay calculation of __builtin_constant_p until after
> - // inlining. Note: This diagnostic won't be shown to the user.
> + else if (Info.InConstantContext)
> + return Success(false, E);
> + else {
> Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
> return false;
> }
> - return Success(false, E);
> }
>
> case Builtin::BI__builtin_is_constant_evaluated:
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=359367&r1=359366&r2=359367&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Apr 26 19:58:17 2019
> @@ -2027,8 +2027,17 @@ RValue CodeGenFunction::EmitBuiltinExpr(
>
> const Expr *Arg = E->getArg(0);
> QualType ArgType = Arg->getType();
> - if (!hasScalarEvaluationKind(ArgType) || ArgType->isFunctionType())
> - // We can only reason about scalar types.
> + // FIXME: The allowance for Obj-C pointers and block pointers is
> historical
> + // and likely a mistake.
> + if (!ArgType->isIntegralOrEnumerationType() &&
> !ArgType->isFloatingType() &&
> + !ArgType->isObjCObjectPointerType() &&
> !ArgType->isBlockPointerType())
> + // Per the GCC documentation, only numeric constants are recognized
> after
> + // inlining.
> + return RValue::get(ConstantInt::get(ResultType, 0));
> +
> + if (Arg->HasSideEffects(getContext()))
> + // The argument is unevaluated, so be conservative if it might have
> + // side-effects.
> return RValue::get(ConstantInt::get(ResultType, 0));
>
> Value *ArgValue = EmitScalarExpr(Arg);
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=359367&r1=359366&r2=359367&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Apr 26 19:58:17 2019
> @@ -1199,10 +1199,14 @@ Sema::CheckBuiltinFunctionCall(FunctionD
> if (checkArgCount(*this, TheCall, 1)) return true;
> TheCall->setType(Context.IntTy);
> break;
> - case Builtin::BI__builtin_constant_p:
> + case Builtin::BI__builtin_constant_p: {
> if (checkArgCount(*this, TheCall, 1)) return true;
> + ExprResult Arg =
> DefaultFunctionArrayLvalueConversion(TheCall->getArg(0));
> + if (Arg.isInvalid()) return true;
> + TheCall->setArg(0, Arg.get());
> TheCall->setType(Context.IntTy);
> break;
> + }
> case Builtin::BI__builtin_launder:
> return SemaBuiltinLaunder(*this, TheCall);
> case Builtin::BI__sync_fetch_and_add:
>
> Modified: cfe/trunk/test/CodeGen/builtin-constant-p.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtin-constant-p.c?rev=359367&r1=359366&r2=359367&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGen/builtin-constant-p.c (original)
> +++ cfe/trunk/test/CodeGen/builtin-constant-p.c Fri Apr 26 19:58:17 2019
> @@ -177,3 +177,11 @@ void test17() {
> // CHECK: call void asm sideeffect "", {{.*}}(i32 -1)
> __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ?
> 1 : -1));
> }
> +
> +int test18_f();
> +// CHECK: define void @test18
> +// CHECK-NOT: call {{.*}}test18_f
> +void test18() {
> + int a, b;
> + (void)__builtin_constant_p((a = b, test18_f()));
> +}
>
> Added: cfe/trunk/test/SemaCXX/builtin-constant-p.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/builtin-constant-p.cpp?rev=359367&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/builtin-constant-p.cpp (added)
> +++ cfe/trunk/test/SemaCXX/builtin-constant-p.cpp Fri Apr 26 19:58:17 2019
> @@ -0,0 +1,61 @@
> +// RUN: %clang_cc1 -std=c++17 -verify %s
> +
> +using intptr_t = __INTPTR_TYPE__;
> +
> +// Test interaction of constexpr and __builtin_constant_p.
> +
> +template<typename T> constexpr bool bcp(T t) {
> + return __builtin_constant_p(t);
> +}
> +template<typename T> constexpr bool bcp_fold(T t) {
> + return __builtin_constant_p(((void)(intptr_t)&t, t));
> +}
> +
> +constexpr intptr_t ensure_fold_is_generally_not_enabled = //
> expected-error {{constant expression}}
> + (intptr_t)&ensure_fold_is_generally_not_enabled; // expected-note
> {{cast}}
> +
> +constexpr intptr_t ptr_to_int(const void *p) {
> + return __builtin_constant_p(1) ? (intptr_t)p : (intptr_t)p;
> +}
> +
> +constexpr int *int_to_ptr(intptr_t n) {
> + return __builtin_constant_p(1) ? (int*)n : (int*)n;
> +}
> +
> +int x;
> +
> +// Integer and floating point constants encountered during constant
> expression
> +// evaluation are considered constant. So is nullptr_t.
> +static_assert(bcp(1));
> +static_assert(bcp_fold(1));
> +static_assert(bcp(1.0));
> +static_assert(bcp_fold(1.0));
> +static_assert(bcp(nullptr));
> +static_assert(bcp_fold(nullptr));
> +
> +// Pointers to the start of strings are considered constant.
> +static_assert(bcp("foo"));
> +static_assert(bcp_fold("foo"));
> +
> +// Null pointers are considered constant.
> +static_assert(bcp<int*>(nullptr));
> +static_assert(bcp_fold<int*>(nullptr));
> +static_assert(bcp<const char*>(nullptr));
> +static_assert(bcp_fold<const char*>(nullptr));
> +
> +// Other pointers are not.
> +static_assert(!bcp(&x));
> +static_assert(!bcp_fold(&x));
> +
> +// Pointers cast to integers follow the rules for pointers.
> +static_assert(bcp(ptr_to_int("foo")));
> +static_assert(bcp_fold(ptr_to_int("foo")));
> +static_assert(!bcp(ptr_to_int(&x)));
> +static_assert(!bcp_fold(ptr_to_int(&x)));
> +
> +// Integers cast to pointers follow the integer rules.
> +static_assert(bcp(int_to_ptr(0)));
> +static_assert(bcp_fold(int_to_ptr(0)));
> +static_assert(bcp(int_to_ptr(123))); // GCC rejects these due to not
> recognizing
> +static_assert(bcp_fold(int_to_ptr(123))); // the bcp conditional in
> 'int_to_ptr' ...
> +static_assert(__builtin_constant_p((int*)123)); // ... but GCC accepts
> this
>
> Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=359367&r1=359366&r2=359367&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
> +++ cfe/trunk/test/SemaCXX/enable_if.cpp Fri Apr 26 19:58:17 2019
> @@ -522,3 +522,14 @@ void test() {
> InConstantContext::foo("abc");
> }
> } // namespace InConstantContext
> +
> +namespace StringLiteralDetector {
> + void need_string_literal(const char *p)
> __attribute__((enable_if(__builtin_constant_p(p), "argument is not a string
> literal"))); // expected-note 2{{not a string literal}}
> + void test(const char *unknown) {
> + need_string_literal("foo");
> + need_string_literal(unknown); // expected-error {{no matching
> function}}
> + constexpr char str[] = "bar";
> + need_string_literal(str); // expected-error {{no matching function}}
> + }
> +}
> +
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
I bisected a failure to what I believe to be this commit which broke the
following C testcase:
extern int a;
int i[] = {
__builtin_constant_p (a && 0) ? a && 0 : -1
};
In this specific context `__builtin_constant_p (a && 0)` resolves to true,
but the `a && 0` in the true branch of the conditional is not treated as a
constant. If you make it not an array clang behaves properly.
- Michael Spencer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190909/9046ead9/attachment-0001.html>
More information about the cfe-commits
mailing list