[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.

Author: jurahul

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>

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);

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,61 +54,43 @@ 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.'!");
   }
 
   // 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.
+    if (!Prefix.consume_front(TargetPrefix) || !Prefix.starts_with('.'))
       PrintFatalError(DefLoc, "Intrinsic '" + DefName +
                                   "' does not start with 'llvm." +
                                   TargetPrefix + ".'!");
@@ -129,15 +111,15 @@ 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!");
 
     setProperty(Property);
   }
 
   // Set default properties to true.
-  setDefaultProperties(R, DefaultProperties);
+  setDefaultProperties(DefaultProperties);
 
   // Also record the SDPatternOperator Properties.
   Properties = parseSDPatternOperatorProperties(R);
@@ -148,16 +130,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") {

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);
+  /// Helper function to set property \p Name to true.
+  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,15 +155,16 @@ struct CodeGenIntrinsic {
 
   bool isParamImmArg(unsigned ParamIdx) const;
 
-  CodeGenIntrinsic(Record *R, ArrayRef<Record *> DefaultProperties);
+  CodeGenIntrinsic(const Record *R,
+                   ArrayRef<const Record *> DefaultProperties = {});
 };
 
 class CodeGenIntrinsicTable {
   std::vector<CodeGenIntrinsic> Intrinsics;
 
 public:
   struct TargetSet {
-    std::string Name;
+    StringRef Name;
     size_t Offset;
     size_t Count;
   };

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)

llvm/utils/TableGen/Basic/SDNodeProperties.h

@@ -32,7 +32,7 @@ enum SDNP {
   SDNPWantParent
 };
 
-unsigned parseSDPatternOperatorProperties(Record *R);
+unsigned parseSDPatternOperatorProperties(const Record *R);
 
 } // namespace llvm
 

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;
 }
 

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();
 }