[PATCH] D91595: [Debugify] Support checking Machine IR debug info

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCheckDebugify.cpp:56
+      MachineFunction &MF = *MaybeMF;
+      for (MachineBasicBlock &MBB : MF) {
+        for (MachineInstr &MI : MBB) {
----------------
djtodoro wrote:
> I'm not against adding two loops -- one for finding missing lines, and the other one for finding missing variables.
Yes, two loops is clear


================
Comment at: llvm/lib/CodeGen/MachineCheckDebugify.cpp:58
+        for (MachineInstr &MI : MBB) {
+          if (MI.isDebugValue()) {
+            const DILocalVariable *LocalVar = MI.getDebugVariable();
----------------
djtodoro wrote:
> Not sure we want to report missing dbgLoc for meta instructions other than `DBG_VALUE` (e.g., `DBG_LABEL` does not end up in the final code as well..)
For missing line, we should check all the MIRs which contribute the line number in MachineDebugify -- (non DBG_VALUEs), so here we just need to check MI.isDebugValue().


================
Comment at: llvm/lib/CodeGen/MachineCheckDebugify.cpp:67
+            const DebugLoc DL = MI.getDebugLoc();
+            if (!DL) {
+              errs() << "WARNING: Instruction with empty DebugLoc in function ";
----------------
djtodoro wrote:
> Early continue?
Still need to do line 73, I'll let it clear by using 2 loops soon. thank you!


================
Comment at: llvm/lib/CodeGen/MachineCheckDebugify.cpp:71
+              MI.print(errs());
+              errs() << "\n";
+            } else if (DL.getLine() != 0)
----------------
Duplicated "\n" with MI.print(), remove here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91595/new/

https://reviews.llvm.org/D91595