Switch Neon, SVE, SME, and RVV builtins to use a prefix

chandlerc · chandlerc · commit 2bcc4e5f7043 · 2024-12-19T08:34:55.000Z
This avoids repeating this part of the name in every string, shrinking
the string tables. For SVE in particular, which is by-far the largest
builtin string table, this gets us well under 200KiB. Others shrink by
30% - 50% depending on how long the rest of the strings end up.

Overall, this completes restructuring the builtin string tables to try
and minimize their size and hopefully avoid both toolchain bugs and
compile-time memory overheads of the full sized string tables.
diff --git a/clang/include/clang/Basic/Builtins.h b/clang/include/clang/Basic/Builtins.h
@@ -70,6 +70,8 @@ enum ID {
   FirstTSBuiltin
 };
 
+struct InfosShard;
+
 /// The info used to represent each builtin.
 struct Info {
   // Rather than store pointers to the string literals describing these four
@@ -83,6 +85,11 @@ struct Info {
 
   HeaderDesc Header;
   LanguageID Langs;
+
+  /// Get the name for the builtin represented by this `Info` object.
+  ///
+  /// Must be provided the `Shard` for this `Info` object.
+  std::string getName(const InfosShard &Shard) const;
 };
 
 /// A constexpr function to construct an infos array from X-macros.
@@ -120,6 +127,8 @@ static constexpr std::array<Info, N> MakeInfos(std::array<Info, N> Infos) {
 struct InfosShard {
   const llvm::StringTable *Strings;
   llvm::ArrayRef<Info> Infos;
+
+  llvm::StringLiteral NamePrefix = "";
 };
 
 // A detail macro used below to emit a string literal that, after string literal
@@ -235,7 +244,11 @@ class Context {
 
   /// Return the identifier name for the specified builtin,
   /// e.g. "__builtin_abs".
-  llvm::StringRef getName(unsigned ID) const;
+  std::string getName(unsigned ID) const;
+
+  /// Return the identifier name for the specified builtin inside single quotes
+  /// for a diagnostic, e.g. "'__builtin_abs'".
+  std::string getQuotedName(unsigned ID) const;
 
   /// Get the type descriptor string for the specified builtin.
   const char *getTypeString(unsigned ID) const;
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -154,7 +154,7 @@ static void diagnoseNonConstexprBuiltin(InterpState &S, CodePtr OpPC,
   if (S.getLangOpts().CPlusPlus11)
     S.CCEDiag(Loc, diag::note_constexpr_invalid_function)
         << /*isConstexpr=*/0 << /*isConstructor=*/0
-        << ("'" + S.getASTContext().BuiltinInfo.getName(ID) + "'").str();
+        << S.getASTContext().BuiltinInfo.getQuotedName(ID);
   else
     S.CCEDiag(Loc, diag::note_invalid_subexpr_in_const_expr);
 }
@@ -1936,7 +1936,7 @@ static bool interp__builtin_memcmp(InterpState &S, CodePtr OpPC,
                   !isOneByteCharacterType(PtrB.getType()))) {
     S.FFDiag(S.Current->getSource(OpPC),
              diag::note_constexpr_memcmp_unsupported)
-        << ("'" + ASTCtx.BuiltinInfo.getName(ID) + "'").str() << PtrA.getType()
+        << ASTCtx.BuiltinInfo.getQuotedName(ID) << PtrA.getType()
         << PtrB.getType();
     return false;
   }
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
@@ -9858,7 +9858,7 @@ bool PointerExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
     if (Info.getLangOpts().CPlusPlus11)
       Info.CCEDiag(E, diag::note_constexpr_invalid_function)
           << /*isConstexpr*/ 0 << /*isConstructor*/ 0
-          << ("'" + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'").str();
+          << Info.Ctx.BuiltinInfo.getQuotedName(BuiltinOp);
     else
       Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
     [[fallthrough]];
@@ -9903,8 +9903,7 @@ bool PointerExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
     // FIXME: We can compare the bytes in the correct order.
     if (IsRawByte && !isOneByteCharacterType(CharTy)) {
       Info.FFDiag(E, diag::note_constexpr_memchr_unsupported)
-          << ("'" + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'").str()
-          << CharTy;
+          << Info.Ctx.BuiltinInfo.getQuotedName(BuiltinOp) << CharTy;
       return false;
     }
     // Figure out what value we're actually looking for (after converting to
@@ -9966,7 +9965,7 @@ bool PointerExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
     if (Info.getLangOpts().CPlusPlus11)
       Info.CCEDiag(E, diag::note_constexpr_invalid_function)
           << /*isConstexpr*/ 0 << /*isConstructor*/ 0
-          << ("'" + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'").str();
+          << Info.Ctx.BuiltinInfo.getQuotedName(BuiltinOp);
     else
       Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
     [[fallthrough]];
@@ -13241,7 +13240,7 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
     if (Info.getLangOpts().CPlusPlus11)
       Info.CCEDiag(E, diag::note_constexpr_invalid_function)
           << /*isConstexpr*/ 0 << /*isConstructor*/ 0
-          << ("'" + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'").str();
+          << Info.Ctx.BuiltinInfo.getQuotedName(BuiltinOp);
     else
       Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
     [[fallthrough]];
@@ -13266,7 +13265,7 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
     if (Info.getLangOpts().CPlusPlus11)
       Info.CCEDiag(E, diag::note_constexpr_invalid_function)
           << /*isConstexpr*/ 0 << /*isConstructor*/ 0
-          << ("'" + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'").str();
+          << Info.Ctx.BuiltinInfo.getQuotedName(BuiltinOp);
     else
       Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
     [[fallthrough]];
@@ -13321,8 +13320,8 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E,
         !(isOneByteCharacterType(CharTy1) && isOneByteCharacterType(CharTy2))) {
       // FIXME: Consider using our bit_cast implementation to support this.
       Info.FFDiag(E, diag::note_constexpr_memcmp_unsupported)
-          << ("'" + Info.Ctx.BuiltinInfo.getName(BuiltinOp) + "'").str()
-          << CharTy1 << CharTy2;
+          << Info.Ctx.BuiltinInfo.getQuotedName(BuiltinOp) << CharTy1
+          << CharTy2;
       return false;
     }
 
diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp
@@ -75,11 +75,22 @@ Builtin::Context::getShardAndInfo(unsigned ID) const {
   llvm_unreachable("Invalid target builtin shard structure!");
 }
 
+std::string Builtin::Info::getName(const Builtin::InfosShard &Shard) const {
+  return (Twine(Shard.NamePrefix) + (*Shard.Strings)[Offsets.Name]).str();
+}
+
 /// Return the identifier name for the specified builtin,
 /// e.g. "__builtin_abs".
-llvm::StringRef Builtin::Context::getName(unsigned ID) const {
+std::string Builtin::Context::getName(unsigned ID) const {
+  const auto &[Shard, I] = getShardAndInfo(ID);
+  return I.getName(Shard);
+}
+
+std::string Builtin::Context::getQuotedName(unsigned ID) const {
   const auto &[Shard, I] = getShardAndInfo(ID);
-  return (*Shard.Strings)[I.Offsets.Name];
+  return (Twine("'") + Shard.NamePrefix + (*Shard.Strings)[I.Offsets.Name] +
+          "'")
+      .str();
 }
 
 const char *Builtin::Context::getTypeString(unsigned ID) const {
@@ -116,12 +127,14 @@ void Builtin::Context::InitializeTarget(const TargetInfo &Target,
 bool Builtin::Context::isBuiltinFunc(llvm::StringRef FuncName) {
   bool InStdNamespace = FuncName.consume_front("std-");
   for (const auto &Shard : {InfosShard{&BuiltinStrings, BuiltinInfos}})
-    for (const auto &I : Shard.Infos)
-      if (FuncName == (*Shard.Strings)[I.Offsets.Name] &&
-          (bool)strchr((*Shard.Strings)[I.Offsets.Attributes].data(), 'z') ==
-              InStdNamespace)
-        return strchr((*Shard.Strings)[I.Offsets.Attributes].data(), 'f') !=
-               nullptr;
+    if (llvm::StringRef FuncNameSuffix = FuncName;
+        FuncNameSuffix.consume_front(Shard.NamePrefix))
+      for (const auto &I : Shard.Infos)
+        if (FuncNameSuffix == (*Shard.Strings)[I.Offsets.Name] &&
+            (bool)strchr((*Shard.Strings)[I.Offsets.Attributes].data(), 'z') ==
+                InStdNamespace)
+          return strchr((*Shard.Strings)[I.Offsets.Attributes].data(), 'f') !=
+                 nullptr;
 
   return false;
 }
@@ -197,7 +210,7 @@ void Builtin::Context::initializeBuiltins(IdentifierTable &Table,
       for (const auto &I : Shard.Infos) {
         // If this is a real builtin (ID != 0) and is supported, add it.
         if (ID != 0 && builtinIsSupported(*Shard.Strings, I, LangOpts))
-          Table.get((*Shard.Strings)[I.Offsets.Name]).setBuiltinID(ID);
+          Table.get(I.getName(Shard)).setBuiltinID(ID);
         ++ID;
       }
     assert(ID == FirstTSBuiltin && "Should have added all non-target IDs!");
@@ -206,14 +219,14 @@ void Builtin::Context::initializeBuiltins(IdentifierTable &Table,
     for (const auto &Shard : TargetShards)
       for (const auto &I : Shard.Infos) {
         if (builtinIsSupported(*Shard.Strings, I, LangOpts))
-          Table.get((*Shard.Strings)[I.Offsets.Name]).setBuiltinID(ID);
+          Table.get(I.getName(Shard)).setBuiltinID(ID);
         ++ID;
       }
 
     // Step #3: Register target-specific builtins for AuxTarget.
     for (const auto &Shard : AuxTargetShards)
       for (const auto &I : Shard.Infos) {
-        Table.get((*Shard.Strings)[I.Offsets.Name]).setBuiltinID(ID);
+        Table.get(I.getName(Shard)).setBuiltinID(ID);
         ++ID;
       }
   }
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
@@ -770,11 +770,12 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
 llvm::SmallVector<Builtin::InfosShard>
 AArch64TargetInfo::getTargetBuiltins() const {
   return {
-      {&NEON::BuiltinStrings, NEON::BuiltinInfos},
-      {&NEON::FP16::BuiltinStrings, NEON::FP16::BuiltinInfos},
-      {&SVE::BuiltinStrings, SVE::BuiltinInfos},
+      {&NEON::BuiltinStrings, NEON::BuiltinInfos, "__builtin_neon_"},
+      {&NEON::FP16::BuiltinStrings, NEON::FP16::BuiltinInfos,
+       "__builtin_neon_"},
+      {&SVE::BuiltinStrings, SVE::BuiltinInfos, "__builtin_sve_"},
       {&BuiltinSVENeonBridgeStrings, BuiltinSVENeonBridgeInfos},
-      {&SME::BuiltinStrings, SME::BuiltinInfos},
+      {&SME::BuiltinStrings, SME::BuiltinInfos, "__builtin_sme_"},
       {&BuiltinAArch64Strings, BuiltinAArch64Infos},
   };
 }
diff --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
@@ -1126,8 +1126,9 @@ static constexpr auto BuiltinInfos = Builtin::MakeInfos<NumARMBuiltins>({
 llvm::SmallVector<Builtin::InfosShard>
 ARMTargetInfo::getTargetBuiltins() const {
   return {
-      {&NEON::BuiltinStrings, NEON::BuiltinInfos},
-      {&NEON::FP16::BuiltinStrings, NEON::FP16::BuiltinInfos},
+      {&NEON::BuiltinStrings, NEON::BuiltinInfos, "__builtin_neon_"},
+      {&NEON::FP16::BuiltinStrings, NEON::FP16::BuiltinInfos,
+       "__builtin_neon_"},
       {&BuiltinStrings, BuiltinInfos},
   };
 }
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
@@ -255,7 +255,6 @@ namespace RVV {
 #define GET_RISCVV_BUILTIN_STR_TABLE
 #include "clang/Basic/riscv_vector_builtins.inc"
 #undef GET_RISCVV_BUILTIN_STR_TABLE
-static_assert(BuiltinStrings.size() < 100'000);
 
 static constexpr std::array<Builtin::Info, NumRVVBuiltins> BuiltinInfos = {
 #define GET_RISCVV_BUILTIN_INFOS
@@ -293,8 +292,8 @@ static constexpr auto BuiltinInfos = Builtin::MakeInfos<NumRISCVBuiltins>({
 llvm::SmallVector<Builtin::InfosShard>
 RISCVTargetInfo::getTargetBuiltins() const {
   return {
-      {&RVV::BuiltinStrings, RVV::BuiltinInfos},
-      {&RVVSiFive::BuiltinStrings, RVVSiFive::BuiltinInfos},
+      {&RVV::BuiltinStrings, RVV::BuiltinInfos, "__builtin_rvv_"},
+      {&RVVSiFive::BuiltinStrings, RVVSiFive::BuiltinInfos, "__builtin_rvv_"},
       {&BuiltinStrings, BuiltinInfos},
   };
 }
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -263,8 +263,10 @@ llvm::Constant *CodeGenModule::getBuiltinLibFunction(const FunctionDecl *FD,
                                                      unsigned BuiltinID) {
   assert(Context.BuiltinInfo.isLibFunction(BuiltinID));
 
-  // Get the name, skip over the __builtin_ prefix (if necessary).
-  StringRef Name;
+  // Get the name, skip over the __builtin_ prefix (if necessary). We may have
+  // to build this up so provide a small stack buffer to handle the vast
+  // majority of names.
+  llvm::SmallString<64> Name;
   GlobalDecl D(FD);
 
   // TODO: This list should be expanded or refactored after all GCC-compatible
@@ -6566,7 +6568,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     LargestVectorWidth = std::max(LargestVectorWidth, VectorWidth);
 
   // See if we have a target specific intrinsic.
-  StringRef Name = getContext().BuiltinInfo.getName(BuiltinID);
+  std::string Name = getContext().BuiltinInfo.getName(BuiltinID);
   Intrinsic::ID IntrinsicID = Intrinsic::not_intrinsic;
   StringRef Prefix =
       llvm::Triple::getArchTypePrefix(getTarget().getTriple().getArch());
@@ -21213,7 +21215,7 @@ static Value *MakeHalfType(unsigned IntrinsicID, unsigned BuiltinID,
   auto &C = CGF.CGM.getContext();
   if (!(C.getLangOpts().NativeHalfType ||
         !C.getTargetInfo().useFP16ConversionIntrinsics())) {
-    CGF.CGM.Error(E->getExprLoc(), C.BuiltinInfo.getName(BuiltinID).str() +
+    CGF.CGM.Error(E->getExprLoc(), C.BuiltinInfo.getQuotedName(BuiltinID) +
                                        " requires native half type support.");
     return nullptr;
   }
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3983,7 +3983,8 @@ namespace {
       unsigned BuiltinID = FD->getBuiltinID();
       if (!BuiltinID || !BI.isLibFunction(BuiltinID))
         return false;
-      StringRef BuiltinName = BI.getName(BuiltinID);
+      std::string BuiltinNameStr = BI.getName(BuiltinID);
+      StringRef BuiltinName = BuiltinNameStr;
       if (BuiltinName.starts_with("__builtin_") &&
           Name == BuiltinName.slice(strlen("__builtin_"), StringRef::npos)) {
         return true;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
@@ -1235,7 +1235,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
   bool IsChkVariant = false;
 
   auto GetFunctionName = [&]() {
-    StringRef FunctionName = getASTContext().BuiltinInfo.getName(BuiltinID);
+    std::string FunctionNameStr =
+        getASTContext().BuiltinInfo.getName(BuiltinID);
+    llvm::StringRef FunctionName = FunctionNameStr;
     // Skim off the details of whichever builtin was called to produce a better
     // diagnostic, as it's unlikely that the user wrote the __builtin
     // explicitly.
@@ -1245,7 +1247,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
     } else {
       FunctionName.consume_front("__builtin_");
     }
-    return FunctionName;
+    return FunctionName.str();
   };
 
   switch (BuiltinID) {
@@ -1289,7 +1291,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
                         unsigned SourceSize) {
       DiagID = diag::warn_fortify_scanf_overflow;
       unsigned Index = ArgIndex + DataIndex;
-      StringRef FunctionName = GetFunctionName();
+      std::string FunctionName = GetFunctionName();
       DiagRuntimeBehavior(TheCall->getArg(Index)->getBeginLoc(), TheCall,
                           PDiag(DiagID) << FunctionName << (Index + 1)
                                         << DestSize << SourceSize);
@@ -1438,7 +1440,7 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
       llvm::APSInt::compareValues(*SourceSize, *DestinationSize) <= 0)
     return;
 
-  StringRef FunctionName = GetFunctionName();
+  std::string FunctionName = GetFunctionName();
 
   SmallString<16> DestinationStr;
   SmallString<16> SourceStr;
@@ -4546,7 +4548,7 @@ ExprResult Sema::BuiltinAtomicOverloaded(ExprResult TheCallResult) {
   // 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];
-  StringRef NewBuiltinName = Context.BuiltinInfo.getName(NewBuiltinID);
+  std::string NewBuiltinName = Context.BuiltinInfo.getName(NewBuiltinID);
   FunctionDecl *NewBuiltinDecl;
   if (NewBuiltinID == BuiltinID)
     NewBuiltinDecl = FDecl;
@@ -8332,7 +8334,7 @@ static void emitReplacement(Sema &S, SourceLocation Loc, SourceRange Range,
                             unsigned AbsKind, QualType ArgType) {
   bool EmitHeaderHint = true;
   const char *HeaderName = nullptr;
-  StringRef FunctionName;
+  std::string FunctionName;
   if (S.getLangOpts().CPlusPlus && !ArgType->isAnyComplexType()) {
     FunctionName = "std::abs";
     if (ArgType->isIntegralOrEnumerationType()) {
@@ -8481,7 +8483,7 @@ void Sema::CheckAbsoluteValueFunction(const CallExpr *Call,
   // Unsigned types cannot be negative.  Suggest removing the absolute value
   // function call.
   if (ArgType->isUnsignedIntegerType()) {
-    StringRef FunctionName =
+    std::string FunctionName =
         IsStdAbs ? "std::abs" : Context.BuiltinInfo.getName(AbsKind);
     Diag(Call->getExprLoc(), diag::warn_unsigned_abs) << ArgType << ParamType;
     Diag(Call->getExprLoc(), diag::note_remove_abs)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
@@ -6687,7 +6687,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc,
 
 Expr *Sema::BuildBuiltinCallExpr(SourceLocation Loc, Builtin::ID Id,
                                  MultiExprArg CallArgs) {
-  StringRef Name = Context.BuiltinInfo.getName(Id);
+  std::string Name = Context.BuiltinInfo.getName(Id);
   LookupResult R(*this, &Context.Idents.get(Name), Loc,
                  Sema::LookupOrdinaryName);
   LookupName(R, TUScope, /*AllowBuiltinCreation=*/true);
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp b/clang/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -55,7 +55,7 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl *FD,
   if (BId != 0) {
     if (Name.empty())
       return true;
-    StringRef BName = FD->getASTContext().BuiltinInfo.getName(BId);
+    std::string BName = FD->getASTContext().BuiltinInfo.getName(BId);
     size_t start = BName.find(Name);
     if (start != StringRef::npos) {
       // Accept exact match.
diff --git a/clang/utils/TableGen/NeonEmitter.cpp b/clang/utils/TableGen/NeonEmitter.cpp
@@ -2068,16 +2068,12 @@ void NeonEmitter::genBuiltinsDef(raw_ostream &OS,
   Table.GetOrAddStringOffset("");
   Table.GetOrAddStringOffset("n");
 
-  auto PrefixName = [](Intrinsic *Def) -> std::string {
-    return (llvm::Twine("__builtin_neon_") + Def->getMangledName()).str();
-  };
-
   for (auto *Def : Defs) {
     if (Def->hasBody())
       continue;
 
     if (Builtins.insert({Def->getMangledName(), Def}).second) {
-      Table.GetOrAddStringOffset(PrefixName(Def));
+      Table.GetOrAddStringOffset(Def->getMangledName());
       Table.GetOrAddStringOffset(Def->getBuiltinTypeStr());
       Table.GetOrAddStringOffset(Def->getTargetGuard());
     }
@@ -2097,8 +2093,8 @@ void NeonEmitter::genBuiltinsDef(raw_ostream &OS,
   OS << "#ifdef GET_NEON_BUILTIN_INFOS\n";
   for (const auto &[Name, Def] : Builtins) {
     OS << "    Builtin::Info{Builtin::Info::StrOffsets{"
-       << Table.GetStringOffset(PrefixName(Def)) << " /* " << PrefixName(Def)
-       << " */, ";
+       << Table.GetStringOffset(Def->getMangledName()) << " /* "
+       << Def->getMangledName() << " */, ";
     OS << Table.GetStringOffset(Def->getBuiltinTypeStr()) << " /* "
        << Def->getBuiltinTypeStr() << " */, ";
     OS << Table.GetStringOffset("n") << " /* n */, ";
diff --git a/clang/utils/TableGen/RISCVVEmitter.cpp b/clang/utils/TableGen/RISCVVEmitter.cpp
diff --git a/clang/utils/TableGen/SveEmitter.cpp b/clang/utils/TableGen/SveEmitter.cpp
