[llvm] [NFC][DebugInfo] Deprecate iterator-taking moveBefore and getFirstNonPHI (PR #124290)
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 27 10:29:46 PST 2025
https://github.com/jmorse updated https://github.com/llvm/llvm-project/pull/124290
>From cbfc1053b045bf52e33fb6573667196061ebf301 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Fri, 24 Jan 2025 09:06:01 +0000
Subject: [PATCH 1/2] [NFC][DebugInfo] Deprecate iterator-taking moveBefore and
getFirstNonPHI
The RemoveDIs project has moved debugging information out of intrinsics,
but metadata is sometimes required when inserting instructions at the start
of blocks. To maintain accuracy, in the future instruction insertion needs
to be performed with a BasicBlock::iterator, so that a debug-info bit can
be stored in the iterator.
To that end, we're deprecating getFirstNonPHI and moveBefore for
instruction pointers. They're replaced by getFirstNonPHIIt and an
iterator-taking moveBefore: switching to the replacement is
straightforwards, and 99% of call-sites need only to unwrap the iterator
with &* or call getIterator() on an Instruction pointer.
The exception is when inserting instructions at the start of a block: if
you call getFirstNonPHI() (or begin() or getFirstInsertionPt()) and then
insert something at that position, you must pass the BasicBlock::iterator
returned into the insertion method. Unwrapping with &* and then calling
getIterator strips the debug-info bit we wish to preserve. Please do
contact us about any use case that's confusing or unclear:
https://discourse.llvm.org/t/psa-ir-output-changing-from-debug-intrinsics-to-debug-records/79578
---
llvm/include/llvm/IR/BasicBlock.h | 16 +++++++++++++--
llvm/include/llvm/IR/Instruction.h | 33 +++++++++++++++++++++++-------
llvm/lib/IR/BasicBlock.cpp | 14 +++++++++----
llvm/lib/IR/Instruction.cpp | 2 +-
4 files changed, 51 insertions(+), 14 deletions(-)
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..2d38fbabcca397 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -280,14 +280,26 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
/// When adding instructions to the beginning of the basic block, they should
/// be added before the returned value, not before the first instruction,
/// which might be PHI. Returns 0 is there's no non-PHI instruction.
+ ///
+ /// Deprecated in favour of getFirstNonPHIIt, which returns an iterator that
+ /// preserves some debugging information.
+ LLVM_DEPRECATED("Use iterators as instruction positions", "getFirstNonPHIIt")
const Instruction* getFirstNonPHI() const;
+ LLVM_DEPRECATED("Use iterators as instruction positions", "getFirstNonPHIIt")
Instruction* getFirstNonPHI() {
return const_cast<Instruction *>(
static_cast<const BasicBlock *>(this)->getFirstNonPHI());
}
- /// Iterator returning form of getFirstNonPHI. Installed as a placeholder for
- /// the RemoveDIs project that will eventually remove debug intrinsics.
+ /// Returns an iterator to the first instruction in this block that is not a
+ /// PHINode instruction.
+ ///
+ /// When adding instructions to the beginning of the basic block, they should
+ /// be added before the returned value, not before the first instruction,
+ /// which might be PHI. Returns end() if there's no non-PHI instruction.
+ ///
+ /// Avoid unwrapping the iterator to an Instruction* before inserting here,
+ /// as debug-info relevant information is preserved in the iterator.
InstListType::const_iterator getFirstNonPHIIt() const;
InstListType::iterator getFirstNonPHIIt() {
BasicBlock::iterator It =
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 6cdd79ce16005c..900384432d75d0 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -206,6 +206,12 @@ class Instruction : public User,
/// Insert an unlinked instruction into a basic block immediately before
/// the specified instruction.
+ ///
+ /// Deprecated in favour of the iterator-accepting flavour. Iterators at the
+ /// start of a block such as BasicBlock::getFirstNonPHIIt must be passed into
+ /// insertBefore without unwrapping/rewrapping. For all other positions, call
+ /// getIterator to fetch the instruction iterator.
+ LLVM_DEPRECATED("Use iterators as instruction positions", "")
void insertBefore(Instruction *InsertPos);
/// Insert an unlinked instruction into a basic block immediately before
@@ -229,6 +235,12 @@ class Instruction : public User,
/// Unlink this instruction from its current basic block and insert it into
/// the basic block that MovePos lives in, right before MovePos.
+ ///
+ /// Deprecated in favour of the iterator-accepting flavour. Iterators at the
+ /// start of a block such as BasicBlock::getFirstNonPHIIt must be passed into
+ /// moveBefore without unwrapping/rewrapping. For all other positions, call
+ /// getIterator to fetch the instruction iterator.
+ LLVM_DEPRECATED("Use iterators as instruction positions", "")
void moveBefore(Instruction *MovePos);
/// Unlink this instruction from its current basic block and insert it into
@@ -238,8 +250,20 @@ class Instruction : public User,
/// Perform a \ref moveBefore operation, while signalling that the caller
/// intends to preserve the original ordering of instructions. This implicitly
/// means that any adjacent debug-info should move with this instruction.
- /// This method is currently a no-op placeholder, but it will become
- /// meaningful when the "RemoveDIs" project is enabled.
+ void moveBeforePreserving(InstListType::iterator MovePos);
+
+ /// Perform a \ref moveBefore operation, while signalling that the caller
+ /// intends to preserve the original ordering of instructions. This implicitly
+ /// means that any adjacent debug-info should move with this instruction.
+ void moveBeforePreserving(BasicBlock &BB, InstListType::iterator I);
+
+ /// Perform a \ref moveBefore operation, while signalling that the caller
+ /// intends to preserve the original ordering of instructions. This implicitly
+ /// means that any adjacent debug-info should move with this instruction.
+ ///
+ /// Deprecated in favour of the iterator-accepting flavour of
+ /// moveBeforePreserving, as all insertions should be at iterator positions.
+ LLVM_DEPRECATED("Use iterators as instruction positions", "")
void moveBeforePreserving(Instruction *MovePos);
private:
@@ -253,11 +277,6 @@ class Instruction : public User,
/// \pre I is a valid iterator into BB.
void moveBefore(BasicBlock &BB, InstListType::iterator I);
- void moveBeforePreserving(BasicBlock &BB, InstListType::iterator I);
- /// Unlink this instruction from its current basic block and insert it into
- /// the basic block that MovePos lives in, right before MovePos.
- void moveBeforePreserving(InstListType::iterator I);
-
/// Unlink this instruction from its current basic block and insert it into
/// the basic block that MovePos lives in, right after MovePos.
void moveAfter(Instruction *MovePos);
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 0efc04cb2c8679..116869c3f6cfa9 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -372,9 +372,16 @@ const Instruction* BasicBlock::getFirstNonPHI() const {
}
BasicBlock::const_iterator BasicBlock::getFirstNonPHIIt() const {
- const Instruction *I = getFirstNonPHI();
+ const Instruction *I = [&]() -> const Instruction * {
+ for (const Instruction &I : *this)
+ if (!isa<PHINode>(I))
+ return &I;
+ return nullptr;
+ }();
+
if (!I)
return end();
+
BasicBlock::const_iterator It = I->getIterator();
// Set the head-inclusive bit to indicate that this iterator includes
// any debug-info at the start of the block. This is a no-op unless the
@@ -414,11 +421,10 @@ BasicBlock::getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp) const {
}
BasicBlock::const_iterator BasicBlock::getFirstInsertionPt() const {
- const Instruction *FirstNonPHI = getFirstNonPHI();
- if (!FirstNonPHI)
+ const_iterator InsertPt = getFirstNonPHIIt();
+ if (InsertPt == end())
return end();
- const_iterator InsertPt = FirstNonPHI->getIterator();
if (InsertPt->isEHPad()) ++InsertPt;
// Set the head-inclusive bit to indicate that this iterator includes
// any debug-info at the start of the block. This is a no-op unless the
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 4ab47edf3ed7d2..84d9306ca67002 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -1169,7 +1169,7 @@ bool Instruction::mayThrow(bool IncludePhaseOneUnwind) const {
// Landingpads themselves don't unwind -- however, an invoke of a skipped
// landingpad may continue unwinding.
BasicBlock *UnwindDest = cast<InvokeInst>(this)->getUnwindDest();
- Instruction *Pad = UnwindDest->getFirstNonPHI();
+ BasicBlock::iterator Pad = UnwindDest->getFirstNonPHIIt();
if (auto *LP = dyn_cast<LandingPadInst>(Pad))
return canUnwindPastLandingPad(LP, IncludePhaseOneUnwind);
return false;
>From 989550e871c3daf7ccca7b927332dbcb4a3b547a Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at gmail.com>
Date: Mon, 27 Jan 2025 18:29:38 +0000
Subject: [PATCH 2/2] Update llvm/include/llvm/IR/BasicBlock.h
Co-authored-by: Orlando Cazalet-Hyams <orlandoch.och at gmail.com>
---
llvm/include/llvm/IR/BasicBlock.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index 2d38fbabcca397..41f991357cc8d5 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,7 +299,7 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
/// which might be PHI. Returns end() if there's no non-PHI instruction.
///
/// Avoid unwrapping the iterator to an Instruction* before inserting here,
- /// as debug-info relevant information is preserved in the iterator.
+ /// as important debug-info is preserved in the iterator.
InstListType::const_iterator getFirstNonPHIIt() const;
InstListType::iterator getFirstNonPHIIt() {
BasicBlock::iterator It =
More information about the llvm-commits
mailing list