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

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm marked 2 inline comments as done.
xiangzhangllvm added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCheckDebugify.cpp:40
+
+    auto getDebugifyOperand = [&](unsigned Idx) -> unsigned {
+      return mdconst::extract<ConstantInt>(NMD->getOperand(Idx)->getOperand(0))
----------------
xiangzhangllvm wrote:
> vsk wrote:
> > xiangzhangllvm wrote:
> > > vsk wrote:
> > > > 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.
> > > It is not get/check IR-level metadata info, as explained before, here use the getDebugifyOperand to get "llvm.mir.debugify" info which generated by MIR debugify.
> > Hm, I don't think we're on the same page. My concern is that this patch copies logic from checkDebugifyMetadata (in lib/Transforms/Utils/Debugify.cpp), such as the logic to set up bitvectors to track missing lines. Could you please find a way to implement the warnings in a generic way, and reuse that facility in both the IR & MIR check-debugify passes?
> en..., In fact, we intentionally make the logic same with IR debugify, it is reasonable to check MIR similar with IR. (It has already help me to find some debug info problems in some MIR optimizations).  It is trouble to reuse that facility in both the IR & MIR. The logic of MIR-debugify is some with  IR-debugify too, but MIR-debugify also implemented in a independent pass.
> en..., In fact, we intentionally make the logic same with IR debugify, it is reasonable to check MIR similar with IR. (It has already help me to find some debug info problems in some MIR optimizations).  It is trouble to reuse that facility in both the IR & MIR. The logic of MIR-debugify is some with  IR-debugify too, but MIR-debugify also implemented in a independent pass.




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

https://reviews.llvm.org/D91595