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

Mehdi Amini via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 11 11:40:47 PDT 2016


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



More information about the cfe-commits mailing list