[PATCH] D83047: [LiveDebugValues] 2/4 Add instruction-referencing LiveDebugValues implementation

Jeremy Morse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 10 14:45:41 PDT 2020


jmorse added a comment.

In D83047#2208102 <https://reviews.llvm.org/D83047#2208102>, @vsk wrote:

> I'm having trouble paging in enough of the DBG_INSTR_REF patch queue to continue giving meaningful review. I'm not sure the traditional review process is a great fit for this, since the changes are at a prototype stage. Perhaps we might be better served by landing things as they are, as a separate code path guarded behind a cl::opt? This would allow more people to test and contribute patches. Wdyt? (cc @aprantl @davide)

I was a bit worried too; it is after all a massive code bomb, but I can't see a way of splitting it (InstrRefBasedLDV) into independently-useful parts, and it's at the root of all the other DBG_INSTR_REF work.

To state the obvious, it's a bad plan to have a pass that only one person understands. One option might be, assuming landing in a prototype behind-a-flag state, building up a suite of tests that take a specific path through the dataflow code and explain (in test comments) what they're doing and why. This is kind of what @TWeaver did with the tests at llvm/test/DebugInfo/MIR/X86/livedebugvalues_* to stimulate different paths through VarLoc LiveDebugValues, and could / would / should aid future understanding of what's going on.

Almost-worst-case scenario: the whole pass/file could be decomposed into three parts:

- Transfer function building
- Machine value-numbering dataflow
- Variable value dataflow

With debug-like options to spit out the current state between each part, for which tests can be independently written. This would be fairly awkward, but worthwhile if it allows things to be understood.


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

https://reviews.llvm.org/D83047



More information about the llvm-commits mailing list