r283802 - Change Builtins name to be stored as StringRef instead of raw pointers (NFC)
Yung, Douglas via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 11 14:33:18 PDT 2016
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-bounces at 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.kra at 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.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/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 at lists.llvm.org
> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list