[DebugInfo] Enable deprecation of iterator-insertion methods

[jmorse]

Propagate to some other utilities that shouldn't be used, but we don't want to delete just yet. There's some final cleanup in the LLVM Examples which usually don't get built: otherwise everything in the monorepo seems to build cleanly.

I've also added a cast to the default-nullptr PHINode::Create method. Its default-form seems pretty widespread, doing this causes most uses to take the "Being inserted in a BasicBlock that happens to be absent" path as opposed to the "Being inserted with an instruction pointer, which is deprecated" path. We should still get deprecation warnings if anyone passes in an instruction pointer directly.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Jeremy Morse (jmorse)

Changes

Propagate to some other utilities that shouldn't be used, but we don't want to delete just yet. There's some final cleanup in the LLVM Examples which usually don't get built: otherwise everything in the monorepo seems to build cleanly.

I've also added a cast to the default-nullptr PHINode::Create method. Its default-form seems pretty widespread, doing this causes most uses to take the "Being inserted in a BasicBlock that happens to be absent" path as opposed to the "Being inserted with an instruction pointer, which is deprecated" path. We should still get deprecation warnings if anyone passes in an instruction pointer directly.

---

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

5 Files Affected:

• (modified) llvm/examples/IRTransforms/SimplifyCFG.cpp (+3-3)
• (modified) llvm/include/llvm/IR/InstrTypes.h (+16)
• (modified) llvm/include/llvm/IR/Instruction.h (+2-2)
• (modified) llvm/include/llvm/IR/Instructions.h (+1-1)
• (modified) llvm/lib/SandboxIR/SandboxIR.cpp (+1-1)

diff --git a/llvm/examples/IRTransforms/SimplifyCFG.cpp b/llvm/examples/IRTransforms/SimplifyCFG.cpp
index d6364385eb1ec9..7df646c64c7f19 100644
--- a/llvm/examples/IRTransforms/SimplifyCFG.cpp
+++ b/llvm/examples/IRTransforms/SimplifyCFG.cpp
@@ -158,7 +158,7 @@ static bool eliminateCondBranches_v1(Function &F) {
     // Replace the conditional branch with an unconditional one, by creating
     // a new unconditional branch to the selected successor and removing the
     // conditional one.
-    BranchInst::Create(BI->getSuccessor(CI->isZero()), BI);
+    BranchInst::Create(BI->getSuccessor(CI->isZero()), BI->getIterator());
     BI->eraseFromParent();
     Changed = true;
   }
@@ -195,7 +195,7 @@ static bool eliminateCondBranches_v2(Function &F, DominatorTree &DT) {
     // a new unconditional branch to the selected successor and removing the
     // conditional one.
     BranchInst *NewBranch =
-        BranchInst::Create(BI->getSuccessor(CI->isZero()), BI);
+        BranchInst::Create(BI->getSuccessor(CI->isZero()), BI->getIterator());
     BI->eraseFromParent();
 
     // Delete the edge between BB and RemovedSucc in the DominatorTree, iff
@@ -242,7 +242,7 @@ static bool eliminateCondBranches_v3(Function &F, DominatorTree &DT) {
     // a new unconditional branch to the selected successor and removing the
     // conditional one.
 
-    BranchInst *NewBranch = BranchInst::Create(TakenSucc, BB.getTerminator());
+    BranchInst *NewBranch = BranchInst::Create(TakenSucc, BB.getTerminator()->getIterator());
     BB.getTerminator()->eraseFromParent();
 
     // Delete the edge between BB and RemovedSucc in the DominatorTree, iff
diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index afae564bf022d2..638ac66537e73a 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -60,6 +60,8 @@ class UnaryInstruction : public Instruction {
       : Instruction(Ty, iType, &Op<0>(), 1, IB) {
     Op<0>() = V;
   }
+  LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead",
+    "BasicBlock::iterator")
   UnaryInstruction(Type *Ty, unsigned iType, Value *V,
                    Instruction *IB = nullptr)
     : Instruction(Ty, iType, &Op<0>(), 1, IB) {
@@ -140,6 +142,8 @@ class UnaryOperator : public UnaryInstruction {
   }
 #include "llvm/IR/Instruction.def"
 #define HANDLE_UNARY_INST(N, OPC, CLASS) \
+  LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead", \
+    "BasicBlock::iterator") \
   static UnaryOperator *Create##OPC(Value *V, const Twine &Name, \
                                     Instruction *I) {\
     return Create(Instruction::OPC, V, Name, I);\
@@ -230,6 +234,8 @@ class BinaryOperator : public Instruction {
   }
 #include "llvm/IR/Instruction.def"
 #define HANDLE_BINARY_INST(N, OPC, CLASS) \
+  LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead", \
+    "BasicBlock::iterator") \
   static BinaryOperator *Create##OPC(Value *V1, Value *V2, \
                                      const Twine &Name, Instruction *I) {\
     return Create(Instruction::OPC, V1, V2, Name, I);\
@@ -315,6 +321,8 @@ class BinaryOperator : public Instruction {
     BO->setHasNoSignedWrap(true);
     return BO;
   }
+  LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead",
+    "BasicBlock::iterator")
   static BinaryOperator *CreateNSW(BinaryOps Opc, Value *V1, Value *V2,
                                    const Twine &Name, Instruction *I) {
     BinaryOperator *BO = Create(Opc, V1, V2, Name, I);
@@ -340,6 +348,8 @@ class BinaryOperator : public Instruction {
     BO->setHasNoUnsignedWrap(true);
     return BO;
   }
+  LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead",
+    "BasicBlock::iterator")
   static BinaryOperator *CreateNUW(BinaryOps Opc, Value *V1, Value *V2,
                                    const Twine &Name, Instruction *I) {
     BinaryOperator *BO = Create(Opc, V1, V2, Name, I);
@@ -365,6 +375,8 @@ class BinaryOperator : public Instruction {
     BO->setIsExact(true);
     return BO;
   }
+  LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead",
+    "BasicBlock::iterator")
   static BinaryOperator *CreateExact(BinaryOps Opc, Value *V1, Value *V2,
                                      const Twine &Name, Instruction *I) {
     BinaryOperator *BO = Create(Opc, V1, V2, Name, I);
@@ -400,6 +412,8 @@ class BinaryOperator : public Instruction {
       Value *V1, Value *V2, const Twine &Name, BasicBlock *BB) {               \
     return Create##NUWNSWEXACT(Instruction::OPC, V1, V2, Name, BB);            \
   }                                                                            \
+  LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead",           \
+      "BasicBlock::iterator")                                                  \
   static BinaryOperator *Create##NUWNSWEXACT##OPC(                             \
       Value *V1, Value *V2, const Twine &Name, Instruction *I) {               \
     return Create##NUWNSWEXACT(Instruction::OPC, V1, V2, Name, I);             \
@@ -502,6 +516,8 @@ BinaryOperator *BinaryOperator::CreateDisjoint(BinaryOps Opc, Value *V1,
   cast<PossiblyDisjointInst>(BO)->setIsDisjoint(true);
   return BO;
 }
+LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead",
+"BasicBlock::iterator")
 BinaryOperator *BinaryOperator::CreateDisjoint(BinaryOps Opc, Value *V1,
                                                Value *V2, const Twine &Name,
                                                Instruction *I) {
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index c27572300d5063..569346f037b554 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -52,8 +52,8 @@ class InsertPosition {
 
 public:
   InsertPosition(std::nullptr_t) : InsertAt() {}
-  // LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead",
-  // "BasicBlock::iterator")
+  LLVM_DEPRECATED("Use BasicBlock::iterators for insertion instead",
+    "BasicBlock::iterator")
   InsertPosition(Instruction *InsertBefore);
   InsertPosition(BasicBlock *InsertAtEnd);
   InsertPosition(InstListType::iterator InsertAt) : InsertAt(InsertAt) {}
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index 968737a843e292..c3c156e2663456 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -2537,7 +2537,7 @@ class PHINode : public Instruction {
   /// edges that this phi node will have (use 0 if you really have no idea).
   static PHINode *Create(Type *Ty, unsigned NumReservedValues,
                          const Twine &NameStr = "",
-                         InsertPosition InsertBefore = nullptr) {
+                         InsertPosition InsertBefore = (BasicBlock*)nullptr) {
     return new PHINode(Ty, NumReservedValues, NameStr, InsertBefore);
   }
 
diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index 2cb76fc89d9b4d..e48f025ceaf820 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -1142,7 +1142,7 @@ PHINode *PHINode::create(Type *Ty, unsigned NumReservedValues,
                          Instruction *InsertBefore, Context &Ctx,
                          const Twine &Name) {
   llvm::PHINode *NewPHI = llvm::PHINode::Create(
-      Ty, NumReservedValues, Name, InsertBefore->getTopmostLLVMInstruction());
+      Ty, NumReservedValues, Name, InsertBefore->getTopmostLLVMInstruction()->getIterator());
   return Ctx.createPHINode(NewPHI);
 }
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[nikic]

Why is this necessary despite the nullptr_t overload on InsertPosition?

[jmorse]

Apparently it isn't (now removed). Not sure what I was doing before, but I thought I was getting a bunch of deprecated warnings out of here, which I now can't replicate.

[SLTozer]

Changes largely LGTM, but an inline comment about maybe changing some function signatures.

[SLTozer]

I think these (and the other cases in this file where you've added LLVM_DEPRECATED) could be replaced with InsertPosition arguments - especially the HANDLE_ cases, where we have 3 different methods for each type, which all forward to the same InsertPosition fn underneath. Not sure why I missed these the first time, but (as long as it works) it should probably be done now.

[jmorse]

Thanks for the insert-position tips, I've consolidated all of those into InsertPosition object arguments.