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

[Jeremy Morse via llvm-commits]

jmorse wrote:

Thanks for the patch -- I have the feeling that this is treating the symptom rather than the cause, alas. This situation seems consistent with something mentioned on discourse [0], PHIs getting inserted into the wrong place leads to mixing of debug-records and PHIs. We were aware that if you inserted instructions into block using instruction pointers rather than iterators, you might end up with debug records attached one instruction adjacent to where it should be. Unfortunately, with blocks of PHIs, having a debug-record in the wrong place ends up being a hard verifier error, and we've accidentally glossed over that flaw (mea culpa).

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. Plus we would end up with multiple ways of representing the same inputs. However if this flaw is currently causing serious problems then we can land this as a workaround.

If you're OK with waiting a couple of days, there's a more general solution, which is our bulk-updates of all of LLVM to use iterators instead of `Instruction *` for insertion -- see 6b62a9135a28 for example. We've got a patch that does 100% of this which we're cutting up and pushing into LLVM (it's NFC and mechanical). There's a bunch of PHI-creation sites that get updated, and hopefully the use of iterators there will solve whatever's causing your mixing of debug-records and PHIs, assuming that it's coming from an upstream pass. For downstream creation of PHI nodes: use `getFirstNonPHIIt` to identify the location and insert using the iterator-taking insert/Create/constructor methods.

> It's surprising that the verifier modifies the IR but this is presumably temporary until RemoveDI's finishes. More surprising is that F.dump() performs this conversion but BB.dump() does not.

It's an awkward choice; that's another sticking plaster because the textual IR printing situation wasn't finalised -- although I believe that's now landed today. (CC @OCHyams and @SLTozer for visibility)

[0] https://discourse.llvm.org/t/rfc-instruction-api-changes-needed-to-eliminate-debug-intrinsics-from-ir/68939/19

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