From 17cfab381883bd7bbf8139df3701db5209ff63c5 Mon Sep 17 00:00:00 2001 From: Stefan Ilic Date: Fri, 31 Jan 2025 11:40:31 +0000 Subject: [PATCH] Avoid unecessary looping in GenericAddressDynamicResolution Avoid looping through all instructions on each visit. --- .../GenericAddressDynamicResolution.cpp | 104 +++++++++--------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/IGC/Compiler/Optimizer/OpenCLPasses/GenericAddressResolution/GenericAddressDynamicResolution.cpp b/IGC/Compiler/Optimizer/OpenCLPasses/GenericAddressResolution/GenericAddressDynamicResolution.cpp index bd4cfb0841b2..49d8157210cf 100644 --- a/IGC/Compiler/Optimizer/OpenCLPasses/GenericAddressResolution/GenericAddressDynamicResolution.cpp +++ b/IGC/Compiler/Optimizer/OpenCLPasses/GenericAddressResolution/GenericAddressDynamicResolution.cpp @@ -14,9 +14,11 @@ SPDX-License-Identifier: MIT #include "Compiler/CISACodeGen/OpenCLKernelCodeGen.hpp" #include "Compiler/IGCPassSupport.h" #include "Compiler/MetaDataUtilsWrapper.h" + #include "common/LLVMWarningsPush.hpp" #include "llvmWrapper/Support/Alignment.h" #include "llvmWrapper/IR/DerivedTypes.h" +#include #include #include #include @@ -32,8 +34,6 @@ namespace { class GenericAddressDynamicResolution : public FunctionPass { public: static char ID; - Module* m_module = nullptr; - CodeGenContext* m_ctx = nullptr; GenericAddressDynamicResolution() : FunctionPass(ID) @@ -41,25 +41,26 @@ namespace { } ~GenericAddressDynamicResolution() = default; - virtual StringRef getPassName() const override + StringRef getPassName() const override { return "GenericAddressDynamicResolution"; } - virtual void getAnalysisUsage(AnalysisUsage& AU) const override + void getAnalysisUsage(AnalysisUsage& AU) const override { AU.addRequired(); AU.addRequired(); AU.addRequired(); } - virtual bool runOnFunction(Function& F) override; - + bool runOnFunction(Function& F) override; + private: bool visitLoadStoreInst(Instruction& I); bool visitIntrinsicCall(CallInst& I); - Module* getModule() { return m_module; } - private: + Module* m_module = nullptr; + CodeGenContext* m_ctx = nullptr; + llvm::SmallVector generatedLoadStores; bool m_needPrivateBranches = false; bool m_needLocalBranches = false; @@ -96,43 +97,37 @@ bool GenericAddressDynamicResolution::runOnFunction(Function& F) m_needLocalBranches = !GI.isNoLocalToGenericOptionEnabled() && GI.canGenericPointToLocal(F); bool modified = false; - bool changed = false; - // iterate for all the intrinisics used by to_local, to_global, and to_private - do { - changed = false; - - for (inst_iterator i = inst_begin(F); i != inst_end(F); ++i) { - Instruction& instruction = (*i); + llvm::SmallVector callInstructions; + llvm::SmallVector loadStoreInstructions; - if (CallInst * intrinsic = dyn_cast(&instruction)) { - changed = visitIntrinsicCall(*intrinsic); - } + for (auto& instruction: llvm::instructions(F)) { - if (changed) { - modified = true; - break; - } + if (isa(&instruction)) { + callInstructions.push_back(&instruction); + } + if (isa(instruction)) { + loadStoreInstructions.push_back(&instruction); } - } while (changed); + } + // iterate for all the intrinisics used by to_local, to_global, and to_private + for (auto* callInst : callInstructions) { + modified |= visitIntrinsicCall(cast(*callInst)); + } // iterate over all loads/stores with generic address space pointers - do { - changed = false; - - for (inst_iterator i = inst_begin(F); i != inst_end(F); ++i) { - Instruction& instruction = (*i); - - if (isa(instruction) || isa(instruction)) { - changed = visitLoadStoreInst(instruction); - } + for (auto* loadStoreInst : loadStoreInstructions) { + modified |= visitLoadStoreInst(*loadStoreInst); + } - if (changed) { - modified = true; - break; - } + // iterate over all newly generated load/stores + while (!generatedLoadStores.empty()) { + llvm::SmallVector newInstructions = generatedLoadStores; + generatedLoadStores.clear(); + for (auto* loadStoreInst : newInstructions) { + modified |= visitLoadStoreInst(*loadStoreInst); } - } while (changed); + } if (m_numAdditionalControlFlows) { @@ -155,8 +150,7 @@ bool GenericAddressDynamicResolution::runOnFunction(Function& F) Type* GenericAddressDynamicResolution::getPointerAsIntType(LLVMContext& ctx, const unsigned AS) { - Module* pModule = getModule(); - DataLayout dataLayout = pModule->getDataLayout(); + DataLayout dataLayout = m_module->getDataLayout(); unsigned ptrBits(dataLayout.getPointerSizeInBits(AS)); return IntegerType::get(ctx, ptrBits); } @@ -168,11 +162,11 @@ bool GenericAddressDynamicResolution::visitLoadStoreInst(Instruction& I) Value* pointerOperand = nullptr; unsigned int pointerAddressSpace = ADDRESS_SPACE_NUM_ADDRESSES; - if (LoadInst * load = dyn_cast(&I)) { + if (auto* load = dyn_cast(&I)) { pointerOperand = load->getPointerOperand(); pointerAddressSpace = load->getPointerAddressSpace(); } - else if (StoreInst * store = dyn_cast(&I)) { + else if (auto* store = dyn_cast(&I)) { pointerOperand = store->getPointerOperand(); pointerAddressSpace = store->getPointerAddressSpace(); } @@ -241,7 +235,7 @@ void GenericAddressDynamicResolution::resolveGAS(Instruction& I, Value* pointerO // with the corresponding address space. IGCLLVM::IRBuilder<> builder(&I); - PointerType* pointerType = dyn_cast(pointerOperand->getType()); + auto* pointerType = dyn_cast(pointerOperand->getType()); IGC_ASSERT( pointerType != nullptr ); ConstantInt* privateTag = builder.getInt64(1); // tag 001 ConstantInt* localTag = builder.getInt64(2); // tag 010 @@ -275,15 +269,18 @@ void GenericAddressDynamicResolution::resolveGAS(Instruction& I, Value* pointerO builder.SetInsertPoint(BB); PointerType* ptrType = IGCLLVM::getWithSamePointeeType(pointerType, addressSpace); Value* ptr = builder.CreateAddrSpaceCast(pointerOperand, ptrType); + Instruction* generatedLoadStore = nullptr; - if (LoadInst* LI = dyn_cast(&I)) + if (auto* LI = dyn_cast(&I)) { load = builder.CreateAlignedLoad(LI->getType(), ptr, getAlign(*LI), LI->isVolatile(), LoadName); + generatedLoadStore = cast(load); } - else if (StoreInst* SI = dyn_cast(&I)) + else if (auto* SI = dyn_cast(&I)) { - builder.CreateAlignedStore(I.getOperand(0), ptr, getAlign(*SI), SI->isVolatile()); + generatedLoadStore = builder.CreateAlignedStore(I.getOperand(0), ptr, getAlign(*SI), SI->isVolatile()); } + generatedLoadStores.push_back(generatedLoadStore); builder.CreateBr(convergeBlock); return BB; @@ -345,22 +342,25 @@ void GenericAddressDynamicResolution::resolveGAS(Instruction& I, Value* pointerO void GenericAddressDynamicResolution::resolveGASWithoutBranches(Instruction& I, Value* pointerOperand) { IGCLLVM::IRBuilder<> builder(&I); - PointerType* pointerType = dyn_cast(pointerOperand->getType()); + auto* pointerType = dyn_cast(pointerOperand->getType()); IGC_ASSERT( pointerType != nullptr ); Value* nonLocalLoad = nullptr; + Instruction* generatedLoadStore = nullptr; PointerType* ptrType = IGCLLVM::getWithSamePointeeType(pointerType, ADDRESS_SPACE_GLOBAL); Value* globalPtr = builder.CreateAddrSpaceCast(pointerOperand, ptrType); - if (LoadInst* LI = dyn_cast(&I)) + if (auto* LI = dyn_cast(&I)) { nonLocalLoad = builder.CreateAlignedLoad(LI->getType(), globalPtr, getAlign(*LI), LI->isVolatile(), "globalOrPrivateLoad"); + generatedLoadStore = cast(nonLocalLoad); } - else if (StoreInst* SI = dyn_cast(&I)) + else if (auto* SI = dyn_cast(&I)) { - builder.CreateAlignedStore(I.getOperand(0), globalPtr, getAlign(*SI), SI->isVolatile()); + generatedLoadStore = builder.CreateAlignedStore(I.getOperand(0), globalPtr, getAlign(*SI), SI->isVolatile()); } + generatedLoadStores.push_back(generatedLoadStore); if (nonLocalLoad != nullptr) { @@ -386,12 +386,12 @@ bool GenericAddressDynamicResolution::visitIntrinsicCall(CallInst& I) { IGC_ASSERT(IGCLLVM::getNumArgOperands(&I) == 1); Value* arg = I.getArgOperand(0); - PointerType* dstType = dyn_cast(I.getType()); + auto* dstType = dyn_cast(I.getType()); IGC_ASSERT( dstType != nullptr ); const unsigned targetAS = cast(I.getType())->getAddressSpace(); IGCLLVM::IRBuilder<> builder(&I); - PointerType* pointerType = dyn_cast(arg->getType()); + auto* pointerType = dyn_cast(arg->getType()); IGC_ASSERT( pointerType != nullptr ); ConstantInt* globalTag = builder.getInt64(0); // tag 000/111 ConstantInt* privateTag = builder.getInt64(1); // tag 001 @@ -411,7 +411,7 @@ bool GenericAddressDynamicResolution::visitIntrinsicCall(CallInst& I) // Force distinguishing private and global pointers if a kernel uses explicit casts. // For more details please refer to section "Generic Address Space Explicit Casts" in // documentation directory under igc/generic-pointers/generic-pointers.md - auto ClContext = static_cast(m_ctx); + auto* ClContext = static_cast(m_ctx); ClContext->setDistinguishBetweenPrivateAndGlobalPtr(true); }