[PATCH] D40836: [CodeGen] Use MachineOperand::print in the MIRPrinter for MO_Register.

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added inline comments.


================
Comment at: lib/CodeGen/LiveVariables.cpp:238
     // AH =
-    // AL = ... <imp-def EAX>, <imp-kill AH>
     //    = AH
----------------
I also assumed `<imp-kill>` here meant `implicit-use killed`.


================
Comment at: lib/CodeGen/MachineOperand.cpp:261-276
+    if (const MachineInstr *MI = getParent()) {
+      if (const MachineBasicBlock *MBB = MI->getParent()) {
+        if (const MachineFunction *MF = MBB->getParent()) {
+          // Calculate the size of the RegMask
+          const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
+          unsigned RegMaskSize = (TRI->getNumRegs() + 31) / 32;
 
----------------
MatzeB wrote:
> Is this necessary for the printing changes?
No, not necessary. I can commit this separately.


================
Comment at: lib/CodeGen/MachineOperand.cpp:368
                            const TargetIntrinsicInfo *IntrinsicInfo) const {
+  tryToGetTargetInfo(*this, TRI, IntrinsicInfo);
+
----------------
MatzeB wrote:
> This feels a bit strange to me in cases where the user already specified TRI/IntrinsicInfo/...
Indeed, it was one of the questions I was having: should we assume that if the user is using the "complex" printing method then we're not trying to get the target info for them (and remove the default arguments to make it more obvious) ? In other words, should we only call tryToGetTargetInfo on the first ::print method (and obviously document this behavior and do the same for MI, MBB, MF) ?


================
Comment at: test/DebugInfo/MIR/X86/live-debug-vars-unused-arg-debugonly.mir:156
 # CHECKDBG-LABEL: ********** EMITTING LIVE DEBUG VARIABLES **********
-# CHECKDBG-NEXT: !"argc,5"        [0B;0e):0 Loc0=%edi
+# CHECKDBG-NEXT: !"argc,5"        [0B;0e):0 Loc0=debug-use %edi
 # CHECKDBG-NEXT:         [0B;0e):0 %bb.0-160B
----------------
I don't know how much we want this `debug-use` to show up here. What do you think @aprantl?


https://reviews.llvm.org/D40836