r283802 - Change Builtins name to be stored as StringRef instead of raw pointers (NFC)

[Benjamin Kramer via cfe-commits]

Committing this patch before the constexpr change seems backwards
then? The static initializers are already breaking stuff because it
takes GCC with optimization and debug info takes 10+ minutes to
generate megabytes of static initializer code in Targets.cpp. Can you
please revert this until the constexpr change is ready?

On Tue, Oct 11, 2016 at 8:40 PM, Mehdi Amini <mehdi.amini at apple.com> 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> 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> 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
>>> 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
>>> ==============================================================================
>>> --- 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
>>> ==============================================================================
>>> --- 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
>>> ==============================================================================
>>> --- 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
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>