[PATCH] D82545: [Debugify] Make the debugify aware of the original (-g) Debug Info

Djordje Todorovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 3 01:13:20 PST 2021


djtodoro added inline comments.


================
Comment at: llvm/test/DebugInfo/debugify-original-dbginfo.ll:5
+
+; CHECK: CheckModuleDebugify (original debuginfo): PASS
+
----------------
aprantl wrote:
> djtodoro wrote:
> > aprantl wrote:
> > > This test has the same fundamental problem: It may fail on a different target, when instcombine is updated, ...
> > > Can you do the same unittest trick here? (perhaps with an idempotent transformation?)
> > Sure.
> > 
> > >This test has the same fundamental problem: It may fail on a different target, when instcombine is updated, ...
> > just curious -- I thought that all targets should handle Debug Info on the IR level the same way, but obviously that is not true?
> > just curious -- I thought that all targets should handle Debug Info on the IR level the same way, but obviously that is not true?
> 
> My understanding is that InstCombine has hooks for targets to apply target-specific combines that are profitable only on that architecture, but I'm far from an expert here :-)
> 
> The main problem is that this RUN line basically asserts that "instcombine behaves correctly in terms of debug info". While this may be true at the moment, at some point in the future someone will make an unrelated change to the IR, or to instcombine, or add a new target that will cause this test to break, but there is no indication in this test for how to fix it.
> I think this test as it is might be okay inside the InstCombine subdirectory, to cement the fact that whatever InstCombine rules apply here behave correctly, but it's not so good as a test for the debugify pass, because it depends on the whole complex InstCombine machinery to behave in a specific way.
> 
got it, thanks for the explanation -- I've learned something new! :)


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

https://reviews.llvm.org/D82545



More information about the llvm-commits mailing list