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

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 11 11:26:59 PDT 2016


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