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

[Benjamin Kramer via cfe-commits]

Yup, GCC is "fast enough" again. Thanks :)

On Tue, Oct 11, 2016 at 9:13 PM, Mehdi Amini  wrote:
> 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  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  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  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
  wrote:
> Author: mehdi_amini
> Date: Mon Oct 10 16:34:29 2016
> New Revision: 283802
>
> URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
>Name = getMangledName(D);
>  else
> -Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
> +Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
>
>  llvm::FunctionType *Ty =
>cast(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=283801=283802=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 = 

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

[Yung, Douglas via cfe-commits]

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 
> 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 
> 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 
> 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
> >>>  wrote:
>  Author: mehdi_amini
>  Date: Mon Oct 10 16:34:29 2016
>  New Revision: 283802
> 
>  URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
> Name = getMangledName(D);
>   else
>  -Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
>  +Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
> 
>   llvm::FunctionType *Ty =
> cast(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 

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

[Mehdi Amini via cfe-commits]

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  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  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  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
>>>  wrote:
 Author: mehdi_amini
 Date: Mon Oct 10 16:34:29 2016
 New Revision: 283802
 
 URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
Name = getMangledName(D);
  else
 -Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
 +Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
 
  llvm::FunctionType *Ty =
cast(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=283801=283802=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 {
// 

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

[Mehdi Amini via cfe-commits]

Yes, do you have a specific concern for this table size in particular?

> On Oct 11, 2016, at 12:07 PM, Craig Topper  wrote:
> 
> But this also increases the size of the builtin table too right? Since 
> StringRef is twice the size of a pointer.
> 
> ~Craig
> 
> On Tue, Oct 11, 2016 at 11:40 AM, Mehdi Amini via cfe-commits 
> > 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  > > 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
> > > wrote:
> >> Author: mehdi_amini
> >> Date: Mon Oct 10 16:34:29 2016
> >> New Revision: 283802
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
> >> Name = getMangledName(D);
> >>   else
> >> -Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
> >> +Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
> >>
> >>   llvm::FunctionType *Ty =
> >> cast(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=283801=283802=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 

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

[Mehdi Amini via cfe-commits]

> On Oct 11, 2016, at 12:04 PM, Benjamin Kramer  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  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  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
>>>  wrote:
 Author: mehdi_amini
 Date: Mon Oct 10 16:34:29 2016
 New Revision: 283802
 
 URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
Name = getMangledName(D);
  else
 -Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
 +Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
 
  llvm::FunctionType *Ty =
cast(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=283801=283802=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 = 

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

[Craig Topper via cfe-commits]

But this also increases the size of the builtin table too right? Since
StringRef is twice the size of a pointer.

~Craig

On Tue, Oct 11, 2016 at 11:40 AM, Mehdi Amini via cfe-commits <
cfe-commits@lists.llvm.org> 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 
> 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
> >  wrote:
> >> Author: mehdi_amini
> >> Date: Mon Oct 10 16:34:29 2016
> >> New Revision: 283802
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
> >> Name = getMangledName(D);
> >>   else
> >> -Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
> >> +Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
> >>
> >>   llvm::FunctionType *Ty =
> >> cast(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=283801=283802=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((NewBuiltinName));
> >> LookupResult Res(*this, DN, DRE->getLocStart(), LookupOrdinaryName);
> >> LookupName(Res, TUScope, /*AllowBuiltinCreation=*/true);
> >> @@ 

Re: 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  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  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
>>  wrote:
>>> Author: mehdi_amini
>>> Date: Mon Oct 10 16:34:29 2016
>>> New Revision: 283802
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
>>> Name = getMangledName(D);
>>>   else
>>> -Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
>>> +Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
>>>
>>>   llvm::FunctionType *Ty =
>>> cast(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=283801=283802=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((NewBuiltinName));
>>> LookupResult Res(*this, DN, DRE->getLocStart(), LookupOrdinaryName);
>>> 

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

[Mehdi Amini via cfe-commits]

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  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
>  wrote:
>> Author: mehdi_amini
>> Date: Mon Oct 10 16:34:29 2016
>> New Revision: 283802
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
>> Name = getMangledName(D);
>>   else
>> -Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
>> +Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
>> 
>>   llvm::FunctionType *Ty =
>> cast(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=283801=283802=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((NewBuiltinName));
>> LookupResult Res(*this, DN, DRE->getLocStart(), LookupOrdinaryName);
>> LookupName(Res, TUScope, /*AllowBuiltinCreation=*/true);
>> @@ -6263,7 +6263,7 @@ static void emitReplacement(Sema , 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()) {
>> @@ 

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

[Benjamin Kramer via cfe-commits]

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
 wrote:
> Author: mehdi_amini
> Date: Mon Oct 10 16:34:29 2016
> New Revision: 283802
>
> URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
>  Name = getMangledName(D);
>else
> -Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
> +Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
>
>llvm::FunctionType *Ty =
>  cast(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=283801=283802=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((NewBuiltinName));
>  LookupResult Res(*this, DN, DRE->getLocStart(), LookupOrdinaryName);
>  LookupName(Res, TUScope, /*AllowBuiltinCreation=*/true);
> @@ -6263,7 +6263,7 @@ static void emitReplacement(Sema , 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 =
>  

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

[Mehdi Amini via cfe-commits]

Author: mehdi_amini
Date: Mon Oct 10 16:34:29 2016
New Revision: 283802

URL: http://llvm.org/viewvc/llvm-project?rev=283802=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=283801=283802=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=283801=283802=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())
 Name = getMangledName(D);
   else
-Name = Context.BuiltinInfo.getName(BuiltinID) + 10;
+Name = Context.BuiltinInfo.getName(BuiltinID).drop_front(10);
 
   llvm::FunctionType *Ty =
 cast(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=283801=283802=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((NewBuiltinName));
 LookupResult Res(*this, DN, DRE->getLocStart(), LookupOrdinaryName);
 LookupName(Res, TUScope, /*AllowBuiltinCreation=*/true);
@@ -6263,7 +6263,7 @@ static void emitReplacement(Sema , 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