Thanks, fixed in r371557. On Mon, 9 Sep 2019 at 19:12, Michael Spencer via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> On Fri, Apr 26, 2019 at 7:56 PM Richard Smith via cfe-commits < > cfe-commits@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@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@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
