Yes, do you have a specific concern for this table size in particular? > On Oct 11, 2016, at 12:07 PM, Craig Topper <craig.top...@gmail.com> wrote: > > But this also increases the size of the builtin table too right? Since > StringRef is twice the size of a pointer. > > ~Craig > > On Tue, Oct 11, 2016 at 11:40 AM, Mehdi Amini via cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: > This is temporary: the last patch of my series of patches adds the constexpr > ctor and remove all these static initializers. > > > On Oct 11, 2016, at 11:26 AM, Benjamin Kramer <benny....@gmail.com > > <mailto:benny....@gmail.com>> wrote: > > > > I don't think this change is worth it. We create huge static arrays > > with Builtin::Info in Builtins.cpp and Targets.cpp, StringRef(const > > char*) is not constexpr (because of strlen). This means you'll get a > > huge generated initialization function for it. We want to reduce the > > number of global initializers in LLVM, not create new ones. > > > > On Mon, Oct 10, 2016 at 11:34 PM, Mehdi Amini via cfe-commits > > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: > >> Author: mehdi_amini > >> Date: Mon Oct 10 16:34:29 2016 > >> New Revision: 283802 > >> > >> URL: http://llvm.org/viewvc/llvm-project?rev=283802&view=rev > >> <http://llvm.org/viewvc/llvm-project?rev=283802&view=rev> > >> Log: > >> Change Builtins name to be stored as StringRef instead of raw pointers > >> (NFC) > >> > >> Modified: > >> cfe/trunk/include/clang/Basic/Builtins.h > >> cfe/trunk/lib/CodeGen/CGBuiltin.cpp > >> cfe/trunk/lib/Sema/SemaChecking.cpp > >> > >> Modified: cfe/trunk/include/clang/Basic/Builtins.h > >> URL: > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.h?rev=283802&r1=283801&r2=283802&view=diff > >> > >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.h?rev=283802&r1=283801&r2=283802&view=diff> > >> ============================================================================== > >> --- cfe/trunk/include/clang/Basic/Builtins.h (original) > >> +++ cfe/trunk/include/clang/Basic/Builtins.h Mon Oct 10 16:34:29 2016 > >> @@ -51,7 +51,8 @@ enum ID { > >> }; > >> > >> struct Info { > >> - const char *Name, *Type, *Attributes, *HeaderName; > >> + llvm::StringRef Name; > >> + const char *Type, *Attributes, *HeaderName; > >> LanguageID Langs; > >> const char *Features; > >> }; > >> @@ -80,7 +81,7 @@ public: > >> > >> /// \brief Return the identifier name for the specified builtin, > >> /// e.g. "__builtin_abs". > >> - const char *getName(unsigned ID) const { > >> + llvm::StringRef getName(unsigned ID) const { > >> return getRecord(ID).Name; > >> } > >> > >> > >> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp > >> URL: > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=283802&r1=283801&r2=283802&view=diff > >> > >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=283802&r1=283801&r2=283802&view=diff> > >> ============================================================================== > >> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) > >> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Oct 10 16:34:29 2016 > >> @@ -50,7 +50,7 @@ llvm::Value *CodeGenModule::getBuiltinLi > >> if (FD->hasAttr<AsmLabelAttr>()) > >> Name = getMangledName(D); > >> else > >> - Name = Context.BuiltinInfo.getName(BuiltinID) + 10; > >> + Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10); > >> > >> llvm::FunctionType *Ty = > >> cast<llvm::FunctionType>(getTypes().ConvertType(FD->getType())); > >> @@ -2523,11 +2523,11 @@ RValue CodeGenFunction::EmitBuiltinExpr( > >> checkTargetFeatures(E, FD); > >> > >> // See if we have a target specific intrinsic. > >> - const char *Name = getContext().BuiltinInfo.getName(BuiltinID); > >> Intrinsic::ID IntrinsicID = Intrinsic::not_intrinsic; > >> StringRef Prefix = > >> llvm::Triple::getArchTypePrefix(getTarget().getTriple().getArch()); > >> if (!Prefix.empty()) { > >> + StringRef Name = getContext().BuiltinInfo.getName(BuiltinID); > >> IntrinsicID = Intrinsic::getIntrinsicForGCCBuiltin(Prefix.data(), > >> Name); > >> // NOTE we dont need to perform a compatibility flag check here since > >> the > >> // intrinsics are declared in Builtins*.def via LANGBUILTIN which > >> filter the > >> > >> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp > >> URL: > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=283802&r1=283801&r2=283802&view=diff > >> > >> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=283802&r1=283801&r2=283802&view=diff> > >> ============================================================================== > >> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) > >> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Oct 10 16:34:29 2016 > >> @@ -3199,12 +3199,12 @@ Sema::SemaBuiltinAtomicOverloaded(ExprRe > >> // Get the decl for the concrete builtin from this, we can tell what the > >> // concrete integer type we should convert to is. > >> unsigned NewBuiltinID = BuiltinIndices[BuiltinIndex][SizeIndex]; > >> - const char *NewBuiltinName = Context.BuiltinInfo.getName(NewBuiltinID); > >> FunctionDecl *NewBuiltinDecl; > >> if (NewBuiltinID == BuiltinID) > >> NewBuiltinDecl = FDecl; > >> else { > >> // Perform builtin lookup to avoid redeclaring it. > >> + StringRef NewBuiltinName = Context.BuiltinInfo.getName(NewBuiltinID); > >> DeclarationName DN(&Context.Idents.get(NewBuiltinName)); > >> LookupResult Res(*this, DN, DRE->getLocStart(), LookupOrdinaryName); > >> LookupName(Res, TUScope, /*AllowBuiltinCreation=*/true); > >> @@ -6263,7 +6263,7 @@ static void emitReplacement(Sema &S, Sou > >> unsigned AbsKind, QualType ArgType) { > >> bool EmitHeaderHint = true; > >> const char *HeaderName = nullptr; > >> - const char *FunctionName = nullptr; > >> + StringRef FunctionName; > >> if (S.getLangOpts().CPlusPlus && !ArgType->isAnyComplexType()) { > >> FunctionName = "std::abs"; > >> if (ArgType->isIntegralOrEnumerationType()) { > >> @@ -6381,7 +6381,7 @@ void Sema::CheckAbsoluteValueFunction(co > >> // Unsigned types cannot be negative. Suggest removing the absolute > >> value > >> // function call. > >> if (ArgType->isUnsignedIntegerType()) { > >> - const char *FunctionName = > >> + StringRef FunctionName = > >> IsStdAbs ? "std::abs" : Context.BuiltinInfo.getName(AbsKind); > >> Diag(Call->getExprLoc(), diag::warn_unsigned_abs) << ArgType << > >> ParamType; > >> Diag(Call->getExprLoc(), diag::note_remove_abs) > >> > >> > >> _______________________________________________ > >> cfe-commits mailing list > >> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits