[PATCH] D40965: [CodeGen] Move printing MO_Immediate operands to MachineOperand::print

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: include/llvm/CodeGen/MachineOperand.h:248-259
+  /// \param IsOperandSubregIdx - MO_Immediate operands can also be subreg
+  /// idices. If it's the case, the subreg index name will be printed.
+  /// MachineInstr::isOperandSubregIdx can be called to check this.
   /// \param TRI - provide more target-specific information to the printer.
   /// Unlike the previous function, this one will not try and get the
   /// information from it's parent.
   /// \param IntrinsicInfo - same as \p TRI.
----------------
Would it make sense to move the subreg index printing into its own function instead of a toggle flag? As in that case most of the other arguments won't be used.


================
Comment at: lib/CodeGen/MachineInstr.cpp:1262-1266
+    bool IsOperandSubregIdx =
+        getOperand(StartOp).isImm() && isOperandSubregIdx(StartOp);
     getOperand(StartOp).print(OS, MST, TypeToPrint, /*PrintDef=*/false,
-                              ShouldPrintRegisterTies, TiedOperandIdx, TRI,
-                              IntrinsicInfo);
+                              ShouldPrintRegisterTies, TiedOperandIdx,
+                              IsOperandSubregIdx, TRI, IntrinsicInfo);
----------------
This case must be a register def I think, so it cannot be a subregidx operand.


================
Comment at: lib/CodeGen/MachineInstr.cpp:1295-1299
+    bool IsOperandSubregIdx =
+        getOperand(OpIdx).isImm() && isOperandSubregIdx(OpIdx);
     getOperand(OpIdx).print(OS, MST, TypeToPrint, /*PrintDef=*/true,
-                            ShouldPrintRegisterTies, TiedOperandIdx, TRI,
-                            IntrinsicInfo);
+                            ShouldPrintRegisterTies, TiedOperandIdx,
+                            IsOperandSubregIdx, TRI, IntrinsicInfo);
----------------
An ASM also wouldn't have subregidx operands.


================
Comment at: lib/CodeGen/MachineInstr.cpp:1345-1348
+        bool IsOperandSubregIdx = MO.isImm() && isOperandSubregIdx(i);
         MO.print(OS, MST, TypeToPrint, /*PrintDef=*/true,
-                 ShouldPrintRegisterTies, TiedOperandIdx, TRI, IntrinsicInfo);
+                 ShouldPrintRegisterTies, TiedOperandIdx, IsOperandSubregIdx,
+                 TRI, IntrinsicInfo);
----------------
DBG_VALUEs also don't have subregidx operands.


https://reviews.llvm.org/D40965