[PATCH] D31750: [globalisel] Enable tracing the legalizer with --debug-only=legalize-mir

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/Legalizer.cpp:174
         continue;
+      SmallVector<MachineInstr *, 4> DebugReportList LLVM_ATTRIBUTE_UNUSED;
       SmallVector<MachineInstr *, 4> WorkList;
----------------
I think that just having an "unsigned DebugNewInstructionsIdx" or something similar could be enough to track the new MIs that are created but haven't been printed yet for debugging; no need for a separate SmallVector?
I think it makes the code slightly easier to understand, as it's then immediately clear that MI's from the WorkList are being printed, rather than having to look through code how this new data structure DebugReportList is being formed.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:74-77
+  MIRBuilder.recordInsertions([&](MachineInstr *MI) {
+    DEBUG(DebugReportList.push_back(MI));
+    WorkList.push_back(MI);
+  });
----------------
dsanders wrote:
> dsanders wrote:
> > kristof.beyls wrote:
> > > This patch looks pretty straightforward.
> > > I'm just wondering why you don't just directly print out the New MI here and instead use a data structure to record it and print it out later?
> > I didn't give it enough thought to notice it was redundant :-). I'll make it emit it directly once I've finished updating the other patches.
> I got around to trying to make this change and remembered why I recorded the MI and printed it out afterwards. The operands haven't been added to the instruction at the point that recordInsertions() is called so it only prints the opcode. This makes it hard to trace the changes the pass is making.
Ah yes, that makes sense.


https://reviews.llvm.org/D31750