> On Oct 11, 2016, at 12:04 PM, Benjamin Kramer <benny....@gmail.com> wrote: > > Committing this patch before the constexpr change seems backwards > then?
Well not really because I need to make StringRef(const char *) explicit, so all the others have to go first. > 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? I already landed > 30 patches like this one, I have no problem temporarily reverting this one in particular, but there are others instance of static initializers in the other patches. — Mehdi > > On Tue, Oct 11, 2016 at 8:40 PM, Mehdi Amini <mehdi.am...@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....@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> 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@lists.llvm.org >>>> 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
