[llvm] [RemoveDIs] Use iterators for moving PHIs in loop-unroll-and-jam (PR #83003)

Jeremy Morse via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 26 05:45:21 PST 2024


https://github.com/jmorse created https://github.com/llvm/llvm-project/pull/83003

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.

>From abd731b1bc63dfe26b6d5fad1e0141bc339cf462 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Mon, 26 Feb 2024 13:37:47 +0000
Subject: [PATCH] [RemoveDIs] Use iterators for shifting PHIs around

With no debug intrinsics, correctly identifying the start of a block with
iterators becomes important. Hence, use the iterator-returning methods here
where we're shifting PHIs around in loop-unroll-and-jam. Otherwise they can
be inserted after debug-info records, leading to debug-info attached to
PHIs, which is ill formed.
---
 llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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



More information about the llvm-commits mailing list