[PATCH] D46100: WIP: [IR/Verifier] Diagnose use-before-def scenarios for debug intrinsics

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

A problem with CodeGenPrepare::placeDbgValues is that it can move debug intrinsics more or less blindly, for example without taking other debug intrinsics refering to the same variable into account.
If there is a use-before-def scenario it often is correct to move the using dbg.value below the def, but that depends on which transform that resulted in such a situation (I guess that often it might be due to an opt-pass not caring about debug intrinsics).
Right now CodeGenPrepare::placeDbgValues also hoists dbg.value instructions when the use already is after the def. If I remember correctly it can even move a dbg.value from one basic block to another.

We have TRs where we have noticed that debug-experience gets really weird due to CodeGenPrepare::placeDbgValues, because it has incorrectly moved dbg.value instructions. I've actually been aiming at removing that function completely.

(FWIW, the description in the beginning of CodeGenPrepare.cpp actually hints that the pass itself should be removed, that is however a different story and maybe that comment is out-dated, but I doubt that the original goal with that pass was that it should be a cleanup pass that tries to mend things that are broken elsewhere without having the knowledge about what the correct IR actually looked like)

I think that CodeGenPrepare::placeDbgValues exists for two different reasons:

1. Bugs in IR passes introducing use-before-def scenarios (so if we remove the function we will lose some debug information that might (or might not) be valid today).
2. Limitations in SelectionDAGBuilder, for example when it comes to properly handle "dangling" debug info between basic blocks, resulting in loss of debug information when the debug-use isn't in the same BB as def.

The verifier check can help us with (1).
I've got some ongoing patches trying to improve (2), but unfortunately I haven't had time to continue on that work since before summer.

One thing that has been nagging me is in which order to do things. Removing CodeGenPrepare::placeDbgValues would be the main goal. But without first doing fixes in SelectionDAG it gives quite different results (getting rid of some faulty debug-info, but also losing some valid debug-info). If I do not remove CodeGenPrepare::placeDbgValues first, then it is harder to motivate patches in SelectionDAG, since the problems are hidden by placeDbgValues (test cases must start after CodeGenPrepare). And maybe we need to solve (1) and adding the verifier check before removing placeDbgValues, because I guess noone has verified that SelectionDAG can handle debug-use-before-def in a safe way.


https://reviews.llvm.org/D46100