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

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: llvm/docs/HowToUpdateDebugInfo.rst:346
+``MachineInstr`` in a ``Module``. And the MIR-level ``mir-check-debugify`` is
+similar with IR-level ``check-debugify`` pass.
 
----------------
nit, s/with/to the/


================
Comment at: llvm/lib/CodeGen/MachineCheckDebugify.cpp:33
+
+    NamedMDNode *NMD = M.getNamedMetadata("llvm.mir.debugify");
+    if (!NMD) {
----------------
Why does this need to be named differently from the IR-level named debugify metadata?


================
Comment at: llvm/lib/CodeGen/MachineCheckDebugify.cpp:40
+
+    auto getDebugifyOperand = [&](unsigned Idx) -> unsigned {
+      return mdconst::extract<ConstantInt>(NMD->getOperand(Idx)->getOperand(0))
----------------
I don't think it's a good idea to introduce a copy of the IR-level check-debugify logic, since this makes it possible for the IR & MIR-level checking behaviors to diverge.

Can you find a way to factor out and reuse the checking logic? One way to do it might be to introduce a generic DebugifyChecker which parses the named metadata and operates on DebugLoc.


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

https://reviews.llvm.org/D91595