llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> First, a builtin won't magically become constexpr between two compilations of the same builtin id, so we're re-doing the check over and over again for every call of a builtin function, which is unnecessary. Second, in the case where the builtin _isn't_ constexpr, there was a diagnostic discrepancy between the two evaluators. We now correctly reject the builtin call before even trying to evaluate its arguments. --- Full diff: https://github.com/llvm/llvm-project/pull/227646.diff 3 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+15) - (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+1-20) - (modified) clang/test/AST/ByteCode/builtin-functions.cpp (+10) ``````````diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index f7d2d202b2c9f7..b4a37470c48bde 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -6145,6 +6145,21 @@ bool Compiler<Emitter>::registerRedecl(const VarDecl *VD, const APValue &Val) { template <class Emitter> bool Compiler<Emitter>::VisitBuiltinCallExpr(const CallExpr *E, unsigned BuiltinID) { + const ASTContext &ASTCtx = Ctx.getASTContext(); + + // BuiltinID is the raw ID baked into the bytecode. The "is constant + // evaluated" gate needs the raw ID so that auxiliary-target IDs resolve into + // the correct (aux-target) builtin records. + if (!Ctx.getASTContext().BuiltinInfo.isConstantEvaluated(BuiltinID)) + return this->emitInvalid(E); + + // Convert an auxiliary x86 target builtin ID to its canonical X86::BI* value + // so the target-specific cases below (and the handlers they call) match. This + // is a cheap integer operation (a single comparison for the common, + // target-independent case); we deliberately avoid re-deriving the ID from the + // call expression, which is comparatively slow. + BuiltinID = ConvertBuiltinIDToX86BuiltinID(ASTCtx, BuiltinID); + if (BuiltinID == Builtin::BI__builtin_constant_p) { // Void argument is always invalid and harder to handle later. if (E->getArg(0)->getType()->isVoidType()) { diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp index 843850af334a2b..3303a56f2c053c 100644 --- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp +++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp @@ -4600,21 +4600,6 @@ static bool interp_builtin_ia32_cvt_vector_to_int(InterpState &S, CodePtr OpPC, bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call, uint32_t BuiltinID) { - const ASTContext &ASTCtx = S.getASTContext(); - - // BuiltinID is the raw ID baked into the bytecode. The "is constant - // evaluated" gate needs the raw ID so that auxiliary-target IDs resolve into - // the correct (aux-target) builtin records. - if (!ASTCtx.BuiltinInfo.isConstantEvaluated(BuiltinID)) - return Invalid(S, OpPC); - - // Convert an auxiliary x86 target builtin ID to its canonical X86::BI* value - // so the target-specific cases below (and the handlers they call) match. This - // is a cheap integer operation (a single comparison for the common, - // target-independent case); we deliberately avoid re-deriving the ID from the - // call expression, which is comparatively slow. - BuiltinID = ConvertBuiltinIDToX86BuiltinID(ASTCtx, BuiltinID); - const InterpFrame *Frame = S.Current; switch (BuiltinID) { case Builtin::BI__builtin_is_constant_evaluated: @@ -6723,11 +6708,7 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const CallExpr *Call, case X86::BI__builtin_ia32_cvttps2dq256: return interp_builtin_ia32_cvt_vector_to_int(S, OpPC, Call); default: - S.FFDiag(S.Current->getLocation(OpPC), - diag::note_invalid_subexpr_in_const_expr) - << S.Current->getRange(OpPC); - - return false; + return Invalid(S, OpPC); } llvm_unreachable("Unhandled builtin ID"); diff --git a/clang/test/AST/ByteCode/builtin-functions.cpp b/clang/test/AST/ByteCode/builtin-functions.cpp index 5d8c0fa2b7b6b0..211c3e98d161c4 100644 --- a/clang/test/AST/ByteCode/builtin-functions.cpp +++ b/clang/test/AST/ByteCode/builtin-functions.cpp @@ -2153,3 +2153,13 @@ namespace ReduceMin { typedef float v4f __attribute__((__vector_size__(16))); static_assert(__builtin_reduce_min((v4f){1.123, 2.123, 3.123, 4.123}) == 0); // both-error {{not an integral constant expression}} } + +namespace Rejected { + constexpr int foo() { // both-error {{never produces a constant expression}} + __builtin_alloca(10 / 0); // both-note 2{{subexpression not valid in a constant expression}} \ + // both-warning {{division by zero is undefined}} + return 1; + } + static_assert(foo() == 1); // both-error {{not an integral constant expression}} \ + // both-note {{in call to}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/227646 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
