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

[Daniel Sanders via llvm-commits]

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

>From 2ed2b4632c447f8de9bf1eb2998e23acd147d42d Mon Sep 17 00:00:00 2001
From: Daniel Sanders <daniel_l_sanders at apple.com>
Date: Fri, 1 Mar 2024 12:46:32 -0800
Subject: [PATCH] Avoid dbg.value between phi nodes when converting from
 DbgRecords

We have a downstream pass that at some point clones a phi instruction that is
carrying some DPValues. When the verifier is called on IR containing phi nodes
like:
```
%0 = phi ... ; has DPValues
%1 = phi ... ; cloned from previous phi
```
it currently converts to the debug intrinsics like so:
```
%0 = phi ...
call @llvm.dbg.value(...)
call @llvm.dbg.value(...)
...
%1 = phi ...
```
and then fails to verify because the phi's aren't first.

This patch moves the insertion point to a position after the phi nodes and
any dbg.value calls it has already added.

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.

Unfortunately, I don't have a test case I can share as this problem occurred
in a downstream pass and I'm not aware of any upstream that can be made to
similarly clone a phi node.
---
 llvm/lib/IR/BasicBlock.cpp | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 25aa3261164513..1de5457e702825 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -109,9 +109,13 @@ void BasicBlock::convertFromNewDbgValues() {
       continue;
 
     DPMarker &Marker = *Inst.DbgMarker;
-    for (DbgRecord &DR : Marker.getDbgValueRange())
-      InstList.insert(Inst.getIterator(),
-                      DR.createDebugIntrinsic(getModule(), nullptr));
+    for (DbgRecord &DR : Marker.getDbgValueRange()) {
+      auto I = Inst.getIterator();
+      // Avoid inserting debug info between phi nodes.
+      if (isa<PHINode>(Inst))
+        I = getFirstNonPHIOrDbg()->getIterator();
+      InstList.insert(I, DR.createDebugIntrinsic(getModule(), nullptr));
+    }
 
     Marker.eraseFromParent();
   }