On Apr 16, 2010, at 7:26 PM, Eric Christopher wrote: > Author: echristo > Date: Fri Apr 16 21:26:23 2010 > New Revision: 101610 > > URL: http://llvm.org/viewvc/llvm-project?rev=101610&view=rev > Log: > Consolidate most of the integer constant expression builtin requirement > checking into a single function and use that throughout. Remove some > now unnecessary diagnostics and update tests with now more accurate > diagnostics.
Thanks for doing this Eric. One stylistic thing: there is no need to say which argument number is problematic, just point to the argument and underline the problematic expression with a source range. Removing the argument # makes it more concise. IOW, please change "argument 0 to '__builtin_return_address' must be a constant integer" to "argument to '__builtin_return_address' must be a constant integer" or better yet, "argument must be a constant integer" if its obvious what the callee is. -Chris > > Modified: > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/Sema/Sema.h > cfe/trunk/lib/Sema/SemaChecking.cpp > cfe/trunk/test/Sema/builtin-prefetch.c > cfe/trunk/test/Sema/builtin-stackaddress.c > cfe/trunk/test/Sema/builtins.c > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=101610&r1=101609&r2=101610&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Apr 16 21:26:23 > 2010 > @@ -2854,22 +2854,15 @@ > "%select{too many|too few}0 elements in vector initialization (expected %1 > elements, have %2)">; > def err_altivec_empty_initializer : Error<"expected initializer">; > > -def err_stack_const_level : Error< > - "level argument for a stack address builtin must be constant">; > - > -def err_prefetch_invalid_arg_type : Error< > - "argument to __builtin_prefetch must be of integer type">; > -def err_prefetch_invalid_arg_ice : Error< > - "argument to __builtin_prefetch must be a constant integer">; > def err_argument_invalid_range : Error< > "argument should be a value from %0 to %1">; > > -def err_object_size_invalid_argument : Error< > - "argument to __builtin_object_size must be a constant integer">; > - > def err_builtin_longjmp_invalid_val : Error< > "argument to __builtin_longjmp must be a constant 1">; > > +def err_constant_integer_arg_type : Error< > + "argument %0 to %1 must be a constant integer">; > + > def ext_mixed_decls_code : Extension< > "ISO C90 forbids mixing declarations and code">; > def err_non_variable_decl_in_for : Error< > > Modified: cfe/trunk/lib/Sema/Sema.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=101610&r1=101609&r2=101610&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/Sema.h (original) > +++ cfe/trunk/lib/Sema/Sema.h Fri Apr 16 21:26:23 2010 > @@ -4352,7 +4352,6 @@ > bool SemaBuiltinVAStart(CallExpr *TheCall); > bool SemaBuiltinUnorderedCompare(CallExpr *TheCall); > bool SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs); > - bool SemaBuiltinStackAddress(CallExpr *TheCall); > > public: > // Used by C++ template instantiation. > @@ -4363,7 +4362,8 @@ > bool SemaBuiltinObjectSize(CallExpr *TheCall); > bool SemaBuiltinLongjmp(CallExpr *TheCall); > bool SemaBuiltinAtomicOverloaded(CallExpr *TheCall); > - bool SemaBuiltinEHReturnDataRegNo(CallExpr *TheCall); > + bool SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum, > + llvm::APSInt &Result); > bool SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall, > bool HasVAListArg, unsigned format_idx, > unsigned firstDataArg); > > Modified: cfe/trunk/lib/Sema/SemaChecking.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=101610&r1=101609&r2=101610&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) > +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Apr 16 21:26:23 2010 > @@ -26,6 +26,7 @@ > #include "clang/Lex/Preprocessor.h" > #include "llvm/ADT/BitVector.h" > #include "llvm/ADT/STLExtras.h" > +#include "clang/Basic/TargetBuiltins.h" > #include <limits> > using namespace clang; > > @@ -155,14 +156,18 @@ > return ExprError(); > break; > case Builtin::BI__builtin_return_address: > - case Builtin::BI__builtin_frame_address: > - if (SemaBuiltinStackAddress(TheCall)) > + case Builtin::BI__builtin_frame_address: { > + llvm::APSInt Result; > + if (SemaBuiltinConstantArg(TheCall, 0, Result)) > return ExprError(); > break; > - case Builtin::BI__builtin_eh_return_data_regno: > - if (SemaBuiltinEHReturnDataRegNo(TheCall)) > + } > + case Builtin::BI__builtin_eh_return_data_regno: { > + llvm::APSInt Result; > + if (SemaBuiltinConstantArg(TheCall, 0, Result)) > return ExprError(); > break; > + } > case Builtin::BI__builtin_shufflevector: > return SemaBuiltinShuffleVector(TheCall); > // TheCall will be freed by the smart pointer here, but that's fine, since > @@ -196,6 +201,15 @@ > if (SemaBuiltinAtomicOverloaded(TheCall)) > return ExprError(); > break; > + > + // Target specific builtins start here. > + case X86::BI__builtin_ia32_palignr128: > + case X86::BI__builtin_ia32_palignr: { > + llvm::APSInt Result; > + if (SemaBuiltinConstantArg(TheCall, 2, Result)) > + return ExprError(); > + break; > + } > } > > return move(TheCallResult); > @@ -606,18 +620,6 @@ > return false; > } > > -bool Sema::SemaBuiltinStackAddress(CallExpr *TheCall) { > - // The signature for these builtins is exact; the only thing we need > - // to check is that the argument is a constant. > - SourceLocation Loc; > - if (!TheCall->getArg(0)->isTypeDependent() && > - !TheCall->getArg(0)->isValueDependent() && > - !TheCall->getArg(0)->isIntegerConstantExpr(Context, &Loc)) > - return Diag(Loc, diag::err_stack_const_level) << > TheCall->getSourceRange(); > - > - return false; > -} > - > /// SemaBuiltinShuffleVector - Handle __builtin_shufflevector. > // This is declared to take (...), so we have to check everything. > Action::OwningExprResult Sema::SemaBuiltinShuffleVector(CallExpr *TheCall) { > @@ -668,11 +670,9 @@ > TheCall->getArg(i)->isValueDependent()) > continue; > > - llvm::APSInt Result(32); > - if (!TheCall->getArg(i)->isIntegerConstantExpr(Result, Context)) > - return ExprError(Diag(TheCall->getLocStart(), > - diag::err_shufflevector_nonconstant_argument) > - << TheCall->getArg(i)->getSourceRange()); > + llvm::APSInt Result; > + if (SemaBuiltinConstantArg(TheCall, i, Result)) > + return ExprError(); > > if (Result.getActiveBits() > 64 || Result.getZExtValue() >= numElements*2) > return ExprError(Diag(TheCall->getLocStart(), > @@ -709,23 +709,10 @@ > // constant integers. > for (unsigned i = 1; i != NumArgs; ++i) { > Expr *Arg = TheCall->getArg(i); > - if (Arg->isTypeDependent()) > - continue; > - > - if (!Arg->getType()->isIntegralType()) > - return Diag(TheCall->getLocStart(), > diag::err_prefetch_invalid_arg_type) > - << Arg->getSourceRange(); > - > - ImpCastExprToType(Arg, Context.IntTy, CastExpr::CK_IntegralCast); > - TheCall->setArg(i, Arg); > - > - if (Arg->isValueDependent()) > - continue; > - > + > llvm::APSInt Result; > - if (!Arg->isIntegerConstantExpr(Result, Context)) > - return Diag(TheCall->getLocStart(), diag::err_prefetch_invalid_arg_ice) > - << SourceRange(Arg->getLocStart(), Arg->getLocEnd()); > + if (SemaBuiltinConstantArg(TheCall, i, Result)) > + return true; > > // FIXME: gcc issues a warning and rewrites these to 0. These > // seems especially odd for the third argument since the default > @@ -744,42 +731,35 @@ > return false; > } > > -/// SemaBuiltinEHReturnDataRegNo - Handle __builtin_eh_return_data_regno, the > -/// operand must be an integer constant. > -bool Sema::SemaBuiltinEHReturnDataRegNo(CallExpr *TheCall) { > - llvm::APSInt Result; > - if (!TheCall->getArg(0)->isIntegerConstantExpr(Result, Context)) > - return Diag(TheCall->getLocStart(), diag::err_expr_not_ice) > - << TheCall->getArg(0)->getSourceRange(); > - > +/// SemaBuiltinConstantArg - Handle a check if argument ArgNum of CallExpr > +/// TheCall is a constant expression. > +bool Sema::SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum, > + llvm::APSInt &Result) { > + Expr *Arg = TheCall->getArg(ArgNum); > + DeclRefExpr *DRE > =cast<DeclRefExpr>(TheCall->getCallee()->IgnoreParenCasts()); > + FunctionDecl *FDecl = cast<FunctionDecl>(DRE->getDecl()); > + > + if (Arg->isTypeDependent() || Arg->isValueDependent()) return false; > + > + if (!Arg->isIntegerConstantExpr(Result, Context)) > + return Diag(TheCall->getLocStart(), diag::err_constant_integer_arg_type) > + << ArgNum << FDecl->getDeclName() << Arg->getSourceRange(); > + > return false; > } > > - > /// SemaBuiltinObjectSize - Handle __builtin_object_size(void *ptr, > /// int type). This simply type checks that type is one of the defined > /// constants (0-3). > // For compatability check 0-3, llvm only handles 0 and 2. > bool Sema::SemaBuiltinObjectSize(CallExpr *TheCall) { > - Expr *Arg = TheCall->getArg(1); > - if (Arg->isTypeDependent()) > - return false; > - > - QualType ArgType = Arg->getType(); > - const BuiltinType *BT = ArgType->getAs<BuiltinType>(); > - llvm::APSInt Result(32); > - if (!BT || BT->getKind() != BuiltinType::Int) > - return Diag(TheCall->getLocStart(), > diag::err_object_size_invalid_argument) > - << SourceRange(Arg->getLocStart(), Arg->getLocEnd()); > - > - if (Arg->isValueDependent()) > - return false; > - > - if (!Arg->isIntegerConstantExpr(Result, Context)) { > - return Diag(TheCall->getLocStart(), > diag::err_object_size_invalid_argument) > - << SourceRange(Arg->getLocStart(), Arg->getLocEnd()); > - } > + llvm::APSInt Result; > + > + // Check constant-ness first. > + if (SemaBuiltinConstantArg(TheCall, 1, Result)) > + return true; > > + Expr *Arg = TheCall->getArg(1); > if (Result.getSExtValue() < 0 || Result.getSExtValue() > 3) { > return Diag(TheCall->getLocStart(), diag::err_argument_invalid_range) > << "0" << "3" << SourceRange(Arg->getLocStart(), > Arg->getLocEnd()); > @@ -792,11 +772,13 @@ > /// This checks that val is a constant 1. > bool Sema::SemaBuiltinLongjmp(CallExpr *TheCall) { > Expr *Arg = TheCall->getArg(1); > - if (Arg->isTypeDependent() || Arg->isValueDependent()) > - return false; > + llvm::APSInt Result; > > - llvm::APSInt Result(32); > - if (!Arg->isIntegerConstantExpr(Result, Context) || Result != 1) > + // TODO: This is less than ideal. Overload this to take a value. > + if (SemaBuiltinConstantArg(TheCall, 1, Result)) > + return true; > + > + if (Result != 1) > return Diag(TheCall->getLocStart(), diag::err_builtin_longjmp_invalid_val) > << SourceRange(Arg->getLocStart(), Arg->getLocEnd()); > > > Modified: cfe/trunk/test/Sema/builtin-prefetch.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-prefetch.c?rev=101610&r1=101609&r2=101610&view=diff > ============================================================================== > --- cfe/trunk/test/Sema/builtin-prefetch.c (original) > +++ cfe/trunk/test/Sema/builtin-prefetch.c Fri Apr 16 21:26:23 2010 > @@ -6,8 +6,8 @@ > __builtin_prefetch(&a, 1); > __builtin_prefetch(&a, 1, 2); > __builtin_prefetch(&a, 1, 9, 3); // expected-error{{too many arguments to > function}} > - __builtin_prefetch(&a, "hello", 2); // expected-error{{argument to > __builtin_prefetch must be of integer type}} > - __builtin_prefetch(&a, a, 2); // expected-error{{argument to > __builtin_prefetch must be a constant integer}} > + __builtin_prefetch(&a, "hello", 2); // expected-error{{argument 1 to > '__builtin_prefetch' must be a constant integer}} > + __builtin_prefetch(&a, a, 2); // expected-error{{argument 1 to > '__builtin_prefetch' must be a constant integer}} > __builtin_prefetch(&a, 2); // expected-error{{argument should be a value > from 0 to 1}} > __builtin_prefetch(&a, 0, 4); // expected-error{{argument should be a value > from 0 to 3}} > __builtin_prefetch(&a, -1, 4); // expected-error{{argument should be a > value from 0 to 1}} > > Modified: cfe/trunk/test/Sema/builtin-stackaddress.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-stackaddress.c?rev=101610&r1=101609&r2=101610&view=diff > ============================================================================== > --- cfe/trunk/test/Sema/builtin-stackaddress.c (original) > +++ cfe/trunk/test/Sema/builtin-stackaddress.c Fri Apr 16 21:26:23 2010 > @@ -4,7 +4,7 @@ > } > > void b(unsigned x) { > -return __builtin_return_address(x); // expected-error{{the level argument > for a stack address builtin must be constant}} > +return __builtin_return_address(x); // expected-error{{argument 0 to > '__builtin_return_address' must be a constant integer}} > } > > void* c(unsigned x) { > @@ -12,5 +12,5 @@ > } > > void d(unsigned x) { > -return __builtin_frame_address(x); // expected-error{{the level argument for > a stack address builtin must be constant}} > +return __builtin_frame_address(x); // expected-error{{argument 0 to > '__builtin_frame_address' must be a constant integer}} > } > > Modified: cfe/trunk/test/Sema/builtins.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins.c?rev=101610&r1=101609&r2=101610&view=diff > ============================================================================== > --- cfe/trunk/test/Sema/builtins.c (original) > +++ cfe/trunk/test/Sema/builtins.c Fri Apr 16 21:26:23 2010 > @@ -60,7 +60,7 @@ > break; > } > > - __builtin_eh_return_data_regno(X); // expected-error {{not an integer > constant expression}} > + __builtin_eh_return_data_regno(X); // expected-error {{argument 0 to > '__builtin_eh_return_data_regno' must be a constant integer}} > } > > // PR5062 > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
