[llvm] [polly] [TableGen] Refactor Intrinsic handling in TableGen (PR #103980)

Rahul Joshi via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 14 14:35:48 PDT 2024


https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/103980

>From 09ab31074e144d319005e1617c1cc9e4084c3914 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Tue, 13 Aug 2024 21:38:07 -0700
Subject: [PATCH] [TableGen] Refactor Intrinsic handling in TableGen

CodeGenIntrinsic changes:
  - Use `const` Record pointers, and `StringRef` when possible.
  - Default initialize several fields with their definition instead of in
    the constructor.
  - Simplify various string checks in the constructor using StringRef
    starts_with()/ends_with() functions.
  - Eliminate first argument to `setDefaultProperties` and use `TheDef`
    class member instead.

IntrinsicEmitter changes:
  - Emit `namespace llvm::Intrinsic` instead of nested namespaces.
  - End generated comments with a .
  - Use range based for loops, and early continue within loops.
  - Emit `static constexpr` instead of `static const` for arrays.
  - Change `compareFnAttributes` to use std::tie() to compare intrinsic
    attributes and return a default value when all attributes are equal.

STLExtras:
  - Add std::replace wrapper which takes a range.
---
 llvm/include/llvm/ADT/STLExtras.h             |   7 +
 llvm/lib/Object/COFFImportFile.cpp            |   2 +-
 .../TableGen/Basic/CodeGenIntrinsics.cpp      |  73 ++++-----
 llvm/utils/TableGen/Basic/CodeGenIntrinsics.h |  59 +++----
 .../utils/TableGen/Basic/SDNodeProperties.cpp |   4 +-
 llvm/utils/TableGen/Basic/SDNodeProperties.h  |   2 +-
 .../GlobalISel/GlobalISelMatchTable.cpp       |   4 +-
 .../Common/GlobalISel/PatternParser.cpp       |   2 +-
 llvm/utils/TableGen/IntrinsicEmitter.cpp      | 154 +++++++-----------
 .../utils/TableGen/SearchableTableEmitter.cpp |   5 +-
 polly/lib/Support/GICHelper.cpp               |  13 +-
 11 files changed, 144 insertions(+), 181 deletions(-)

diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index 3f5ea3a5275f35..a810a1c318c42c 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -1843,6 +1843,13 @@ OutputIt replace_copy(R &&Range, OutputIt Out, const T &OldValue,
                            NewValue);
 }
 
+/// Provide wrappers to std::replace which take ranges instead of having to pass
+/// begin/end explicitly.
+template <typename R, typename T>
+void replace(R &&Range, const T &OldValue, const T &NewValue) {
+  return std::replace(adl_begin(Range), adl_end(Range), OldValue, NewValue);
+}
+
 /// Provide wrappers to std::move which take ranges instead of having to
 /// pass begin/end explicitly.
 template <typename R, typename OutputIt>
diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp
index e67b02405a3a17..cc0a5da7e0d16a 100644
--- a/llvm/lib/Object/COFFImportFile.cpp
+++ b/llvm/lib/Object/COFFImportFile.cpp
@@ -705,7 +705,7 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
         Name = std::string(SymbolName);
       } else {
         Expected<std::string> ReplacedName =
-            replace(SymbolName, E.Name, E.ExtName);
+            object::replace(SymbolName, E.Name, E.ExtName);
         if (!ReplacedName)
           return ReplacedName.takeError();
         Name.swap(*ReplacedName);
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
index 9153645e3d9a11..8b8c902c37daf9 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
@@ -29,16 +29,16 @@ CodeGenIntrinsicTable::CodeGenIntrinsicTable(const RecordKeeper &RC) {
   std::vector<Record *> IntrProperties =
       RC.getAllDerivedDefinitions("IntrinsicProperty");
 
-  std::vector<Record *> DefaultProperties;
-  for (Record *Rec : IntrProperties)
+  std::vector<const Record *> DefaultProperties;
+  for (const Record *Rec : IntrProperties)
     if (Rec->getValueAsBit("IsDefault"))
       DefaultProperties.push_back(Rec);
 
   std::vector<Record *> Defs = RC.getAllDerivedDefinitions("Intrinsic");
   Intrinsics.reserve(Defs.size());
 
-  for (unsigned I = 0, e = Defs.size(); I != e; ++I)
-    Intrinsics.push_back(CodeGenIntrinsic(Defs[I], DefaultProperties));
+  for (const Record *Def : Defs)
+    Intrinsics.push_back(CodeGenIntrinsic(Def, DefaultProperties));
 
   llvm::sort(Intrinsics,
              [](const CodeGenIntrinsic &LHS, const CodeGenIntrinsic &RHS) {
@@ -54,52 +54,34 @@ CodeGenIntrinsicTable::CodeGenIntrinsicTable(const RecordKeeper &RC) {
   Targets.back().Count = Intrinsics.size() - Targets.back().Offset;
 }
 
-CodeGenIntrinsic::CodeGenIntrinsic(Record *R,
-                                   ArrayRef<Record *> DefaultProperties) {
-  TheDef = R;
-  std::string DefName = std::string(R->getName());
+CodeGenIntrinsic::CodeGenIntrinsic(const Record *R,
+                                   ArrayRef<const Record *> DefaultProperties)
+    : TheDef(R) {
+  StringRef DefName = TheDef->getName();
   ArrayRef<SMLoc> DefLoc = R->getLoc();
-  Properties = 0;
-  isOverloaded = false;
-  isCommutative = false;
-  canThrow = false;
-  isNoReturn = false;
-  isNoCallback = false;
-  isNoSync = false;
-  isNoFree = false;
-  isWillReturn = false;
-  isCold = false;
-  isNoDuplicate = false;
-  isNoMerge = false;
-  isConvergent = false;
-  isSpeculatable = false;
-  hasSideEffects = false;
-  isStrictFP = false;
 
-  if (DefName.size() <= 4 || DefName.substr(0, 4) != "int_")
+  if (!DefName.starts_with("int_"))
     PrintFatalError(DefLoc,
                     "Intrinsic '" + DefName + "' does not start with 'int_'!");
 
   EnumName = DefName.substr(4);
 
-  if (R->getValue(
-          "ClangBuiltinName")) // Ignore a missing ClangBuiltinName field.
-    ClangBuiltinName = std::string(R->getValueAsString("ClangBuiltinName"));
-  if (R->getValue("MSBuiltinName")) // Ignore a missing MSBuiltinName field.
-    MSBuiltinName = std::string(R->getValueAsString("MSBuiltinName"));
+  // Ignore a missing ClangBuiltinName field.
+  ClangBuiltinName =
+      R->getValueAsOptionalString("ClangBuiltinName").value_or("");
+  // Ignore a missing MSBuiltinName field.
+  MSBuiltinName = R->getValueAsOptionalString("MSBuiltinName").value_or("");
 
-  TargetPrefix = std::string(R->getValueAsString("TargetPrefix"));
-  Name = std::string(R->getValueAsString("LLVMName"));
+  TargetPrefix = R->getValueAsString("TargetPrefix");
+  Name = R->getValueAsString("LLVMName").str();
 
   if (Name == "") {
     // If an explicit name isn't specified, derive one from the DefName.
-    Name = "llvm.";
-
-    for (unsigned i = 0, e = EnumName.size(); i != e; ++i)
-      Name += (EnumName[i] == '_') ? '.' : EnumName[i];
+    Name = "llvm." + EnumName.str();
+    llvm::replace(Name, '_', '.');
   } else {
     // Verify it starts with "llvm.".
-    if (Name.size() <= 5 || Name.substr(0, 5) != "llvm.")
+    if (!StringRef(Name).starts_with("llvm."))
       PrintFatalError(DefLoc, "Intrinsic '" + DefName +
                                   "'s name does not start with 'llvm.'!");
   }
@@ -107,8 +89,9 @@ CodeGenIntrinsic::CodeGenIntrinsic(Record *R,
   // If TargetPrefix is specified, make sure that Name starts with
   // "llvm.<targetprefix>.".
   if (!TargetPrefix.empty()) {
-    if (Name.size() < 6 + TargetPrefix.size() ||
-        Name.substr(5, 1 + TargetPrefix.size()) != (TargetPrefix + "."))
+    StringRef Prefix = StringRef(Name).drop_front(5); // Drop llvm.
+    Prefix = Prefix.substr(0, TargetPrefix.size() + 1);
+    if (!Prefix.starts_with(TargetPrefix) || !Prefix.ends_with('.'))
       PrintFatalError(DefLoc, "Intrinsic '" + DefName +
                                   "' does not start with 'llvm." +
                                   TargetPrefix + ".'!");
@@ -129,7 +112,7 @@ CodeGenIntrinsic::CodeGenIntrinsic(Record *R,
   // Parse the intrinsic properties.
   ListInit *PropList = R->getValueAsListInit("IntrProperties");
   for (unsigned i = 0, e = PropList->size(); i != e; ++i) {
-    Record *Property = PropList->getElementAsRecord(i);
+    const Record *Property = PropList->getElementAsRecord(i);
     assert(Property->isSubClassOf("IntrinsicProperty") &&
            "Expected a property!");
 
@@ -137,7 +120,7 @@ CodeGenIntrinsic::CodeGenIntrinsic(Record *R,
   }
 
   // Set default properties to true.
-  setDefaultProperties(R, DefaultProperties);
+  setDefaultProperties(DefaultProperties);
 
   // Also record the SDPatternOperator Properties.
   Properties = parseSDPatternOperatorProperties(R);
@@ -148,16 +131,16 @@ CodeGenIntrinsic::CodeGenIntrinsic(Record *R,
 }
 
 void CodeGenIntrinsic::setDefaultProperties(
-    Record *R, ArrayRef<Record *> DefaultProperties) {
+    ArrayRef<const Record *> DefaultProperties) {
   // opt-out of using default attributes.
-  if (R->getValueAsBit("DisableDefaultAttributes"))
+  if (TheDef->getValueAsBit("DisableDefaultAttributes"))
     return;
 
-  for (Record *Rec : DefaultProperties)
+  for (const Record *Rec : DefaultProperties)
     setProperty(Rec);
 }
 
-void CodeGenIntrinsic::setProperty(Record *R) {
+void CodeGenIntrinsic::setProperty(const Record *R) {
   if (R->getName() == "IntrNoMem")
     ME = MemoryEffects::none();
   else if (R->getName() == "IntrReadMem") {
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
index 417063a9490ef1..835818a76b68af 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.h
@@ -26,12 +26,12 @@ class Record;
 class RecordKeeper;
 
 struct CodeGenIntrinsic {
-  Record *TheDef;       // The actual record defining this intrinsic.
+  const Record *TheDef; // The actual record defining this intrinsic.
   std::string Name;     // The name of the LLVM function "llvm.bswap.i32"
-  std::string EnumName; // The name of the enum "bswap_i32"
-  std::string ClangBuiltinName; // Name of the corresponding GCC builtin, or "".
-  std::string MSBuiltinName;    // Name of the corresponding MS builtin, or "".
-  std::string TargetPrefix;     // Target prefix, e.g. "ppc" for t-s intrinsics.
+  StringRef EnumName;   // The name of the enum "bswap_i32"
+  StringRef ClangBuiltinName; // Name of the corresponding GCC builtin, or "".
+  StringRef MSBuiltinName;    // Name of the corresponding MS builtin, or "".
+  StringRef TargetPrefix;     // Target prefix, e.g. "ppc" for t-s intrinsics.
 
   /// This structure holds the return values and parameter values of an
   /// intrinsic. If the number of return values is > 1, then the intrinsic
@@ -43,13 +43,13 @@ struct CodeGenIntrinsic {
     /// only populated when in the context of a target .td file. When building
     /// Intrinsics.td, this isn't available, because we don't know the target
     /// pointer size.
-    std::vector<Record *> RetTys;
+    std::vector<const Record *> RetTys;
 
     /// The MVT::SimpleValueType for each parameter type. Note that this list is
     /// only populated when in the context of a target .td file.  When building
     /// Intrinsics.td, this isn't available, because we don't know the target
     /// pointer size.
-    std::vector<Record *> ParamTys;
+    std::vector<const Record *> ParamTys;
   };
 
   IntrinsicSignature IS;
@@ -58,54 +58,54 @@ struct CodeGenIntrinsic {
   MemoryEffects ME = MemoryEffects::unknown();
 
   /// SDPatternOperator Properties applied to the intrinsic.
-  unsigned Properties;
+  unsigned Properties = 0;
 
   /// This is set to true if the intrinsic is overloaded by its argument
   /// types.
-  bool isOverloaded;
+  bool isOverloaded = false;
 
   /// True if the intrinsic is commutative.
-  bool isCommutative;
+  bool isCommutative = false;
 
   /// True if the intrinsic can throw.
-  bool canThrow;
+  bool canThrow = false;
 
   /// True if the intrinsic is marked as noduplicate.
-  bool isNoDuplicate;
+  bool isNoDuplicate = false;
 
   /// True if the intrinsic is marked as nomerge.
-  bool isNoMerge;
+  bool isNoMerge = false;
 
   /// True if the intrinsic is no-return.
-  bool isNoReturn;
+  bool isNoReturn = false;
 
   /// True if the intrinsic is no-callback.
-  bool isNoCallback;
+  bool isNoCallback = false;
 
   /// True if the intrinsic is no-sync.
-  bool isNoSync;
+  bool isNoSync = false;
 
   /// True if the intrinsic is no-free.
-  bool isNoFree;
+  bool isNoFree = false;
 
   /// True if the intrinsic is will-return.
-  bool isWillReturn;
+  bool isWillReturn = false;
 
   /// True if the intrinsic is cold.
-  bool isCold;
+  bool isCold = false;
 
   /// True if the intrinsic is marked as convergent.
-  bool isConvergent;
+  bool isConvergent = false;
 
   /// True if the intrinsic has side effects that aren't captured by any
   /// of the other flags.
-  bool hasSideEffects;
+  bool hasSideEffects = false;
 
   // True if the intrinsic is marked as speculatable.
-  bool isSpeculatable;
+  bool isSpeculatable = false;
 
   // True if the intrinsic is marked as strictfp.
-  bool isStrictFP;
+  bool isStrictFP = false;
 
   enum ArgAttrKind {
     NoCapture,
@@ -139,12 +139,12 @@ struct CodeGenIntrinsic {
 
   bool hasProperty(enum SDNP Prop) const { return Properties & (1 << Prop); }
 
-  /// Goes through all IntrProperties that have IsDefault
-  /// value set and sets the property.
-  void setDefaultProperties(Record *R, ArrayRef<Record *> DefaultProperties);
+  /// Goes through all IntrProperties that have IsDefault value set and sets
+  /// the property.
+  void setDefaultProperties(ArrayRef<const Record *> DefaultProperties);
 
   /// Helper function to set property \p Name to true;
-  void setProperty(Record *R);
+  void setProperty(const Record *R);
 
   /// Returns true if the parameter at \p ParamIdx is a pointer type. Returns
   /// false if the parameter is not a pointer, or \p ParamIdx is greater than
@@ -155,7 +155,8 @@ struct CodeGenIntrinsic {
 
   bool isParamImmArg(unsigned ParamIdx) const;
 
-  CodeGenIntrinsic(Record *R, ArrayRef<Record *> DefaultProperties);
+  CodeGenIntrinsic(const Record *R,
+                   ArrayRef<const Record *> DefaultProperties = {});
 };
 
 class CodeGenIntrinsicTable {
@@ -163,7 +164,7 @@ class CodeGenIntrinsicTable {
 
 public:
   struct TargetSet {
-    std::string Name;
+    StringRef Name;
     size_t Offset;
     size_t Count;
   };
diff --git a/llvm/utils/TableGen/Basic/SDNodeProperties.cpp b/llvm/utils/TableGen/Basic/SDNodeProperties.cpp
index 2aec41aac6257f..46babbbc419446 100644
--- a/llvm/utils/TableGen/Basic/SDNodeProperties.cpp
+++ b/llvm/utils/TableGen/Basic/SDNodeProperties.cpp
@@ -13,9 +13,9 @@
 
 using namespace llvm;
 
-unsigned llvm::parseSDPatternOperatorProperties(Record *R) {
+unsigned llvm::parseSDPatternOperatorProperties(const Record *R) {
   unsigned Properties = 0;
-  for (Record *Property : R->getValueAsListOfDefs("Properties")) {
+  for (const Record *Property : R->getValueAsListOfDefs("Properties")) {
     auto Offset = StringSwitch<unsigned>(Property->getName())
                       .Case("SDNPCommutative", SDNPCommutative)
                       .Case("SDNPAssociative", SDNPAssociative)
diff --git a/llvm/utils/TableGen/Basic/SDNodeProperties.h b/llvm/utils/TableGen/Basic/SDNodeProperties.h
index 10d0bb965fbb6c..1fe4044edcea23 100644
--- a/llvm/utils/TableGen/Basic/SDNodeProperties.h
+++ b/llvm/utils/TableGen/Basic/SDNodeProperties.h
@@ -32,7 +32,7 @@ enum SDNP {
   SDNPWantParent
 };
 
-unsigned parseSDPatternOperatorProperties(Record *R);
+unsigned parseSDPatternOperatorProperties(const Record *R);
 
 } // namespace llvm
 
diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
index 9b9686c627b752..139bf2d8218032 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
+++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
@@ -1314,7 +1314,7 @@ void IntrinsicIDOperandMatcher::emitPredicateOpcodes(MatchTable &Table,
   Table << MatchTable::Opcode("GIM_CheckIntrinsicID")
         << MatchTable::Comment("MI") << MatchTable::ULEB128Value(InsnVarID)
         << MatchTable::Comment("Op") << MatchTable::ULEB128Value(OpIdx)
-        << MatchTable::NamedValue(2, "Intrinsic::" + II->EnumName)
+        << MatchTable::NamedValue(2, "Intrinsic::" + II->EnumName.str())
         << MatchTable::LineBreak;
 }
 
@@ -2103,7 +2103,7 @@ void IntrinsicIDRenderer::emitRenderOpcodes(MatchTable &Table,
                                             RuleMatcher &Rule) const {
   Table << MatchTable::Opcode("GIR_AddIntrinsicID") << MatchTable::Comment("MI")
         << MatchTable::ULEB128Value(InsnID)
-        << MatchTable::NamedValue(2, "Intrinsic::" + II->EnumName)
+        << MatchTable::NamedValue(2, "Intrinsic::" + II->EnumName.str())
         << MatchTable::LineBreak;
 }
 
diff --git a/llvm/utils/TableGen/Common/GlobalISel/PatternParser.cpp b/llvm/utils/TableGen/Common/GlobalISel/PatternParser.cpp
index 1d6c4c73a26405..82fa3c8f3121f0 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/PatternParser.cpp
+++ b/llvm/utils/TableGen/Common/GlobalISel/PatternParser.cpp
@@ -112,7 +112,7 @@ static const CodeGenIntrinsic *getCodeGenIntrinsic(Record *R) {
 
   auto &Ptr = AllIntrinsics[R];
   if (!Ptr)
-    Ptr = std::make_unique<CodeGenIntrinsic>(R, std::vector<Record *>());
+    Ptr = std::make_unique<CodeGenIntrinsic>(R);
   return Ptr.get();
 }
 
diff --git a/llvm/utils/TableGen/IntrinsicEmitter.cpp b/llvm/utils/TableGen/IntrinsicEmitter.cpp
index a7e99fa4c05068..b1307153e9109b 100644
--- a/llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -35,18 +35,18 @@
 #include <vector>
 using namespace llvm;
 
-cl::OptionCategory GenIntrinsicCat("Options for -gen-intrinsic-enums");
-cl::opt<std::string>
+static cl::OptionCategory GenIntrinsicCat("Options for -gen-intrinsic-enums");
+static cl::opt<std::string>
     IntrinsicPrefix("intrinsic-prefix",
                     cl::desc("Generate intrinsics with this target prefix"),
                     cl::value_desc("target prefix"), cl::cat(GenIntrinsicCat));
 
 namespace {
 class IntrinsicEmitter {
-  RecordKeeper &Records;
+  const RecordKeeper &Records;
 
 public:
-  IntrinsicEmitter(RecordKeeper &R) : Records(R) {}
+  IntrinsicEmitter(const RecordKeeper &R) : Records(R) {}
 
   void run(raw_ostream &OS, bool Enums);
 
@@ -123,7 +123,7 @@ void IntrinsicEmitter::EmitEnumInfo(const CodeGenIntrinsicTable &Ints,
     std::vector<std::string> KnownTargets;
     for (const auto &Target : Ints.Targets)
       if (!Target.Name.empty())
-        KnownTargets.push_back(Target.Name);
+        KnownTargets.push_back(Target.Name.str());
     PrintFatalError("tried to generate intrinsics for unknown target " +
                     IntrinsicPrefix +
                     "\nKnown targets are: " + join(KnownTargets, ", ") + "\n");
@@ -136,12 +136,11 @@ void IntrinsicEmitter::EmitEnumInfo(const CodeGenIntrinsicTable &Ints,
     std::string UpperPrefix = StringRef(IntrinsicPrefix).upper();
     OS << "#ifndef LLVM_IR_INTRINSIC_" << UpperPrefix << "_ENUMS_H\n";
     OS << "#define LLVM_IR_INTRINSIC_" << UpperPrefix << "_ENUMS_H\n\n";
-    OS << "namespace llvm {\n";
-    OS << "namespace Intrinsic {\n";
+    OS << "namespace llvm::Intrinsic {\n";
     OS << "enum " << UpperPrefix << "Intrinsics : unsigned {\n";
   }
 
-  OS << "// Enum values for intrinsics\n";
+  OS << "// Enum values for intrinsics.\n";
   for (unsigned i = Set->Offset, e = Set->Offset + Set->Count; i != e; ++i) {
     OS << "    " << Ints[i].EnumName;
 
@@ -162,8 +161,7 @@ void IntrinsicEmitter::EmitEnumInfo(const CodeGenIntrinsicTable &Ints,
     OS << "#endif\n\n";
   } else {
     OS << "}; // enum\n";
-    OS << "} // namespace Intrinsic\n";
-    OS << "} // namespace llvm\n\n";
+    OS << "} // namespace llvm::Intrinsic\n\n";
     OS << "#endif\n";
   }
 }
@@ -171,7 +169,7 @@ void IntrinsicEmitter::EmitEnumInfo(const CodeGenIntrinsicTable &Ints,
 void IntrinsicEmitter::EmitArgKind(raw_ostream &OS) {
   if (!IntrinsicPrefix.empty())
     return;
-  OS << "// llvm::Intrinsic::IITDescriptor::ArgKind\n";
+  OS << "// llvm::Intrinsic::IITDescriptor::ArgKind.\n";
   OS << "#ifdef GET_INTRINSIC_ARGKIND\n";
   if (auto RecArgKind = Records.getDef("ArgKind")) {
     for (auto &RV : RecArgKind->getValues())
@@ -205,7 +203,7 @@ void IntrinsicEmitter::EmitIITInfo(raw_ostream &OS) {
 
 void IntrinsicEmitter::EmitTargetInfo(const CodeGenIntrinsicTable &Ints,
                                       raw_ostream &OS) {
-  OS << "// Target mapping\n";
+  OS << "// Target mapping.\n";
   OS << "#ifdef GET_INTRINSIC_TARGET_DATA\n";
   OS << "struct IntrinsicTargetInfo {\n"
      << "  llvm::StringLiteral Name;\n"
@@ -222,19 +220,19 @@ void IntrinsicEmitter::EmitTargetInfo(const CodeGenIntrinsicTable &Ints,
 
 void IntrinsicEmitter::EmitIntrinsicToNameTable(
     const CodeGenIntrinsicTable &Ints, raw_ostream &OS) {
-  OS << "// Intrinsic ID to name table\n";
+  OS << "// Intrinsic ID to name table.\n";
   OS << "#ifdef GET_INTRINSIC_NAME_TABLE\n";
   OS << "  // Note that entry #0 is the invalid intrinsic!\n";
-  for (unsigned i = 0, e = Ints.size(); i != e; ++i)
-    OS << "  \"" << Ints[i].Name << "\",\n";
+  for (const auto &Int : Ints)
+    OS << "  \"" << Int.Name << "\",\n";
   OS << "#endif\n\n";
 }
 
 void IntrinsicEmitter::EmitIntrinsicToOverloadTable(
     const CodeGenIntrinsicTable &Ints, raw_ostream &OS) {
-  OS << "// Intrinsic ID to overload bitset\n";
+  OS << "// Intrinsic ID to overload bitset.\n";
   OS << "#ifdef GET_INTRINSIC_OVERLOAD_TABLE\n";
-  OS << "static const uint8_t OTable[] = {\n";
+  OS << "static constexpr uint8_t OTable[] = {\n";
   OS << "  0";
   for (unsigned i = 0, e = Ints.size(); i != e; ++i) {
     // Add one to the index so we emit a null bit for the invalid #0 intrinsic.
@@ -315,7 +313,7 @@ void IntrinsicEmitter::EmitGenerator(const CodeGenIntrinsicTable &Ints,
   OS << "// Global intrinsic function declaration type table.\n";
   OS << "#ifdef GET_INTRINSIC_GENERATOR_GLOBAL\n";
 
-  OS << "static const unsigned IIT_Table[] = {\n  ";
+  OS << "static constexpr unsigned IIT_Table[] = {\n  ";
 
   for (unsigned i = 0, e = FixedEncodings.size(); i != e; ++i) {
     if ((i & 7) == 7)
@@ -338,7 +336,7 @@ void IntrinsicEmitter::EmitGenerator(const CodeGenIntrinsicTable &Ints,
   OS << "0\n};\n\n";
 
   // Emit the shared table of register lists.
-  OS << "static const unsigned char IIT_LongEncodingTable[] = {\n";
+  OS << "static constexpr unsigned char IIT_LongEncodingTable[] = {\n";
   if (!LongEncodingTable.empty())
     LongEncodingTable.emit(OS, printIITEntry);
   OS << "  255\n};\n\n";
@@ -346,72 +344,44 @@ void IntrinsicEmitter::EmitGenerator(const CodeGenIntrinsicTable &Ints,
   OS << "#endif\n\n"; // End of GET_INTRINSIC_GENERATOR_GLOBAL
 }
 
-namespace {
-std::optional<bool> compareFnAttributes(const CodeGenIntrinsic *L,
-                                        const CodeGenIntrinsic *R) {
-  // Sort throwing intrinsics after non-throwing intrinsics.
-  if (L->canThrow != R->canThrow)
-    return R->canThrow;
-
-  if (L->isNoDuplicate != R->isNoDuplicate)
-    return R->isNoDuplicate;
-
-  if (L->isNoMerge != R->isNoMerge)
-    return R->isNoMerge;
-
-  if (L->isNoReturn != R->isNoReturn)
-    return R->isNoReturn;
-
-  if (L->isNoCallback != R->isNoCallback)
-    return R->isNoCallback;
-
-  if (L->isNoSync != R->isNoSync)
-    return R->isNoSync;
-
-  if (L->isNoFree != R->isNoFree)
-    return R->isNoFree;
-
-  if (L->isWillReturn != R->isWillReturn)
-    return R->isWillReturn;
-
-  if (L->isCold != R->isCold)
-    return R->isCold;
+static bool compareFnAttributes(const CodeGenIntrinsic *L,
+                                const CodeGenIntrinsic *R, bool Default) {
+  auto TieBoolAttributes = [](const CodeGenIntrinsic *I) -> auto {
+    // Sort throwing intrinsics after non-throwing intrinsics.
+    return std::tie(I->canThrow, I->isNoDuplicate, I->isNoMerge, I->isNoReturn,
+                    I->isNoCallback, I->isNoSync, I->isNoFree, I->isWillReturn,
+                    I->isCold, I->isConvergent, I->isSpeculatable,
+                    I->hasSideEffects, I->isStrictFP);
+  };
 
-  if (L->isConvergent != R->isConvergent)
-    return R->isConvergent;
+  auto TieL = TieBoolAttributes(L);
+  auto TieR = TieBoolAttributes(R);
 
-  if (L->isSpeculatable != R->isSpeculatable)
-    return R->isSpeculatable;
-
-  if (L->hasSideEffects != R->hasSideEffects)
-    return R->hasSideEffects;
-
-  if (L->isStrictFP != R->isStrictFP)
-    return R->isStrictFP;
+  if (TieL != TieR)
+    return TieL < TieR;
 
   // Try to order by readonly/readnone attribute.
   uint32_t LK = L->ME.toIntValue();
   uint32_t RK = R->ME.toIntValue();
   if (LK != RK)
-    return (LK > RK);
+    return LK > RK;
 
-  return std::nullopt;
+  return Default;
 }
 
+namespace {
 struct FnAttributeComparator {
   bool operator()(const CodeGenIntrinsic *L, const CodeGenIntrinsic *R) const {
-    return compareFnAttributes(L, R).value_or(false);
+    return compareFnAttributes(L, R, false);
   }
 };
 
 struct AttributeComparator {
   bool operator()(const CodeGenIntrinsic *L, const CodeGenIntrinsic *R) const {
-    if (std::optional<bool> Res = compareFnAttributes(L, R))
-      return *Res;
-
-    // Order by argument attributes.
+    // Order by argument attributes if function attributes are equal.
     // This is reliable because each side is already sorted internally.
-    return (L->ArgumentAttributes < R->ArgumentAttributes);
+    return compareFnAttributes(L, R,
+                               L->ArgumentAttributes < R->ArgumentAttributes);
   }
 };
 } // End anonymous namespace
@@ -559,7 +529,7 @@ void IntrinsicEmitter::EmitAttributes(const CodeGenIntrinsicTable &Ints,
 
   // Emit an array of AttributeList.  Most intrinsics will have at least one
   // entry, for the function itself (index ~1), which is usually nounwind.
-  OS << "  static const uint16_t IntrinsicsToAttributesMap[] = {\n";
+  OS << "  static constexpr uint16_t IntrinsicsToAttributesMap[] = {\n";
 
   for (unsigned i = 0, e = Ints.size(); i != e; ++i) {
     const CodeGenIntrinsic &intrinsic = Ints[i];
@@ -624,31 +594,34 @@ void IntrinsicEmitter::EmitAttributes(const CodeGenIntrinsicTable &Ints,
 
 void IntrinsicEmitter::EmitIntrinsicToBuiltinMap(
     const CodeGenIntrinsicTable &Ints, bool IsClang, raw_ostream &OS) {
-  StringRef CompilerName = (IsClang ? "Clang" : "MS");
-  StringRef UpperCompilerName = (IsClang ? "CLANG" : "MS");
-  typedef std::map<std::string, std::map<std::string, std::string>> BIMTy;
+  StringRef CompilerName = IsClang ? "Clang" : "MS";
+  StringRef UpperCompilerName = IsClang ? "CLANG" : "MS";
+  // map<TargetPrefix, map<BuiltinName, EnumName>>. Note that we iterate over
+  // both maps in the code below. For the inner map, entries need to be emitted
+  // in the sorted order of `BuiltinName` because we use std::lower_bound to
+  // search these entries. For the outer map, it doesn't need be be sorted, but
+  // we use a map to eliminate non-determinism in the emitted code.
+  typedef std::map<StringRef, std::map<StringRef, StringRef>> BIMTy;
   BIMTy BuiltinMap;
   StringToOffsetTable Table;
-  for (unsigned i = 0, e = Ints.size(); i != e; ++i) {
-    const std::string &BuiltinName =
-        IsClang ? Ints[i].ClangBuiltinName : Ints[i].MSBuiltinName;
-    if (!BuiltinName.empty()) {
-      // Get the map for this target prefix.
-      std::map<std::string, std::string> &BIM =
-          BuiltinMap[Ints[i].TargetPrefix];
-
-      if (!BIM.insert(std::pair(BuiltinName, Ints[i].EnumName)).second)
-        PrintFatalError(Ints[i].TheDef->getLoc(),
-                        "Intrinsic '" + Ints[i].TheDef->getName() +
-                            "': duplicate " + CompilerName + " builtin name!");
-      Table.GetOrAddStringOffset(BuiltinName);
-    }
+  for (const CodeGenIntrinsic &Int : Ints) {
+    StringRef BuiltinName = IsClang ? Int.ClangBuiltinName : Int.MSBuiltinName;
+    if (BuiltinName.empty())
+      continue;
+    // Get the map for this target prefix.
+    std::map<StringRef, StringRef> &BIM = BuiltinMap[Int.TargetPrefix];
+
+    if (!BIM.insert(std::pair(BuiltinName, Int.EnumName)).second)
+      PrintFatalError(Int.TheDef->getLoc(),
+                      "Intrinsic '" + Int.TheDef->getName() + "': duplicate " +
+                          CompilerName + " builtin name!");
+    Table.GetOrAddStringOffset(BuiltinName);
   }
 
   OS << "// Get the LLVM intrinsic that corresponds to a builtin.\n";
   OS << "// This is used by the C front-end.  The builtin name is passed\n";
   OS << "// in as BuiltinName, and a target prefix (e.g. 'ppc') is passed\n";
-  OS << "// in as TargetPrefix.  The result is assigned to 'IntrinsicID'.\n";
+  OS << "// in as TargetPrefix. The result is assigned to 'IntrinsicID'.\n";
   OS << "#ifdef GET_LLVM_INTRINSIC_FOR_" << UpperCompilerName << "_BUILTIN\n";
 
   OS << "Intrinsic::ID Intrinsic::getIntrinsicFor" << CompilerName
@@ -662,7 +635,7 @@ void IntrinsicEmitter::EmitIntrinsicToBuiltinMap(
     return;
   }
 
-  OS << "  static const char BuiltinNames[] = {\n";
+  OS << "  static constexpr char BuiltinNames[] = {\n";
   Table.EmitCharArray(OS);
   OS << "  };\n\n";
 
@@ -689,7 +662,7 @@ void IntrinsicEmitter::EmitIntrinsicToBuiltinMap(
     OS << "{\n";
 
     // Emit the comparisons for this target prefix.
-    OS << "    static const BuiltinEntry " << I.first << "Names[] = {\n";
+    OS << "    static constexpr BuiltinEntry " << I.first << "Names[] = {\n";
     for (const auto &P : I.second) {
       OS << "      {Intrinsic::" << P.second << ", "
          << Table.GetOrAddStringOffset(P.first) << "}, // " << P.first << "\n";
@@ -703,8 +676,7 @@ void IntrinsicEmitter::EmitIntrinsicToBuiltinMap(
     OS << "      return I->IntrinID;\n";
     OS << "  }\n";
   }
-  OS << "  return ";
-  OS << "Intrinsic::not_intrinsic;\n";
+  OS << "  return Intrinsic::not_intrinsic;\n";
   OS << "}\n";
   OS << "#endif\n\n";
 }
@@ -721,4 +693,4 @@ static void EmitIntrinsicImpl(RecordKeeper &RK, raw_ostream &OS) {
 }
 
 static TableGen::Emitter::Opt Y("gen-intrinsic-impl", EmitIntrinsicImpl,
-                                "Generate intrinsic information");
+                                "Generate intrinsic implementation code");
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index ac50c9ed2aeba9..59ae3bf3daedcb 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -125,7 +125,7 @@ class SearchableTableEmitter {
     else if (BitInit *BI = dyn_cast<BitInit>(I))
       return BI->getValue() ? "true" : "false";
     else if (Field.IsIntrinsic)
-      return "Intrinsic::" + getIntrinsic(I).EnumName;
+      return "Intrinsic::" + getIntrinsic(I).EnumName.str();
     else if (Field.IsInstruction)
       return I->getAsString();
     else if (Field.Enum) {
@@ -148,8 +148,7 @@ class SearchableTableEmitter {
   CodeGenIntrinsic &getIntrinsic(Init *I) {
     std::unique_ptr<CodeGenIntrinsic> &Intr = Intrinsics[I];
     if (!Intr)
-      Intr = std::make_unique<CodeGenIntrinsic>(cast<DefInit>(I)->getDef(),
-                                                std::vector<Record *>());
+      Intr = std::make_unique<CodeGenIntrinsic>(cast<DefInit>(I)->getDef());
     return *Intr;
   }
 
diff --git a/polly/lib/Support/GICHelper.cpp b/polly/lib/Support/GICHelper.cpp
index 04d422cf994623..e4726ec5ebd94c 100644
--- a/polly/lib/Support/GICHelper.cpp
+++ b/polly/lib/Support/GICHelper.cpp
@@ -134,7 +134,8 @@ ISL_C_OBJECT_TO_STRING(union_map)
 ISL_C_OBJECT_TO_STRING(union_pw_aff)
 ISL_C_OBJECT_TO_STRING(union_pw_multi_aff)
 
-static void replace(std::string &str, StringRef find, StringRef replace) {
+static void replace_substr(std::string &str, StringRef find,
+                           StringRef replace) {
   size_t pos = 0;
   while ((pos = str.find(find, pos)) != std::string::npos) {
     str.replace(pos, find.size(), replace);
@@ -143,11 +144,11 @@ static void replace(std::string &str, StringRef find, StringRef replace) {
 }
 
 static void makeIslCompatible(std::string &str) {
-  replace(str, ".", "_");
-  replace(str, "\"", "_");
-  replace(str, " ", "__");
-  replace(str, "=>", "TO");
-  replace(str, "+", "_");
+  replace(str, '.', '_');
+  replace(str, '\"', '_');
+  replace_substr(str, " ", "__");
+  replace_substr(str, "=>", "TO");
+  replace(str, '+', '_');
 }
 
 std::string polly::getIslCompatibleName(const std::string &Prefix,



More information about the llvm-commits mailing list