[RemoveDIs] Use iterators for moving PHIs in loop-unroll-and-jam

[jmorse]

With no debug intrinsics, correctly identifying the start of a block with iterators becomes important. We need to use the iterator-returning methods here in loop-unroll-and-jam where we're shifting PHIs around. Otherwise they can be inserted after debug-info records, leading to debug-info attached to PHIs, which is ill formed.

Fixes #83000

~

It looks like this wasn't caused in various other bits of QA, which is unfortunate. The sure-fire way of eliminating all these scenarios is deleting the relevant instruction-insertion APIs, patches for which we're now uploading.

I'm inclined to not add a regression test for this: it's an error that's liable to pop up anywhere in LLVM, and we're going to eliminate all these scenarios through using the correct types in the future. If needed we can add the one in the problem report.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

Changes

With no debug intrinsics, correctly identifying the start of a block with iterators becomes important. We need to use the iterator-returning methods here in loop-unroll-and-jam where we're shifting PHIs around. Otherwise they can be inserted after debug-info records, leading to debug-info attached to PHIs, which is ill formed.

Fixes #83000

~

It looks like this wasn't caused in various other bits of QA, which is unfortunate. The sure-fire way of eliminating all these scenarios is deleting the relevant instruction-insertion APIs, patches for which we're now uploading.

I'm inclined to not add a regression test for this: it's an error that's liable to pop up anywhere in LLVM, and we're going to eliminate all these scenarios through using the correct types in the future. If needed we can add the one in the problem report.

---

Full diff: https://github.com/llvm/llvm-project/pull/83003.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp (+2-2)

diff --git a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
index 3c06a6e47a3035..26b8c790f2a062 100644
--- a/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
@@ -473,9 +473,9 @@ llvm::UnrollAndJamLoop(Loop *L, unsigned Count, unsigned TripCount,
   };
   // Move all the phis from Src into Dest
   auto movePHIs = [](BasicBlock *Src, BasicBlock *Dest) {
-    Instruction *insertPoint = Dest->getFirstNonPHI();
+    BasicBlock::iterator insertPoint = Dest->getFirstNonPHIIt();
     while (PHINode *Phi = dyn_cast<PHINode>(Src->begin()))
-      Phi->moveBefore(insertPoint);
+      Phi->moveBefore(*Dest, insertPoint);
   };
 
   // Update the PHI values outside the loop to point to the last block

[OCHyams]

I think not-adding a test fits with the existing strategy for these kinds of fixes - LGTM