[llvm] [SPIR-V]: Memory leak fix in SPIRVEmitIntrinsics (PR #83015)

via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 5 03:59:48 PST 2024


https://github.com/bwlodarcz updated https://github.com/llvm/llvm-project/pull/83015

>From 40ff1a741b0984606fed6536815b49b12f2bbcbb Mon Sep 17 00:00:00 2001
From: "Wlodarczyk, Bertrand" <bertrand.wlodarczyk at intel.com>
Date: Tue, 5 Mar 2024 12:27:18 +0100
Subject: [PATCH 1/2] [SPIR-V]: Memory leak fix in SPIRVEmitIntrinsics

The architecture of SPIRVEmitIntrinsics is build in
such way that every private method is called by one
main function runOnFunction which then calls private methods.
Private member IRB is allocated in runOnFunction method but it's not freed.
Due to that every time when IR function contains intrinsics to emit,
runOnFunction is entered and memory is leaked on exit.
It's especially true when there are two or more IR functions to emit.
IRB is set to nullptr during construction of object
and it's left without pointing resource until runOnFunction is entered.
This also create possibility of simple mistake when
private method is called but there is no resource pointed.
Change requires passing IRBuilder by reference to private methods.
The visit* functions create it's own IRBuilder instance.
In addition there is a small performance improvement
because IRBuilder is not allocated by heap.
---
 llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 266 ++++++++++--------
 1 file changed, 149 insertions(+), 117 deletions(-)

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index c83537bc7ae8a5..dfdfae551dccf9 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -52,16 +52,15 @@ class SPIRVEmitIntrinsics
     : public FunctionPass,
       public InstVisitor<SPIRVEmitIntrinsics, Instruction *> {
   SPIRVTargetMachine *TM = nullptr;
-  IRBuilder<> *IRB = nullptr;
   Function *F = nullptr;
   bool TrackConstants = true;
   DenseMap<Instruction *, Constant *> AggrConsts;
   DenseSet<Instruction *> AggrStores;
-  void preprocessCompositeConstants();
-  void preprocessUndefs();
+  void preprocessCompositeConstants(IRBuilder<> &B);
+  void preprocessUndefs(IRBuilder<> &B);
   CallInst *buildIntrWithMD(Intrinsic::ID IntrID, ArrayRef<Type *> Types,
-                            Value *Arg, Value *Arg2,
-                            ArrayRef<Constant *> Imms) {
+                            Value *Arg, Value *Arg2, ArrayRef<Constant *> Imms,
+                            IRBuilder<> &B) {
     ConstantAsMetadata *CM = ValueAsMetadata::getConstant(Arg);
     MDTuple *TyMD = MDNode::get(F->getContext(), CM);
     MetadataAsValue *VMD = MetadataAsValue::get(F->getContext(), TyMD);
@@ -70,19 +69,20 @@ class SPIRVEmitIntrinsics
     Args.push_back(VMD);
     for (auto *Imm : Imms)
       Args.push_back(Imm);
-    return IRB->CreateIntrinsic(IntrID, {Types}, Args);
+    return B.CreateIntrinsic(IntrID, {Types}, Args);
   }
-  void replaceMemInstrUses(Instruction *Old, Instruction *New);
-  void processInstrAfterVisit(Instruction *I);
-  void insertAssignPtrTypeIntrs(Instruction *I);
-  void insertAssignTypeIntrs(Instruction *I);
+  void replaceMemInstrUses(Instruction *Old, Instruction *New, IRBuilder<> &B);
+  void processInstrAfterVisit(Instruction *I, IRBuilder<> &B);
+  void insertAssignPtrTypeIntrs(Instruction *I, IRBuilder<> &B);
+  void insertAssignTypeIntrs(Instruction *I, IRBuilder<> &B);
   void insertAssignTypeInstrForTargetExtTypes(TargetExtType *AssignedType,
-                                              Value *V);
+                                              Value *V, IRBuilder<> &B);
   void replacePointerOperandWithPtrCast(Instruction *I, Value *Pointer,
                                         Type *ExpectedElementType,
-                                        unsigned OperandToReplace);
-  void insertPtrCastOrAssignTypeInstr(Instruction *I);
-  void processGlobalValue(GlobalVariable &GV);
+                                        unsigned OperandToReplace,
+                                        IRBuilder<> &B);
+  void insertPtrCastOrAssignTypeInstr(Instruction *I, IRBuilder<> &B);
+  void processGlobalValue(GlobalVariable &GV, IRBuilder<> &B);
 
 public:
   static char ID;
@@ -156,13 +156,14 @@ static inline void reportFatalOnTokenType(const Instruction *I) {
 }
 
 void SPIRVEmitIntrinsics::replaceMemInstrUses(Instruction *Old,
-                                              Instruction *New) {
+                                              Instruction *New,
+                                              IRBuilder<> &B) {
   while (!Old->user_empty()) {
     auto *U = Old->user_back();
     if (isAssignTypeInstr(U)) {
-      IRB->SetInsertPoint(U);
+      B.SetInsertPoint(U);
       SmallVector<Value *, 2> Args = {New, U->getOperand(1)};
-      IRB->CreateIntrinsic(Intrinsic::spv_assign_type, {New->getType()}, Args);
+      B.CreateIntrinsic(Intrinsic::spv_assign_type, {New->getType()}, Args);
       U->eraseFromParent();
     } else if (isMemInstrToReplace(U) || isa<ReturnInst>(U) ||
                isa<CallInst>(U)) {
@@ -174,7 +175,7 @@ void SPIRVEmitIntrinsics::replaceMemInstrUses(Instruction *Old,
   Old->eraseFromParent();
 }
 
-void SPIRVEmitIntrinsics::preprocessUndefs() {
+void SPIRVEmitIntrinsics::preprocessUndefs(IRBuilder<> &B) {
   std::queue<Instruction *> Worklist;
   for (auto &I : instructions(F))
     Worklist.push(&I);
@@ -188,8 +189,8 @@ void SPIRVEmitIntrinsics::preprocessUndefs() {
       if (!AggrUndef || !Op->getType()->isAggregateType())
         continue;
 
-      IRB->SetInsertPoint(I);
-      auto *IntrUndef = IRB->CreateIntrinsic(Intrinsic::spv_undef, {}, {});
+      B.SetInsertPoint(I);
+      auto *IntrUndef = B.CreateIntrinsic(Intrinsic::spv_undef, {}, {});
       Worklist.push(IntrUndef);
       I->replaceUsesOfWith(Op, IntrUndef);
       AggrConsts[IntrUndef] = AggrUndef;
@@ -197,7 +198,7 @@ void SPIRVEmitIntrinsics::preprocessUndefs() {
   }
 }
 
-void SPIRVEmitIntrinsics::preprocessCompositeConstants() {
+void SPIRVEmitIntrinsics::preprocessCompositeConstants(IRBuilder<> &B) {
   std::queue<Instruction *> Worklist;
   for (auto &I : instructions(F))
     Worklist.push(&I);
@@ -207,31 +208,35 @@ void SPIRVEmitIntrinsics::preprocessCompositeConstants() {
     assert(I);
     bool KeepInst = false;
     for (const auto &Op : I->operands()) {
-      auto BuildCompositeIntrinsic = [&KeepInst, &Worklist, &I, &Op,
-                                      this](Constant *AggrC,
-                                            ArrayRef<Value *> Args) {
-        IRB->SetInsertPoint(I);
-        auto *CCI =
-            IRB->CreateIntrinsic(Intrinsic::spv_const_composite, {}, {Args});
-        Worklist.push(CCI);
-        I->replaceUsesOfWith(Op, CCI);
-        KeepInst = true;
-        AggrConsts[CCI] = AggrC;
-      };
+      auto BuildCompositeIntrinsic =
+          [](Constant *AggrC, ArrayRef<Value *> Args, Value *Op, Instruction *I,
+             IRBuilder<> &B, std::queue<Instruction *> &Worklist,
+             bool &KeepInst, SPIRVEmitIntrinsics &SEI) {
+            B.SetInsertPoint(I);
+            auto *CCI =
+                B.CreateIntrinsic(Intrinsic::spv_const_composite, {}, {Args});
+            Worklist.push(CCI);
+            I->replaceUsesOfWith(Op, CCI);
+            KeepInst = true;
+            SEI.AggrConsts[CCI] = AggrC;
+          };
 
       if (auto *AggrC = dyn_cast<ConstantAggregate>(Op)) {
         SmallVector<Value *> Args(AggrC->op_begin(), AggrC->op_end());
-        BuildCompositeIntrinsic(AggrC, Args);
+        BuildCompositeIntrinsic(AggrC, Args, Op, I, B, Worklist, KeepInst,
+                                *this);
       } else if (auto *AggrC = dyn_cast<ConstantDataArray>(Op)) {
         SmallVector<Value *> Args;
         for (unsigned i = 0; i < AggrC->getNumElements(); ++i)
           Args.push_back(AggrC->getElementAsConstant(i));
-        BuildCompositeIntrinsic(AggrC, Args);
+        BuildCompositeIntrinsic(AggrC, Args, Op, I, B, Worklist, KeepInst,
+                                *this);
       } else if (isa<ConstantAggregateZero>(Op) &&
                  !Op->getType()->isVectorTy()) {
         auto *AggrC = cast<ConstantAggregateZero>(Op);
         SmallVector<Value *> Args(AggrC->op_begin(), AggrC->op_end());
-        BuildCompositeIntrinsic(AggrC, Args);
+        BuildCompositeIntrinsic(AggrC, Args, Op, I, B, Worklist, KeepInst,
+                                *this);
       }
     }
     if (!KeepInst)
@@ -240,29 +245,34 @@ void SPIRVEmitIntrinsics::preprocessCompositeConstants() {
 }
 
 Instruction *SPIRVEmitIntrinsics::visitSwitchInst(SwitchInst &I) {
+  IRBuilder<> B(I.getParent());
   SmallVector<Value *, 4> Args;
   for (auto &Op : I.operands())
     if (Op.get()->getType()->isSized())
       Args.push_back(Op);
-  IRB->SetInsertPoint(&I);
-  IRB->CreateIntrinsic(Intrinsic::spv_switch, {I.getOperand(0)->getType()},
+  B.SetInsertPoint(&I);
+  B.CreateIntrinsic(Intrinsic::spv_switch, {I.getOperand(0)->getType()},
                        {Args});
   return &I;
 }
 
 Instruction *SPIRVEmitIntrinsics::visitGetElementPtrInst(GetElementPtrInst &I) {
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   SmallVector<Type *, 2> Types = {I.getType(), I.getOperand(0)->getType()};
   SmallVector<Value *, 4> Args;
-  Args.push_back(IRB->getInt1(I.isInBounds()));
+  Args.push_back(B.getInt1(I.isInBounds()));
   for (auto &Op : I.operands())
     Args.push_back(Op);
-  auto *NewI = IRB->CreateIntrinsic(Intrinsic::spv_gep, {Types}, {Args});
+  auto *NewI = B.CreateIntrinsic(Intrinsic::spv_gep, {Types}, {Args});
   I.replaceAllUsesWith(NewI);
   I.eraseFromParent();
   return NewI;
 }
 
 Instruction *SPIRVEmitIntrinsics::visitBitCastInst(BitCastInst &I) {
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   Value *Source = I.getOperand(0);
 
   // SPIR-V, contrary to LLVM 17+ IR, supports bitcasts between pointers of
@@ -277,7 +287,7 @@ Instruction *SPIRVEmitIntrinsics::visitBitCastInst(BitCastInst &I) {
 
   SmallVector<Type *, 2> Types = {I.getType(), Source->getType()};
   SmallVector<Value *> Args(I.op_begin(), I.op_end());
-  auto *NewI = IRB->CreateIntrinsic(Intrinsic::spv_bitcast, {Types}, {Args});
+  auto *NewI = B.CreateIntrinsic(Intrinsic::spv_bitcast, {Types}, {Args});
   std::string InstName = I.hasName() ? I.getName().str() : "";
   I.replaceAllUsesWith(NewI);
   I.eraseFromParent();
@@ -286,7 +296,7 @@ Instruction *SPIRVEmitIntrinsics::visitBitCastInst(BitCastInst &I) {
 }
 
 void SPIRVEmitIntrinsics::insertAssignTypeInstrForTargetExtTypes(
-    TargetExtType *AssignedType, Value *V) {
+    TargetExtType *AssignedType, Value *V, IRBuilder<> &B) {
   // Do not emit spv_assign_type if the V is of the AssignedType already.
   if (V->getType() == AssignedType)
     return;
@@ -311,12 +321,12 @@ void SPIRVEmitIntrinsics::insertAssignTypeInstrForTargetExtTypes(
   }
 
   Constant *Const = UndefValue::get(AssignedType);
-  buildIntrWithMD(Intrinsic::spv_assign_type, {V->getType()}, Const, V, {});
+  buildIntrWithMD(Intrinsic::spv_assign_type, {V->getType()}, Const, V, {}, B);
 }
 
 void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
     Instruction *I, Value *Pointer, Type *ExpectedElementType,
-    unsigned OperandToReplace) {
+    unsigned OperandToReplace, IRBuilder<> &B) {
   // If Pointer is the result of nop BitCastInst (ptr -> ptr), use the source
   // pointer instead. The BitCastInst should be later removed when visited.
   while (BitCastInst *BC = dyn_cast<BitCastInst>(Pointer))
@@ -338,7 +348,7 @@ void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
   if (GEPI && GEPI->getResultElementType() == ExpectedElementType)
     return;
 
-  setInsertPointSkippingPhis(*IRB, I);
+  setInsertPointSkippingPhis(B, I);
   Constant *ExpectedElementTypeConst =
       Constant::getNullValue(ExpectedElementType);
   ConstantAsMetadata *CM =
@@ -392,34 +402,36 @@ void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
       (isa<Instruction>(Pointer) || isa<Argument>(Pointer))) {
     buildIntrWithMD(Intrinsic::spv_assign_ptr_type, {Pointer->getType()},
                     ExpectedElementTypeConst, Pointer,
-                    {IRB->getInt32(AddressSpace)});
+                    {B.getInt32(AddressSpace)}, B);
     return;
   }
 
   // Emit spv_ptrcast
   SmallVector<Type *, 2> Types = {Pointer->getType(), Pointer->getType()};
-  SmallVector<Value *, 2> Args = {Pointer, VMD, IRB->getInt32(AddressSpace)};
-  auto *PtrCastI = IRB->CreateIntrinsic(Intrinsic::spv_ptrcast, {Types}, Args);
+  SmallVector<Value *, 2> Args = {Pointer, VMD, B.getInt32(AddressSpace)};
+  auto *PtrCastI = B.CreateIntrinsic(Intrinsic::spv_ptrcast, {Types}, Args);
   I->setOperand(OperandToReplace, PtrCastI);
 }
 
-void SPIRVEmitIntrinsics::insertPtrCastOrAssignTypeInstr(Instruction *I) {
+void SPIRVEmitIntrinsics::insertPtrCastOrAssignTypeInstr(Instruction *I,
+                                                         IRBuilder<> &B) {
   // Handle basic instructions:
   StoreInst *SI = dyn_cast<StoreInst>(I);
   if (SI && F->getCallingConv() == CallingConv::SPIR_KERNEL &&
       SI->getValueOperand()->getType()->isPointerTy() &&
       isa<Argument>(SI->getValueOperand())) {
     return replacePointerOperandWithPtrCast(
-        I, SI->getValueOperand(), IntegerType::getInt8Ty(F->getContext()), 0);
+        I, SI->getValueOperand(), IntegerType::getInt8Ty(F->getContext()), 0,
+        B);
   } else if (SI) {
     return replacePointerOperandWithPtrCast(
-        I, SI->getPointerOperand(), SI->getValueOperand()->getType(), 1);
+        I, SI->getPointerOperand(), SI->getValueOperand()->getType(), 1, B);
   } else if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
     return replacePointerOperandWithPtrCast(I, LI->getPointerOperand(),
-                                            LI->getType(), 0);
+                                            LI->getType(), 0, B);
   } else if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(I)) {
     return replacePointerOperandWithPtrCast(I, GEPI->getPointerOperand(),
-                                            GEPI->getSourceElementType(), 0);
+                                            GEPI->getSourceElementType(), 0, B);
   }
 
   // Handle calls to builtins (non-intrinsics):
@@ -448,9 +460,9 @@ void SPIRVEmitIntrinsics::insertPtrCastOrAssignTypeInstr(Instruction *I) {
 
     if (ExpectedType->isTargetExtTy())
       insertAssignTypeInstrForTargetExtTypes(cast<TargetExtType>(ExpectedType),
-                                             ArgOperand);
+                                             ArgOperand, B);
     else
-      replacePointerOperandWithPtrCast(CI, ArgOperand, ExpectedType, OpIdx);
+      replacePointerOperandWithPtrCast(CI, ArgOperand, ExpectedType, OpIdx, B);
   }
 }
 
@@ -458,8 +470,10 @@ Instruction *SPIRVEmitIntrinsics::visitInsertElementInst(InsertElementInst &I) {
   SmallVector<Type *, 4> Types = {I.getType(), I.getOperand(0)->getType(),
                                   I.getOperand(1)->getType(),
                                   I.getOperand(2)->getType()};
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   SmallVector<Value *> Args(I.op_begin(), I.op_end());
-  auto *NewI = IRB->CreateIntrinsic(Intrinsic::spv_insertelt, {Types}, {Args});
+  auto *NewI = B.CreateIntrinsic(Intrinsic::spv_insertelt, {Types}, {Args});
   std::string InstName = I.hasName() ? I.getName().str() : "";
   I.replaceAllUsesWith(NewI);
   I.eraseFromParent();
@@ -469,10 +483,12 @@ Instruction *SPIRVEmitIntrinsics::visitInsertElementInst(InsertElementInst &I) {
 
 Instruction *
 SPIRVEmitIntrinsics::visitExtractElementInst(ExtractElementInst &I) {
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   SmallVector<Type *, 3> Types = {I.getType(), I.getVectorOperandType(),
                                   I.getIndexOperand()->getType()};
   SmallVector<Value *, 2> Args = {I.getVectorOperand(), I.getIndexOperand()};
-  auto *NewI = IRB->CreateIntrinsic(Intrinsic::spv_extractelt, {Types}, {Args});
+  auto *NewI = B.CreateIntrinsic(Intrinsic::spv_extractelt, {Types}, {Args});
   std::string InstName = I.hasName() ? I.getName().str() : "";
   I.replaceAllUsesWith(NewI);
   I.eraseFromParent();
@@ -481,29 +497,33 @@ SPIRVEmitIntrinsics::visitExtractElementInst(ExtractElementInst &I) {
 }
 
 Instruction *SPIRVEmitIntrinsics::visitInsertValueInst(InsertValueInst &I) {
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   SmallVector<Type *, 1> Types = {I.getInsertedValueOperand()->getType()};
   SmallVector<Value *> Args;
   for (auto &Op : I.operands())
     if (isa<UndefValue>(Op))
-      Args.push_back(UndefValue::get(IRB->getInt32Ty()));
+      Args.push_back(UndefValue::get(B.getInt32Ty()));
     else
       Args.push_back(Op);
   for (auto &Op : I.indices())
-    Args.push_back(IRB->getInt32(Op));
+    Args.push_back(B.getInt32(Op));
   Instruction *NewI =
-      IRB->CreateIntrinsic(Intrinsic::spv_insertv, {Types}, {Args});
-  replaceMemInstrUses(&I, NewI);
+      B.CreateIntrinsic(Intrinsic::spv_insertv, {Types}, {Args});
+  replaceMemInstrUses(&I, NewI, B);
   return NewI;
 }
 
 Instruction *SPIRVEmitIntrinsics::visitExtractValueInst(ExtractValueInst &I) {
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   SmallVector<Value *> Args;
   for (auto &Op : I.operands())
     Args.push_back(Op);
   for (auto &Op : I.indices())
-    Args.push_back(IRB->getInt32(Op));
+    Args.push_back(B.getInt32(Op));
   auto *NewI =
-      IRB->CreateIntrinsic(Intrinsic::spv_extractv, {I.getType()}, {Args});
+      B.CreateIntrinsic(Intrinsic::spv_extractv, {I.getType()}, {Args});
   I.replaceAllUsesWith(NewI);
   I.eraseFromParent();
   return NewI;
@@ -512,30 +532,34 @@ Instruction *SPIRVEmitIntrinsics::visitExtractValueInst(ExtractValueInst &I) {
 Instruction *SPIRVEmitIntrinsics::visitLoadInst(LoadInst &I) {
   if (!I.getType()->isAggregateType())
     return &I;
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   TrackConstants = false;
   const auto *TLI = TM->getSubtargetImpl()->getTargetLowering();
   MachineMemOperand::Flags Flags =
       TLI->getLoadMemOperandFlags(I, F->getParent()->getDataLayout());
   auto *NewI =
-      IRB->CreateIntrinsic(Intrinsic::spv_load, {I.getOperand(0)->getType()},
-                           {I.getPointerOperand(), IRB->getInt16(Flags),
-                            IRB->getInt8(I.getAlign().value())});
-  replaceMemInstrUses(&I, NewI);
+      B.CreateIntrinsic(Intrinsic::spv_load, {I.getOperand(0)->getType()},
+                           {I.getPointerOperand(), B.getInt16(Flags),
+                            B.getInt8(I.getAlign().value())});
+  replaceMemInstrUses(&I, NewI, B);
   return NewI;
 }
 
 Instruction *SPIRVEmitIntrinsics::visitStoreInst(StoreInst &I) {
   if (!AggrStores.contains(&I))
     return &I;
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   TrackConstants = false;
   const auto *TLI = TM->getSubtargetImpl()->getTargetLowering();
   MachineMemOperand::Flags Flags =
       TLI->getStoreMemOperandFlags(I, F->getParent()->getDataLayout());
   auto *PtrOp = I.getPointerOperand();
-  auto *NewI = IRB->CreateIntrinsic(
+  auto *NewI = B.CreateIntrinsic(
       Intrinsic::spv_store, {I.getValueOperand()->getType(), PtrOp->getType()},
-      {I.getValueOperand(), PtrOp, IRB->getInt16(Flags),
-       IRB->getInt8(I.getAlign().value())});
+      {I.getValueOperand(), PtrOp, B.getInt16(Flags),
+       B.getInt8(I.getAlign().value())});
   I.eraseFromParent();
   return NewI;
 }
@@ -552,14 +576,15 @@ Instruction *SPIRVEmitIntrinsics::visitAllocaInst(AllocaInst &I) {
           false);
     ArraySize = I.getArraySize();
   }
-
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   TrackConstants = false;
   Type *PtrTy = I.getType();
   auto *NewI =
       ArraySize
-          ? IRB->CreateIntrinsic(Intrinsic::spv_alloca_array,
+          ? B.CreateIntrinsic(Intrinsic::spv_alloca_array,
                                  {PtrTy, ArraySize->getType()}, {ArraySize})
-          : IRB->CreateIntrinsic(Intrinsic::spv_alloca, {PtrTy}, {});
+          : B.CreateIntrinsic(Intrinsic::spv_alloca, {PtrTy}, {});
   std::string InstName = I.hasName() ? I.getName().str() : "";
   I.replaceAllUsesWith(NewI);
   I.eraseFromParent();
@@ -569,50 +594,55 @@ Instruction *SPIRVEmitIntrinsics::visitAllocaInst(AllocaInst &I) {
 
 Instruction *SPIRVEmitIntrinsics::visitAtomicCmpXchgInst(AtomicCmpXchgInst &I) {
   assert(I.getType()->isAggregateType() && "Aggregate result is expected");
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
   SmallVector<Value *> Args;
   for (auto &Op : I.operands())
     Args.push_back(Op);
-  Args.push_back(IRB->getInt32(I.getSyncScopeID()));
-  Args.push_back(IRB->getInt32(
+  Args.push_back(B.getInt32(I.getSyncScopeID()));
+  Args.push_back(B.getInt32(
       static_cast<uint32_t>(getMemSemantics(I.getSuccessOrdering()))));
-  Args.push_back(IRB->getInt32(
+  Args.push_back(B.getInt32(
       static_cast<uint32_t>(getMemSemantics(I.getFailureOrdering()))));
-  auto *NewI = IRB->CreateIntrinsic(Intrinsic::spv_cmpxchg,
+  auto *NewI = B.CreateIntrinsic(Intrinsic::spv_cmpxchg,
                                     {I.getPointerOperand()->getType()}, {Args});
-  replaceMemInstrUses(&I, NewI);
+  replaceMemInstrUses(&I, NewI, B);
   return NewI;
 }
 
 Instruction *SPIRVEmitIntrinsics::visitUnreachableInst(UnreachableInst &I) {
-  IRB->SetInsertPoint(&I);
-  IRB->CreateIntrinsic(Intrinsic::spv_unreachable, {}, {});
+  IRBuilder<> B(I.getParent());
+  B.SetInsertPoint(&I);
+  B.CreateIntrinsic(Intrinsic::spv_unreachable, {}, {});
   return &I;
 }
 
-void SPIRVEmitIntrinsics::processGlobalValue(GlobalVariable &GV) {
+void SPIRVEmitIntrinsics::processGlobalValue(GlobalVariable &GV,
+                                             IRBuilder<> &B) {
   // Skip special artifical variable llvm.global.annotations.
   if (GV.getName() == "llvm.global.annotations")
     return;
   if (GV.hasInitializer() && !isa<UndefValue>(GV.getInitializer())) {
     Constant *Init = GV.getInitializer();
-    Type *Ty = isAggrToReplace(Init) ? IRB->getInt32Ty() : Init->getType();
-    Constant *Const = isAggrToReplace(Init) ? IRB->getInt32(1) : Init;
-    auto *InitInst = IRB->CreateIntrinsic(Intrinsic::spv_init_global,
-                                          {GV.getType(), Ty}, {&GV, Const});
+    Type *Ty = isAggrToReplace(Init) ? B.getInt32Ty() : Init->getType();
+    Constant *Const = isAggrToReplace(Init) ? B.getInt32(1) : Init;
+    auto *InitInst = B.CreateIntrinsic(Intrinsic::spv_init_global,
+                                       {GV.getType(), Ty}, {&GV, Const});
     InitInst->setArgOperand(1, Init);
   }
   if ((!GV.hasInitializer() || isa<UndefValue>(GV.getInitializer())) &&
       GV.getNumUses() == 0)
-    IRB->CreateIntrinsic(Intrinsic::spv_unref_global, GV.getType(), &GV);
+    B.CreateIntrinsic(Intrinsic::spv_unref_global, GV.getType(), &GV);
 }
 
-void SPIRVEmitIntrinsics::insertAssignPtrTypeIntrs(Instruction *I) {
+void SPIRVEmitIntrinsics::insertAssignPtrTypeIntrs(Instruction *I,
+                                                   IRBuilder<> &B) {
   reportFatalOnTokenType(I);
   if (!I->getType()->isPointerTy() || !requireAssignType(I) ||
       isa<BitCastInst>(I))
     return;
 
-  setInsertPointSkippingPhis(*IRB, I->getNextNode());
+  setInsertPointSkippingPhis(B, I->getNextNode());
 
   Constant *EltTyConst;
   unsigned AddressSpace = I->getType()->getPointerAddressSpace();
@@ -624,14 +654,15 @@ void SPIRVEmitIntrinsics::insertAssignPtrTypeIntrs(Instruction *I) {
     EltTyConst = UndefValue::get(IntegerType::getInt8Ty(I->getContext()));
 
   buildIntrWithMD(Intrinsic::spv_assign_ptr_type, {I->getType()}, EltTyConst, I,
-                  {IRB->getInt32(AddressSpace)});
+                  {B.getInt32(AddressSpace)}, B);
 }
 
-void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I) {
+void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I,
+                                                IRBuilder<> &B) {
   reportFatalOnTokenType(I);
   Type *Ty = I->getType();
   if (!Ty->isVoidTy() && !Ty->isPointerTy() && requireAssignType(I)) {
-    setInsertPointSkippingPhis(*IRB, I->getNextNode());
+    setInsertPointSkippingPhis(B, I->getNextNode());
     Type *TypeToAssign = Ty;
     if (auto *II = dyn_cast<IntrinsicInst>(I)) {
       if (II->getIntrinsicID() == Intrinsic::spv_const_composite ||
@@ -642,33 +673,34 @@ void SPIRVEmitIntrinsics::insertAssignTypeIntrs(Instruction *I) {
       }
     }
     Constant *Const = UndefValue::get(TypeToAssign);
-    buildIntrWithMD(Intrinsic::spv_assign_type, {Ty}, Const, I, {});
+    buildIntrWithMD(Intrinsic::spv_assign_type, {Ty}, Const, I, {}, B);
   }
   for (const auto &Op : I->operands()) {
     if (isa<ConstantPointerNull>(Op) || isa<UndefValue>(Op) ||
         // Check GetElementPtrConstantExpr case.
         (isa<ConstantExpr>(Op) && isa<GEPOperator>(Op))) {
-      setInsertPointSkippingPhis(*IRB, I);
+      setInsertPointSkippingPhis(B, I);
       if (isa<UndefValue>(Op) && Op->getType()->isAggregateType())
-        buildIntrWithMD(Intrinsic::spv_assign_type, {IRB->getInt32Ty()}, Op,
-                        UndefValue::get(IRB->getInt32Ty()), {});
+        buildIntrWithMD(Intrinsic::spv_assign_type, {B.getInt32Ty()}, Op,
+                        UndefValue::get(B.getInt32Ty()), {}, B);
       else if (!isa<Instruction>(Op)) // TODO: This case could be removed
-        buildIntrWithMD(Intrinsic::spv_assign_type, {Op->getType()}, Op, Op,
-                        {});
+        buildIntrWithMD(Intrinsic::spv_assign_type, {Op->getType()}, Op, Op, {},
+                        B);
     }
   }
 }
 
-void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I) {
+void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
+                                                 IRBuilder<> &B) {
   auto *II = dyn_cast<IntrinsicInst>(I);
   if (II && II->getIntrinsicID() == Intrinsic::spv_const_composite &&
       TrackConstants) {
-    IRB->SetInsertPoint(I->getNextNode());
-    Type *Ty = IRB->getInt32Ty();
+    B.SetInsertPoint(I->getNextNode());
+    Type *Ty = B.getInt32Ty();
     auto t = AggrConsts.find(I);
     assert(t != AggrConsts.end());
     auto *NewOp = buildIntrWithMD(Intrinsic::spv_track_constant, {Ty, Ty},
-                                  t->second, I, {});
+                                  t->second, I, {}, B);
     I->replaceAllUsesWith(NewOp);
     NewOp->setArgOperand(0, I);
   }
@@ -681,18 +713,19 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I) {
       if (II && ((II->getIntrinsicID() == Intrinsic::spv_gep && OpNo == 0) ||
                  (II->paramHasAttr(OpNo, Attribute::ImmArg))))
         continue;
-      IRB->SetInsertPoint(I);
-      auto *NewOp = buildIntrWithMD(Intrinsic::spv_track_constant,
-                                    {Op->getType(), Op->getType()}, Op, Op, {});
+      B.SetInsertPoint(I);
+      auto *NewOp =
+          buildIntrWithMD(Intrinsic::spv_track_constant,
+                          {Op->getType(), Op->getType()}, Op, Op, {}, B);
       I->setOperand(OpNo, NewOp);
     }
   }
   if (I->hasName()) {
     reportFatalOnTokenType(I);
-    setInsertPointSkippingPhis(*IRB, I->getNextNode());
+    setInsertPointSkippingPhis(B, I->getNextNode());
     std::vector<Value *> Args = {I};
-    addStringImm(I->getName(), *IRB, Args);
-    IRB->CreateIntrinsic(Intrinsic::spv_assign_name, {I->getType()}, Args);
+    addStringImm(I->getName(), B, Args);
+    B.CreateIntrinsic(Intrinsic::spv_assign_name, {I->getType()}, Args);
   }
 }
 
@@ -700,7 +733,7 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
   if (Func.isDeclaration())
     return false;
   F = &Func;
-  IRB = new IRBuilder<>(Func.getContext());
+  IRBuilder<> B(Func.getContext());
   AggrConsts.clear();
   AggrStores.clear();
 
@@ -715,32 +748,31 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
       AggrStores.insert(&I);
   }
 
-  IRB->SetInsertPoint(&Func.getEntryBlock(), Func.getEntryBlock().begin());
+  B.SetInsertPoint(&Func.getEntryBlock(), Func.getEntryBlock().begin());
   for (auto &GV : Func.getParent()->globals())
-    processGlobalValue(GV);
+    processGlobalValue(GV, B);
 
-  preprocessUndefs();
-  preprocessCompositeConstants();
+  preprocessUndefs(B);
+  preprocessCompositeConstants(B);
   SmallVector<Instruction *> Worklist;
   for (auto &I : instructions(Func))
     Worklist.push_back(&I);
 
   for (auto &I : Worklist) {
-    insertAssignPtrTypeIntrs(I);
-    insertAssignTypeIntrs(I);
-    insertPtrCastOrAssignTypeInstr(I);
+    insertAssignPtrTypeIntrs(I, B);
+    insertAssignTypeIntrs(I, B);
+    insertPtrCastOrAssignTypeInstr(I, B);
   }
-
   for (auto *I : Worklist) {
     TrackConstants = true;
     if (!I->getType()->isVoidTy() || isa<StoreInst>(I))
-      IRB->SetInsertPoint(I->getNextNode());
+      B.SetInsertPoint(I->getNextNode());
     // Visitors return either the original/newly created instruction for further
     // processing, nullptr otherwise.
     I = visit(*I);
     if (!I)
       continue;
-    processInstrAfterVisit(I);
+    processInstrAfterVisit(I, B);
   }
   return true;
 }

>From a791c181a2eb1c807e00d1ad32964338b4e30f8a Mon Sep 17 00:00:00 2001
From: "Wlodarczyk, Bertrand" <bertrand.wlodarczyk at intel.com>
Date: Tue, 5 Mar 2024 12:59:31 +0100
Subject: [PATCH 2/2] Fix format

---
 llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index dfdfae551dccf9..5c432d68273234 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -252,7 +252,7 @@ Instruction *SPIRVEmitIntrinsics::visitSwitchInst(SwitchInst &I) {
       Args.push_back(Op);
   B.SetInsertPoint(&I);
   B.CreateIntrinsic(Intrinsic::spv_switch, {I.getOperand(0)->getType()},
-                       {Args});
+                    {Args});
   return &I;
 }
 
@@ -540,8 +540,8 @@ Instruction *SPIRVEmitIntrinsics::visitLoadInst(LoadInst &I) {
       TLI->getLoadMemOperandFlags(I, F->getParent()->getDataLayout());
   auto *NewI =
       B.CreateIntrinsic(Intrinsic::spv_load, {I.getOperand(0)->getType()},
-                           {I.getPointerOperand(), B.getInt16(Flags),
-                            B.getInt8(I.getAlign().value())});
+                        {I.getPointerOperand(), B.getInt16(Flags),
+                         B.getInt8(I.getAlign().value())});
   replaceMemInstrUses(&I, NewI, B);
   return NewI;
 }
@@ -581,10 +581,9 @@ Instruction *SPIRVEmitIntrinsics::visitAllocaInst(AllocaInst &I) {
   TrackConstants = false;
   Type *PtrTy = I.getType();
   auto *NewI =
-      ArraySize
-          ? B.CreateIntrinsic(Intrinsic::spv_alloca_array,
-                                 {PtrTy, ArraySize->getType()}, {ArraySize})
-          : B.CreateIntrinsic(Intrinsic::spv_alloca, {PtrTy}, {});
+      ArraySize ? B.CreateIntrinsic(Intrinsic::spv_alloca_array,
+                                    {PtrTy, ArraySize->getType()}, {ArraySize})
+                : B.CreateIntrinsic(Intrinsic::spv_alloca, {PtrTy}, {});
   std::string InstName = I.hasName() ? I.getName().str() : "";
   I.replaceAllUsesWith(NewI);
   I.eraseFromParent();
@@ -605,7 +604,7 @@ Instruction *SPIRVEmitIntrinsics::visitAtomicCmpXchgInst(AtomicCmpXchgInst &I) {
   Args.push_back(B.getInt32(
       static_cast<uint32_t>(getMemSemantics(I.getFailureOrdering()))));
   auto *NewI = B.CreateIntrinsic(Intrinsic::spv_cmpxchg,
-                                    {I.getPointerOperand()->getType()}, {Args});
+                                 {I.getPointerOperand()->getType()}, {Args});
   replaceMemInstrUses(&I, NewI, B);
   return NewI;
 }



More information about the llvm-commits mailing list