r283802 - Change Builtins name to be stored as StringRef instead of raw pointers (NFC)
Mehdi Amini via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 11 12:09:37 PDT 2016
Yes, do you have a specific concern for this table size in particular?
> On Oct 11, 2016, at 12:07 PM, Craig Topper <craig.topper at 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 at lists.llvm.org <mailto:cfe-commits at 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.kra at gmail.com <mailto:benny.kra at 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 at lists.llvm.org <mailto:cfe-commits at 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 at lists.llvm.org <mailto:cfe-commits at 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 at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://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/20161011/66882a5b/attachment-0001.html>
More information about the cfe-commits
mailing list