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
