[PATCH] D70597: [PHIEliminate] skip dbg instruction when LowerPHINode

Thu Nov 28 03:53:47 PST 2019

jmorse requested changes to this revision.
jmorse added a subscriber: debug-info.
jmorse added a comment.
This revision now requires changes to proceed.

(Responding to inline comments) I agree with @bjope, this would have a large effect on bunches of PHIs immediately followed by bunches of DBG_VALUEs that refer to them. This is a common case in most of the IR that I look at, mem2reg puts them there, and we'd lose quite a lot of variable coverage.

I like the idea of canonicalising the order of these instructions so that the DBG_VALUEs always follow the other meta instructions. I'm pretty confident we should be able to re-order DBG_VALUEs with other non-debug meta instructions -- I'm not aware of debug facilities depending on EH_LABEL / GC_LABEL in any way, and I don't believe it'd make sense for them to depend on debugging information.

Alternately, it might be easier to study where EH_LABEL / GC_LABEL are created and guarantee that they always come before debug instructions. This might be as simple as having the DBG_VALUE creating code placing DBG_VALUEs after PHIs and labels [0], but I don't know how the *_LABEL things work. There's then the risk that optimisations moving DBG_VALUEs re-introduce this condition, but it would avoid extra computation during PHI elimination.

[0] https://github.com/llvm/llvm-project/blob/742043047c973999eac7734e53f7872973933f24/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp#L935

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

https://reviews.llvm.org/D70597