[PATCH] D40512: [Debugify] Add a pass to test debug info preservation

[Vedant Kumar via Phabricator via llvm-commits]

vsk marked 21 inline comments as done.
vsk added a comment.

Thanks everybody for your feedback. I think I've addressed all of it now. I'll commit this tomorrow barring any further comments.



================
Comment at: tools/opt/Debugify.cpp:10-11
+//
+// This pass attaches synthetic debug info to everything. It can be used to
+// create targeted tests for debug info preservation.
+//
----------------
MatzeB wrote:
> vsk wrote:
> > MatzeB wrote:
> > > Could use `/// \file This pass ...` so doxygen picks it up.
> > Will fix.
> Needs three slashes for doxygen :)
Will fix.


================
Comment at: tools/opt/Debugify.cpp:79
+      for (Instruction &I : BB)
+        I.setDebugLoc(DILocation::get(Ctx, NextLine++, 1, SP));
+
----------------
vsk wrote:
> aprantl wrote:
> > Sorry for having this idea so late, but: Would it make sense to combine this with the DebugIR pass, so, instead of assigning arbitrary artificial DILocations, the DILocations actually point back to the original instructions in the IR and are perhaps more useful when debugging a pass?
> I don't think there's a way to unify the two tools in a way that's simpler than keeping them separate. We need separate code paths to mark-up *.ll files on disk vs. in-memory Modules. Secondly, to implement -check-debugify on DebugIR output, we'd need to do do extra work to store a list of all DILocations in a separate piece of metadata. IMO having a separate code path for DebugIR would be neater and less complicated.
Adrian and I had an off-line conversation about this, in which we reached the conclusion that it would be neater to keep the DebugIR code path separate from this pass.


https://reviews.llvm.org/D40512