[llvm] Avoid dbg.value between phi nodes when converting from DbgRecords (PR #83620)

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 4 17:39:44 PST 2024


dsandersllvm wrote:

Thanks for the review. This is occurring in a downstream pass that I've just confirmed is already using iterators as the insertion point for the cloned instructions (specifically, end() or getFirstInsertionPt()) so I suspect that upcoming patch won't fix our crash unless it somehow ensures that phis never have debug info so that adding a new phi doesn't leave the second-to-last phi with debug info.

> I think we should avoid tolerating debug-records attached to PHIs (as this patch leans towards) as there's code out there which scans through all post-PHI instructions in blocks for debug-info and we'll be messing with that. 

Am I right in understanding this as "phi's should never have debug info"? If so, two questions arise from that. The first is: What happens when code like this is converted?:
```
%0 = phi ...
call @llvm.dbg.value(%0, ...)
```
If the new debug info attaches to previous instructions as I currently think it does then any debug info attaching to a phi like this won't be found by code that scans the post-PHI instructions because those dbg.values end up attaching to the last phi and therefore aren't in the post-PHI instructions anymore. It may be that the debug info transfers to the phi's inputs but if that's the case then I'm not sure how a variable would still be tracked after the phi, nor how it's transferred back to it's original position and vreg in time for codegen which is still based on dbg.value's from what I've read so far.

The second is how we had a phi with debug info attached in the input to our pass to begin with. Our crash arises from adding a phi node after one that's already carrying debug info. The debug info in question originates from debugify so if there's new restrictions on the placement of debug info then that pass might not be aware of them

> However if this flaw is currently causing serious problems then we can land this as a workaround.

We already landed this patch downstream to clear up our CI problems and we'll revert+replace or adapt it based on how it's resolved upstream.

https://github.com/llvm/llvm-project/pull/83620


More information about the llvm-commits mailing list