r359367 - Reinstate r359059, reverted in r359361, with a fix to properly prevent
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 10 14:22:52 PDT 2019
Thanks, fixed in r371557.
On Mon, 9 Sep 2019 at 19:12, Michael Spencer via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> 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
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190910/bf690207/attachment-0001.html>
More information about the cfe-commits
mailing list