[clang] [DebugInfo][RemoveDIs] Use iterator-inserters in clang (PR #102006)

Jeremy Morse via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 8 09:49:53 PDT 2024


https://github.com/jmorse updated https://github.com/llvm/llvm-project/pull/102006

>From cf967317327aa3971da62a15f357ee5b29268f14 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Mon, 5 Aug 2024 16:21:53 +0100
Subject: [PATCH 1/3] [DebugInfo][RemoveDIs] Use iterator-inserters in clang

As part of the LLVM effort to eliminate debug-info intrinsics, we're moving
to a world where only iterators should be used to insert instructions. This
isn't a problem in clang when instructions get generated before any
debug-info is inserted, however we're planning on deprecating and removing
the instruction-pointer insertion routines.

Scatter some calls to getIterator in a few places, remove a
deref-then-addrof on another iterator, and add an overload for the
createLoadInstBefore utility. Some callers passes a null insertion point,
which we need to handle explicitly now.
---
 clang/lib/CodeGen/CGCUDANV.cpp      |  6 +++---
 clang/lib/CodeGen/CGCall.cpp        |  4 ++--
 clang/lib/CodeGen/CGCleanup.cpp     | 20 ++++++++++++++------
 clang/lib/CodeGen/CGCoroutine.cpp   |  5 +++--
 clang/lib/CodeGen/CGExpr.cpp        |  2 +-
 clang/lib/CodeGen/CGObjC.cpp        |  3 ++-
 clang/lib/CodeGen/CGStmt.cpp        |  2 +-
 clang/lib/CodeGen/CodeGenFunction.h |  4 ++--
 clang/lib/CodeGen/CodeGenModule.cpp | 10 +++++-----
 9 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/clang/lib/CodeGen/CGCUDANV.cpp b/clang/lib/CodeGen/CGCUDANV.cpp
index 43dfbbb90dd52..1f5fb630a4703 100644
--- a/clang/lib/CodeGen/CGCUDANV.cpp
+++ b/clang/lib/CodeGen/CGCUDANV.cpp
@@ -505,9 +505,9 @@ static void replaceManagedVar(llvm::GlobalVariable *Var,
     }
     if (auto *I = dyn_cast<llvm::Instruction>(U)) {
       llvm::Value *OldV = Var;
-      llvm::Instruction *NewV =
-          new llvm::LoadInst(Var->getType(), ManagedVar, "ld.managed", false,
-                             llvm::Align(Var->getAlignment()), I);
+      llvm::Instruction *NewV = new llvm::LoadInst(
+          Var->getType(), ManagedVar, "ld.managed", false,
+          llvm::Align(Var->getAlignment()), I->getIterator());
       WorkItem.pop_back();
       // Replace constant expressions directly or indirectly using the managed
       // variable with instructions.
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 2f3dd5d01fa6c..437aa7bc67dc7 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5081,8 +5081,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
     llvm::AllocaInst *AI;
     if (IP) {
       IP = IP->getNextNode();
-      AI = new llvm::AllocaInst(ArgStruct, DL.getAllocaAddrSpace(),
-                                "argmem", IP);
+      AI = new llvm::AllocaInst(ArgStruct, DL.getAllocaAddrSpace(), "argmem",
+                                IP->getIterator());
     } else {
       AI = CreateTempAlloca(ArgStruct, "argmem");
     }
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index 4e210a9e3c95f..ba0632de31473 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -295,18 +295,25 @@ void EHScopeStack::Cleanup::anchor() {}
 static void createStoreInstBefore(llvm::Value *value, Address addr,
                                   llvm::Instruction *beforeInst,
                                   CodeGenFunction &CGF) {
-  auto store = new llvm::StoreInst(value, addr.emitRawPointer(CGF), beforeInst);
+  auto store = new llvm::StoreInst(value, addr.emitRawPointer(CGF),
+                                   beforeInst->getIterator());
   store->setAlignment(addr.getAlignment().getAsAlign());
 }
 
 static llvm::LoadInst *createLoadInstBefore(Address addr, const Twine &name,
-                                            llvm::Instruction *beforeInst,
+                                            llvm::BasicBlock::iterator beforeInst,
                                             CodeGenFunction &CGF) {
   return new llvm::LoadInst(addr.getElementType(), addr.emitRawPointer(CGF),
                             name, false, addr.getAlignment().getAsAlign(),
                             beforeInst);
 }
 
+static llvm::LoadInst *createLoadInstBefore(Address addr, const Twine &name,
+                                            CodeGenFunction &CGF) {
+  return new llvm::LoadInst(addr.getElementType(), addr.emitRawPointer(CGF),
+                            name, false, addr.getAlignment().getAsAlign());
+}
+
 /// All the branch fixups on the EH stack have propagated out past the
 /// outermost normal cleanup; resolve them all by adding cases to the
 /// given switch instruction.
@@ -358,7 +365,7 @@ static llvm::SwitchInst *TransitionToCleanupSwitch(CodeGenFunction &CGF,
   if (llvm::BranchInst *Br = dyn_cast<llvm::BranchInst>(Term)) {
     assert(Br->isUnconditional());
     auto Load = createLoadInstBefore(CGF.getNormalCleanupDestSlot(),
-                                     "cleanup.dest", Term, CGF);
+                                     "cleanup.dest", Term->getIterator(), CGF);
     llvm::SwitchInst *Switch =
       llvm::SwitchInst::Create(Load, Br->getSuccessor(0), 4, Block);
     Br->eraseFromParent();
@@ -612,7 +619,8 @@ static void destroyOptimisticNormalEntry(CodeGenFunction &CGF,
     llvm::SwitchInst *si = cast<llvm::SwitchInst>(use.getUser());
     if (si->getNumCases() == 1 && si->getDefaultDest() == unreachableBB) {
       // Replace the switch with a branch.
-      llvm::BranchInst::Create(si->case_begin()->getCaseSuccessor(), si);
+      llvm::BranchInst::Create(si->case_begin()->getCaseSuccessor(),
+                               si->getIterator());
 
       // The switch operand is a load from the cleanup-dest alloca.
       llvm::LoadInst *condition = cast<llvm::LoadInst>(si->getCondition());
@@ -908,8 +916,8 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
         // pass the abnormal exit flag to Fn (SEH cleanup)
         cleanupFlags.setHasExitSwitch();
 
-        llvm::LoadInst *Load = createLoadInstBefore(
-            getNormalCleanupDestSlot(), "cleanup.dest", nullptr, *this);
+        llvm::LoadInst *Load = createLoadInstBefore(getNormalCleanupDestSlot(),
+                                                    "cleanup.dest", *this);
         llvm::SwitchInst *Switch =
           llvm::SwitchInst::Create(Load, Default, SwitchCapacity);
 
diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index a8a70186c2c5a..0c09ff96f9d6b 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -866,8 +866,9 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) {
     EmitStmt(S.getPromiseDeclStmt());
 
     Address PromiseAddr = GetAddrOfLocalVar(S.getPromiseDecl());
-    auto *PromiseAddrVoidPtr = new llvm::BitCastInst(
-        PromiseAddr.emitRawPointer(*this), VoidPtrTy, "", CoroId);
+    auto *PromiseAddrVoidPtr =
+        new llvm::BitCastInst(PromiseAddr.emitRawPointer(*this), VoidPtrTy, "",
+                              CoroId->getIterator());
     // Update CoroId to refer to the promise. We could not do it earlier because
     // promise local variable was not emitted yet.
     CoroId->setArgOperand(1, PromiseAddrVoidPtr);
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5f58a64d8386c..7f7ef5e64a0e5 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -121,7 +121,7 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
     Alloca = Builder.CreateAlloca(Ty, ArraySize, Name);
   else
     Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
-                                  ArraySize, Name, &*AllocaInsertPt);
+                                  ArraySize, Name, AllocaInsertPt);
   if (Allocas) {
     Allocas->Add(Alloca);
   }
diff --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index 80a64d8e4cdd9..a73a71f2d2688 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/lib/CodeGen/CGObjC.cpp
@@ -2393,7 +2393,8 @@ static llvm::Value *emitOptimizedARCReturnCall(llvm::Value *value,
     llvm::OperandBundleDef OB("clang.arc.attachedcall", bundleArgs);
     auto *oldCall = cast<llvm::CallBase>(value);
     llvm::CallBase *newCall = llvm::CallBase::addOperandBundle(
-        oldCall, llvm::LLVMContext::OB_clang_arc_attachedcall, OB, oldCall);
+        oldCall, llvm::LLVMContext::OB_clang_arc_attachedcall, OB,
+        oldCall->getIterator());
     newCall->copyMetadata(*oldCall);
     oldCall->replaceAllUsesWith(newCall);
     oldCall->eraseFromParent();
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index e16aa3cdd5506..87cd323bbd78c 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -3226,7 +3226,7 @@ CodeGenFunction::addConvergenceControlToken(llvm::CallBase *Input,
   llvm::Value *bundleArgs[] = {ParentToken};
   llvm::OperandBundleDef OB("convergencectrl", bundleArgs);
   auto Output = llvm::CallBase::addOperandBundle(
-      Input, llvm::LLVMContext::OB_convergencectrl, OB, Input);
+      Input, llvm::LLVMContext::OB_convergencectrl, OB, Input->getIterator());
   Input->replaceAllUsesWith(Output);
   Input->eraseFromParent();
   return Output;
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 89cc819c43bb5..2a2b4dd733b74 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1310,8 +1310,8 @@ class CodeGenFunction : public CodeGenTypeCache {
                                      CodeGenFunction &CGF) {
     assert(isInConditionalBranch());
     llvm::BasicBlock *block = OutermostConditional->getStartingBlock();
-    auto store =
-        new llvm::StoreInst(value, addr.emitRawPointer(CGF), &block->back());
+    auto store = new llvm::StoreInst(value, addr.emitRawPointer(CGF),
+                                     block->back().getIterator());
     store->setAlignment(addr.getAlignment().getAsAlign());
   }
 
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 8057812f80311..5e220a47d7ce0 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5893,13 +5893,13 @@ static void replaceUsesOfNonProtoConstant(llvm::Constant *old,
 
     llvm::CallBase *newCall;
     if (isa<llvm::CallInst>(callSite)) {
-      newCall =
-          llvm::CallInst::Create(newFn, newArgs, newBundles, "", callSite);
+      newCall = llvm::CallInst::Create(newFn, newArgs, newBundles, "",
+                                       callSite->getIterator());
     } else {
       auto *oldInvoke = cast<llvm::InvokeInst>(callSite);
-      newCall = llvm::InvokeInst::Create(newFn, oldInvoke->getNormalDest(),
-                                         oldInvoke->getUnwindDest(), newArgs,
-                                         newBundles, "", callSite);
+      newCall = llvm::InvokeInst::Create(
+          newFn, oldInvoke->getNormalDest(), oldInvoke->getUnwindDest(),
+          newArgs, newBundles, "", callSite->getIterator());
     }
     newArgs.clear(); // for the next iteration
 

>From f652bb1d835f506a942ae8bc154d22b4da4545ec Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Thu, 8 Aug 2024 17:31:57 +0100
Subject: [PATCH 2/3] Implement some review feedback (see reviews),

Also fix a stupid build failure where I'd assumed a &* pattern on a
variable was dereferencing-addrofing an iterator. It's actually a
ValueHandle, so we have to call getIterator directly.
---
 clang/lib/CodeGen/CGCleanup.cpp | 12 ++++++------
 clang/lib/CodeGen/CGExpr.cpp    |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index ba0632de31473..1556aed7acfaf 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -293,10 +293,10 @@ void CodeGenFunction::initFullExprCleanupWithFlag(RawAddress ActiveFlag) {
 void EHScopeStack::Cleanup::anchor() {}
 
 static void createStoreInstBefore(llvm::Value *value, Address addr,
-                                  llvm::Instruction *beforeInst,
+                                  llvm::BasicBlock::iterator beforeInst,
                                   CodeGenFunction &CGF) {
   auto store = new llvm::StoreInst(value, addr.emitRawPointer(CGF),
-                                   beforeInst->getIterator());
+                                   beforeInst);
   store->setAlignment(addr.getAlignment().getAsAlign());
 }
 
@@ -337,7 +337,7 @@ static void ResolveAllBranchFixups(CodeGenFunction &CGF,
     // entry which we're currently popping.
     if (Fixup.OptimisticBranchBlock == nullptr) {
       createStoreInstBefore(CGF.Builder.getInt32(Fixup.DestinationIndex),
-                            CGF.getNormalCleanupDestSlot(), Fixup.InitialBranch,
+                            CGF.getNormalCleanupDestSlot(), Fixup.InitialBranch->getIterator(),
                             CGF);
       Fixup.InitialBranch->setSuccessor(0, CleanupEntry);
     }
@@ -965,7 +965,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
         if (!Fixup.Destination) continue;
         if (!Fixup.OptimisticBranchBlock) {
           createStoreInstBefore(Builder.getInt32(Fixup.DestinationIndex),
-                                getNormalCleanupDestSlot(), Fixup.InitialBranch,
+                                getNormalCleanupDestSlot(), Fixup.InitialBranch->getIterator(),
                                 *this);
           Fixup.InitialBranch->setSuccessor(0, NormalEntry);
         }
@@ -1141,7 +1141,7 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) {
 
   // Store the index at the start.
   llvm::ConstantInt *Index = Builder.getInt32(Dest.getDestIndex());
-  createStoreInstBefore(Index, getNormalCleanupDestSlot(), BI, *this);
+  createStoreInstBefore(Index, getNormalCleanupDestSlot(), BI->getIterator(), *this);
 
   // Adjust BI to point to the first cleanup block.
   {
@@ -1260,7 +1260,7 @@ static void SetupCleanupBlockActivation(CodeGenFunction &CGF,
     if (CGF.isInConditionalBranch()) {
       CGF.setBeforeOutermostConditional(value, var, CGF);
     } else {
-      createStoreInstBefore(value, var, dominatingIP, CGF);
+      createStoreInstBefore(value, var, dominatingIP->getIterator(), CGF);
     }
   }
 
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 7f7ef5e64a0e5..bb866a62ebad7 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -121,7 +121,7 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
     Alloca = Builder.CreateAlloca(Ty, ArraySize, Name);
   else
     Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
-                                  ArraySize, Name, AllocaInsertPt);
+                                  ArraySize, Name, AllocaInsertPt->getIterator());
   if (Allocas) {
     Allocas->Add(Alloca);
   }

>From aa220377ca3c62a65cd16cc83519e44b2a3b3e72 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Thu, 8 Aug 2024 17:48:49 +0100
Subject: [PATCH 3/3] clang-format

---
 clang/lib/CodeGen/CGCleanup.cpp | 17 +++++++++--------
 clang/lib/CodeGen/CGExpr.cpp    |  5 +++--
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index 1556aed7acfaf..b1af4a0a97884 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -295,8 +295,7 @@ void EHScopeStack::Cleanup::anchor() {}
 static void createStoreInstBefore(llvm::Value *value, Address addr,
                                   llvm::BasicBlock::iterator beforeInst,
                                   CodeGenFunction &CGF) {
-  auto store = new llvm::StoreInst(value, addr.emitRawPointer(CGF),
-                                   beforeInst);
+  auto store = new llvm::StoreInst(value, addr.emitRawPointer(CGF), beforeInst);
   store->setAlignment(addr.getAlignment().getAsAlign());
 }
 
@@ -337,8 +336,8 @@ static void ResolveAllBranchFixups(CodeGenFunction &CGF,
     // entry which we're currently popping.
     if (Fixup.OptimisticBranchBlock == nullptr) {
       createStoreInstBefore(CGF.Builder.getInt32(Fixup.DestinationIndex),
-                            CGF.getNormalCleanupDestSlot(), Fixup.InitialBranch->getIterator(),
-                            CGF);
+                            CGF.getNormalCleanupDestSlot(),
+                            Fixup.InitialBranch->getIterator(), CGF);
       Fixup.InitialBranch->setSuccessor(0, CleanupEntry);
     }
 
@@ -962,11 +961,12 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
       for (unsigned I = FixupDepth, E = EHStack.getNumBranchFixups();
            I < E; ++I) {
         BranchFixup &Fixup = EHStack.getBranchFixup(I);
-        if (!Fixup.Destination) continue;
+        if (!Fixup.Destination)
+          continue;
         if (!Fixup.OptimisticBranchBlock) {
           createStoreInstBefore(Builder.getInt32(Fixup.DestinationIndex),
-                                getNormalCleanupDestSlot(), Fixup.InitialBranch->getIterator(),
-                                *this);
+                                getNormalCleanupDestSlot(),
+                                Fixup.InitialBranch->getIterator(), *this);
           Fixup.InitialBranch->setSuccessor(0, NormalEntry);
         }
         Fixup.OptimisticBranchBlock = NormalExit;
@@ -1141,7 +1141,8 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) {
 
   // Store the index at the start.
   llvm::ConstantInt *Index = Builder.getInt32(Dest.getDestIndex());
-  createStoreInstBefore(Index, getNormalCleanupDestSlot(), BI->getIterator(), *this);
+  createStoreInstBefore(Index, getNormalCleanupDestSlot(), BI->getIterator(),
+                        *this);
 
   // Adjust BI to point to the first cleanup block.
   {
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index bb866a62ebad7..5642ce5611e9d 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -120,8 +120,9 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
   if (ArraySize)
     Alloca = Builder.CreateAlloca(Ty, ArraySize, Name);
   else
-    Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
-                                  ArraySize, Name, AllocaInsertPt->getIterator());
+    Alloca =
+        new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
+                             ArraySize, Name, AllocaInsertPt->getIterator());
   if (Allocas) {
     Allocas->Add(Alloca);
   }



More information about the cfe-commits mailing list