[llvm] [NFC][AsmMatcherEmitter] Misc code cleanup (PR #157012)

Rahul Joshi via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 5 06:54:05 PDT 2025


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

>From 44ea799b84327f812d55d8a798619bc723f7b462 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Thu, 4 Sep 2025 21:38:30 -0700
Subject: [PATCH 1/2] [NFC][AsmMatcherEmitter] Eliminate redundant StringRef ->
 StringRef conversion

---
 llvm/utils/TableGen/AsmMatcherEmitter.cpp | 92 ++++++++---------------
 1 file changed, 33 insertions(+), 59 deletions(-)

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index a373d2e2014b1..2ad76af602d8e 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -842,43 +842,35 @@ LLVM_DUMP_METHOD void MatchableInfo::dump() const {
 
 static std::pair<StringRef, StringRef>
 parseTwoOperandConstraint(StringRef S, ArrayRef<SMLoc> Loc) {
+  // Trim whitespace and the leading '$' on the operand names.
+  auto TrimWSDollar = [Loc](StringRef OpName) {
+    OpName = OpName.trim(" \t");
+    if (!OpName.consume_front("$"))
+      PrintFatalError(Loc, "expected '$' prefix on asm operand name");
+    return OpName;
+  };
+
   // Split via the '='.
-  std::pair<StringRef, StringRef> Ops = S.split('=');
-  if (Ops.second == "")
+  auto [Src, Dst] = S.split('=');
+  if (Dst == "")
     PrintFatalError(Loc, "missing '=' in two-operand alias constraint");
-  // Trim whitespace and the leading '$' on the operand names.
-  size_t start = Ops.first.find_first_of('$');
-  if (start == std::string::npos)
-    PrintFatalError(Loc, "expected '$' prefix on asm operand name");
-  Ops.first = Ops.first.substr(start + 1);
-  size_t end = Ops.first.find_last_of(" \t");
-  Ops.first = Ops.first.slice(0, end);
-  // Now the second operand.
-  start = Ops.second.find_first_of('$');
-  if (start == std::string::npos)
-    PrintFatalError(Loc, "expected '$' prefix on asm operand name");
-  Ops.second = Ops.second.substr(start + 1);
-  end = Ops.second.find_last_of(" \t");
-  Ops.first = Ops.first.slice(0, end);
-  return Ops;
+  return {TrimWSDollar(Src), TrimWSDollar(Dst)};
 }
 
 void MatchableInfo::formTwoOperandAlias(StringRef Constraint) {
   // Figure out which operands are aliased and mark them as tied.
-  std::pair<StringRef, StringRef> Ops =
-      parseTwoOperandConstraint(Constraint, TheDef->getLoc());
+  auto [Src, Dst] = parseTwoOperandConstraint(Constraint, TheDef->getLoc());
 
   // Find the AsmOperands that refer to the operands we're aliasing.
-  int SrcAsmOperand = findAsmOperandNamed(Ops.first);
-  int DstAsmOperand = findAsmOperandNamed(Ops.second);
+  int SrcAsmOperand = findAsmOperandNamed(Src);
+  int DstAsmOperand = findAsmOperandNamed(Dst);
   if (SrcAsmOperand == -1)
     PrintFatalError(TheDef->getLoc(),
-                    "unknown source two-operand alias operand '" + Ops.first +
-                        "'.");
+                    "unknown source two-operand alias operand '" + Src + "'.");
   if (DstAsmOperand == -1)
     PrintFatalError(TheDef->getLoc(),
-                    "unknown destination two-operand alias operand '" +
-                        Ops.second + "'.");
+                    "unknown destination two-operand alias operand '" + Dst +
+                        "'.");
 
   // Find the ResOperand that refers to the operand we're aliasing away
   // and update it to refer to the combined operand instead.
@@ -894,15 +886,9 @@ void MatchableInfo::formTwoOperandAlias(StringRef Constraint) {
   // Adjust the ResOperand references to any AsmOperands that followed
   // the one we just deleted.
   for (ResOperand &Op : ResOperands) {
-    switch (Op.Kind) {
-    default:
-      // Nothing to do for operands that don't reference AsmOperands.
-      break;
-    case ResOperand::RenderAsmOperand:
-      if (Op.AsmOperandNum > (unsigned)SrcAsmOperand)
-        --Op.AsmOperandNum;
-      break;
-    }
+    if (Op.Kind == ResOperand::RenderAsmOperand &&
+        Op.AsmOperandNum > (unsigned)SrcAsmOperand)
+      --Op.AsmOperandNum;
   }
 }
 
@@ -925,11 +911,10 @@ static void extractSingletonRegisterForAsmOperand(MatchableInfo::AsmOperand &Op,
     return;
   }
 
-  if (!Tok.starts_with(RegisterPrefix))
+  if (!Tok.consume_front(RegisterPrefix))
     return;
 
-  StringRef RegName = Tok.substr(RegisterPrefix.size());
-  if (const CodeGenRegister *Reg = Info.Target.getRegisterByName(RegName))
+  if (const CodeGenRegister *Reg = Info.Target.getRegisterByName(Tok))
     Op.SingletonReg = Reg->TheDef;
 
   // If there is no register prefix (i.e. "%" in "%eax"), then this may
@@ -1542,10 +1527,9 @@ void AsmMatcherInfo::buildInfo() {
     Variant.AsmVariantNo = AsmVariant->getValueAsInt("Variant");
 
     for (const CodeGenInstruction *CGI : Target.getInstructions()) {
-
       // If the tblgen -match-prefix option is specified (for tblgen hackers),
       // filter the set of instructions we consider.
-      if (!StringRef(CGI->getName()).starts_with(MatchPrefix))
+      if (!CGI->getName().starts_with(MatchPrefix))
         continue;
 
       // Ignore "codegen only" instructions.
@@ -1578,7 +1562,7 @@ void AsmMatcherInfo::buildInfo() {
       // If the tblgen -match-prefix option is specified (for tblgen hackers),
       // filter the set of instruction aliases we consider, based on the target
       // instruction.
-      if (!StringRef(Alias->ResultInst->getName()).starts_with(MatchPrefix))
+      if (!Alias->ResultInst->getName().starts_with(MatchPrefix))
         continue;
 
       StringRef V = Alias->TheDef->getValueAsString("AsmVariantName");
@@ -2663,11 +2647,8 @@ static void emitMatchRegisterAltName(const CodeGenTarget &Target,
   std::string Namespace =
       Regs.front().TheDef->getValueAsString("Namespace").str();
   for (const CodeGenRegister &Reg : Regs) {
-
-    auto AltNames = Reg.TheDef->getValueAsListOfStrings("AltNames");
-
-    for (auto AltName : AltNames) {
-      AltName = StringRef(AltName).trim();
+    for (StringRef AltName : Reg.TheDef->getValueAsListOfStrings("AltNames")) {
+      AltName = AltName.trim();
 
       // don't handle empty alternative names
       if (AltName.empty())
@@ -2718,8 +2699,7 @@ static void emitGetSubtargetFeatureName(AsmMatcherInfo &Info, raw_ostream &OS) {
      << "static const char *getSubtargetFeatureName(uint64_t Val) {\n";
   if (!Info.SubtargetFeatures.empty()) {
     OS << "  switch(Val) {\n";
-    for (const auto &SF : Info.SubtargetFeatures) {
-      const SubtargetFeatureInfo &SFI = SF.second;
+    for (const auto &[_, SFI] : Info.SubtargetFeatures) {
       // FIXME: Totally just a placeholder name to get the algorithm working.
       OS << "  case " << SFI.getEnumBitName() << ": return \""
          << SFI.TheDef->getValueAsString("PredicateName") << "\";\n";
@@ -2737,17 +2717,15 @@ static std::string GetAliasRequiredFeatures(const Record *R,
                                             const AsmMatcherInfo &Info) {
   std::string Result;
 
-  bool First = true;
+  ListSeparator LS(" && ");
   for (const Record *RF : R->getValueAsListOfDefs("Predicates")) {
     const SubtargetFeatureInfo *F = Info.getSubtargetFeature(RF);
     if (!F)
       PrintFatalError(R->getLoc(),
                       "Predicate '" + RF->getName() +
                           "' is not marked as an AssemblerPredicate!");
-    if (!First)
-      Result += " && ";
+    Result += LS;
     Result += "Features.test(" + F->getEnumBitName() + ')';
-    First = false;
   }
 
   return Result;
@@ -3638,9 +3616,8 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
     OS << "  ErrorInfo = ~0ULL;\n";
   }
 
-  if (HasOptionalOperands) {
+  if (HasOptionalOperands)
     OS << "  SmallBitVector OptionalOperandsMask(" << MaxNumOperands << ");\n";
-  }
 
   // Emit code to search the table.
   OS << "  // Find the appropriate table for this asm variant.\n";
@@ -3708,9 +3685,8 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
   // Emit check that the subclasses match.
   if (!ReportMultipleNearMisses)
     OS << "    bool OperandsValid = true;\n";
-  if (HasOptionalOperands) {
+  if (HasOptionalOperands)
     OS << "    OptionalOperandsMask.reset(0, " << MaxNumOperands << ");\n";
-  }
   OS << "    for (unsigned FormalIdx = " << (HasMnemonicFirst ? "0" : "SIndex")
      << ", ActualIdx = " << (HasMnemonicFirst ? "1" : "SIndex")
      << "; FormalIdx != " << MaxNumOperands << "; ++FormalIdx) {\n";
@@ -3771,9 +3747,8 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
     OS << "          break;\n";
     OS << "        }\n";
     OS << "        if (isSubclass(Formal, OptionalMatchClass)) {\n";
-    if (HasOptionalOperands) {
+    if (HasOptionalOperands)
       OS << "          OptionalOperandsMask.set(FormalIdx);\n";
-    }
     OS << "          continue;\n";
     OS << "        }\n";
     OS << "        OperandsValid = false;\n";
@@ -3812,9 +3787,8 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
      << "      // then try to match next formal operand\n";
   OS << "      if (Diag == Match_InvalidOperand "
      << "&& isSubclass(Formal, OptionalMatchClass)) {\n";
-  if (HasOptionalOperands) {
+  if (HasOptionalOperands)
     OS << "        OptionalOperandsMask.set(FormalIdx);\n";
-  }
   OS << "        DEBUG_WITH_TYPE(\"asm-matcher\", dbgs() << \"ignoring "
         "optional operand\\n\");\n";
   OS << "        continue;\n";

>From 4a2344862510740ca8181e140118cd7801931725 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Fri, 5 Sep 2025 06:53:20 -0700
Subject: [PATCH 2/2] Use make_second_range

---
 llvm/utils/TableGen/AsmMatcherEmitter.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index 2ad76af602d8e..29d26d5237695 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -2699,7 +2699,8 @@ static void emitGetSubtargetFeatureName(AsmMatcherInfo &Info, raw_ostream &OS) {
      << "static const char *getSubtargetFeatureName(uint64_t Val) {\n";
   if (!Info.SubtargetFeatures.empty()) {
     OS << "  switch(Val) {\n";
-    for (const auto &[_, SFI] : Info.SubtargetFeatures) {
+    for (const SubtargetFeatureInfo &SFI :
+         make_second_range(Info.SubtargetFeatures)) {
       // FIXME: Totally just a placeholder name to get the algorithm working.
       OS << "  case " << SFI.getEnumBitName() << ": return \""
          << SFI.TheDef->getValueAsString("PredicateName") << "\";\n";



More information about the llvm-commits mailing list