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 14:55:12 PDT 2016


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

On Tue, Oct 11, 2016 at 9:13 PM, Mehdi Amini <mehdi.amini at apple.com> 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 <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/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