[NFC][MIR] Fix extra whitespace in MIR printing

[jurahul]

Fix a whitespace regression in MIR printing that was introduced in #137361.

The default value for ListSeparator is ", ", so we don't need to print an additional space in front of tokens for optional symbols and other things printed after operands.

Note, the modified LIT test will fail at trunk without the fix, demonstrating that the extra space before , pre-instr-symbol <mcsymbol > on Line 63 exists currently and is fixed with this change.

[llvmbot]

@llvm/pr-subscribers-llvm-adt

Author: Rahul Joshi (jurahul)

Changes

Fix a whitespace regression in MIR printing that was introduced in #137361.

The default value for ListSeparator is ", ", so we don't need to print an additional space in front of tokens for optional symbols and other things printed after operands.

Note, the modified LIT test will fail at trunk without the fix, demonstrating that the extra space before , pre-instr-symbol <mcsymbol > on Line 63 exists currently and is fixed with this change.

---

Full diff: https://github.com/llvm/llvm-project/pull/162928.diff

3 Files Affected:

• (modified) llvm/include/llvm/ADT/StringExtras.h (+4-2)
• (modified) llvm/lib/CodeGen/MIRPrinter.cpp (+14-16)
• (modified) llvm/test/CodeGen/MIR/AArch64/return-address-signing.mir (+1-1)

diff --git a/llvm/include/llvm/ADT/StringExtras.h b/llvm/include/llvm/ADT/StringExtras.h
index 7d81c63485be2..2440e7678a831 100644
--- a/llvm/include/llvm/ADT/StringExtras.h
+++ b/llvm/include/llvm/ADT/StringExtras.h
@@ -529,13 +529,15 @@ inline std::string join_items(Sep Separator, Args &&... Items) {
 class ListSeparator {
   bool First = true;
   StringRef Separator;
+  StringRef Prefix;
 
 public:
-  ListSeparator(StringRef Separator = ", ") : Separator(Separator) {}
+  ListSeparator(StringRef Separator = ", ", StringRef Prefix = "")
+      : Separator(Separator), Prefix(Prefix) {}
   operator StringRef() {
     if (First) {
       First = false;
-      return {};
+      return Prefix;
     }
     return Separator;
   }
diff --git a/llvm/lib/CodeGen/MIRPrinter.cpp b/llvm/lib/CodeGen/MIRPrinter.cpp
index bf8a6cdf097a9..368aab73498ec 100644
--- a/llvm/lib/CodeGen/MIRPrinter.cpp
+++ b/llvm/lib/CodeGen/MIRPrinter.cpp
@@ -866,48 +866,46 @@ static void printMI(raw_ostream &OS, MFPrintState &State,
 
   OS << TII->getName(MI.getOpcode());
 
-  LS = ListSeparator();
+  // Print a space after the opcode if any additional tokens are printed.
+  LS = ListSeparator(", ", " ");
 
-  if (I < E) {
-    OS << ' ';
-    for (; I < E; ++I) {
-      OS << LS;
-      printMIOperand(OS, State, MI, I, TRI, TII, ShouldPrintRegisterTies,
-                     PrintedTypes, MRI, /*PrintDef=*/true);
-    }
+  for (; I < E; ++I) {
+    OS << LS;
+    printMIOperand(OS, State, MI, I, TRI, TII, ShouldPrintRegisterTies,
+                   PrintedTypes, MRI, /*PrintDef=*/true);
   }
 
   // Print any optional symbols attached to this instruction as-if they were
   // operands.
   if (MCSymbol *PreInstrSymbol = MI.getPreInstrSymbol()) {
-    OS << LS << " pre-instr-symbol ";
+    OS << LS << "pre-instr-symbol ";
     MachineOperand::printSymbol(OS, *PreInstrSymbol);
   }
   if (MCSymbol *PostInstrSymbol = MI.getPostInstrSymbol()) {
-    OS << LS << " post-instr-symbol ";
+    OS << LS << "post-instr-symbol ";
     MachineOperand::printSymbol(OS, *PostInstrSymbol);
   }
   if (MDNode *HeapAllocMarker = MI.getHeapAllocMarker()) {
-    OS << LS << " heap-alloc-marker ";
+    OS << LS << "heap-alloc-marker ";
     HeapAllocMarker->printAsOperand(OS, State.MST);
   }
   if (MDNode *PCSections = MI.getPCSections()) {
-    OS << LS << " pcsections ";
+    OS << LS << "pcsections ";
     PCSections->printAsOperand(OS, State.MST);
   }
   if (MDNode *MMRA = MI.getMMRAMetadata()) {
-    OS << LS << " mmra ";
+    OS << LS << "mmra ";
     MMRA->printAsOperand(OS, State.MST);
   }
   if (uint32_t CFIType = MI.getCFIType())
-    OS << LS << " cfi-type " << CFIType;
+    OS << LS << "cfi-type " << CFIType;
 
   if (auto Num = MI.peekDebugInstrNum())
-    OS << LS << " debug-instr-number " << Num;
+    OS << LS << "debug-instr-number " << Num;
 
   if (PrintLocations) {
     if (const DebugLoc &DL = MI.getDebugLoc()) {
-      OS << LS << " debug-location ";
+      OS << LS << "debug-location ";
       DL->printAsOperand(OS, State.MST);
     }
   }
diff --git a/llvm/test/CodeGen/MIR/AArch64/return-address-signing.mir b/llvm/test/CodeGen/MIR/AArch64/return-address-signing.mir
index 1030917c87419..302f70fc15192 100644
--- a/llvm/test/CodeGen/MIR/AArch64/return-address-signing.mir
+++ b/llvm/test/CodeGen/MIR/AArch64/return-address-signing.mir
@@ -1,4 +1,4 @@
-# RUN: llc -mtriple=aarch64 -run-pass=prologepilog -run-pass=aarch64-ptrauth -o - %s 2>&1 | FileCheck %s
+# RUN: llc -mtriple=aarch64 -run-pass=prologepilog -run-pass=aarch64-ptrauth -o - %s 2>&1 | FileCheck --strict-whitespace %s
 --- |
   target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
   target triple = "aarch64"

[kuhar]

Can you add unit tests for ListSeparator?

[jurahul]

Added unit test for ListSeparator changes.