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