We noticed that this change also caused VS2015 to take a lot longer when building Targets.cpp. The revert in r283920 seems to have fixed it. The upstream PS4 Windows bot went from a build time of 17:53 (http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/12771) to 5:51 (http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/12772).
Douglas Yung > -----Original Message----- > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf > Of Mehdi Amini via cfe-commits > Sent: Tuesday, October 11, 2016 12:14 > To: Benjamin Kramer > Cc: cfe-commits > Subject: Re: r283802 - Change Builtins name to be stored as StringRef > instead of raw pointers (NFC) > > Reverted in r283920, can you check if it is enough to “fix” the GCC > issue? > > > On Oct 11, 2016, at 12:04 PM, Benjamin Kramer <benny....@gmail.com> > wrote: > > > > 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.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/B > >>>> uiltins.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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits