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