Skip to content

Commit 04e2d4e

Browse files
stefan-iligcbot
stefan-il
authored and
igcbot
committed
Avoid unecessary looping in GenericAddressDynamicResolution
Avoid looping through all instructions on each visit.
1 parent 1190c00 commit 04e2d4e

File tree

1 file changed

+57
-54
lines changed

1 file changed

+57
-54
lines changed

‎IGC/Compiler/Optimizer/OpenCLPasses/GenericAddressResolution/GenericAddressDynamicResolution.cpp‎

Lines changed: 57 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ SPDX-License-Identifier: MIT
1414
#include "Compiler/CISACodeGen/OpenCLKernelCodeGen.hpp"
1515
#include "Compiler/IGCPassSupport.h"
1616
#include "Compiler/MetaDataUtilsWrapper.h"
17+
1718
#include "common/LLVMWarningsPush.hpp"
1819
#include "llvmWrapper/Support/Alignment.h"
1920
#include "llvmWrapper/IR/DerivedTypes.h"
21+
#include <llvm/ADT/SmallVector.h>
2022
#include <llvm/IR/Module.h>
2123
#include <llvm/IR/Instructions.h>
2224
#include <llvm/IR/DataLayout.h>
@@ -32,34 +34,33 @@ namespace {
3234
class GenericAddressDynamicResolution : public FunctionPass {
3335
public:
3436
static char ID;
35-
Module* m_module = nullptr;
36-
CodeGenContext* m_ctx = nullptr;
3737

3838
GenericAddressDynamicResolution()
3939
: FunctionPass(ID)
4040
{
4141
}
4242
~GenericAddressDynamicResolution() = default;
4343

44-
virtual StringRef getPassName() const override
44+
StringRef getPassName() const override
4545
{
4646
return "GenericAddressDynamicResolution";
4747
}
4848

49-
virtual void getAnalysisUsage(AnalysisUsage& AU) const override
49+
void getAnalysisUsage(AnalysisUsage& AU) const override
5050
{
5151
AU.addRequired<MetaDataUtilsWrapper>();
5252
AU.addRequired<CodeGenContextWrapper>();
5353
AU.addRequired<CastToGASAnalysis>();
5454
}
5555

56-
virtual bool runOnFunction(Function& F) override;
57-
56+
bool runOnFunction(Function& F) override;
57+
private:
5858
bool visitLoadStoreInst(Instruction& I);
5959
bool visitIntrinsicCall(CallInst& I);
60-
Module* getModule() { return m_module; }
6160

62-
private:
61+
Module* m_module = nullptr;
62+
CodeGenContext* m_ctx = nullptr;
63+
llvm::SmallVector<Instruction*, 32> generatedLoadStores;
6364
bool m_needPrivateBranches = false;
6465
bool m_needLocalBranches = false;
6566

@@ -96,43 +97,36 @@ bool GenericAddressDynamicResolution::runOnFunction(Function& F)
9697
m_needLocalBranches = !GI.isNoLocalToGenericOptionEnabled() && GI.canGenericPointToLocal(F);
9798

9899
bool modified = false;
99-
bool changed = false;
100100

101-
// iterate for all the intrinisics used by to_local, to_global, and to_private
102-
do {
103-
changed = false;
101+
llvm::SmallVector<Instruction*, 32> callInstructions;
102+
llvm::SmallVector<Instruction*, 32> loadStoreInstructions;
104103

105-
for (inst_iterator i = inst_begin(F); i != inst_end(F); ++i) {
106-
Instruction& instruction = (*i);
104+
for (auto& instruction: llvm::instructions(F)) {
107105

108-
if (CallInst * intrinsic = dyn_cast<CallInst>(&instruction)) {
109-
changed = visitIntrinsicCall(*intrinsic);
110-
}
111-
112-
if (changed) {
113-
modified = true;
114-
break;
115-
}
106+
if (isa<CallInst>(instruction)) {
107+
callInstructions.push_back(&instruction);
108+
} else if (isa<LoadInst>(instruction) || isa<StoreInst>(instruction)) {
109+
loadStoreInstructions.push_back(&instruction);
116110
}
117-
} while (changed);
111+
}
112+
// iterate for all the intrinisics used by to_local, to_global, and to_private
113+
for (auto* callInst : callInstructions) {
114+
modified |= visitIntrinsicCall(cast<CallInst>(*callInst));
115+
}
118116

119117
// iterate over all loads/stores with generic address space pointers
120-
do {
121-
changed = false;
122-
123-
for (inst_iterator i = inst_begin(F); i != inst_end(F); ++i) {
124-
Instruction& instruction = (*i);
125-
126-
if (isa<LoadInst>(instruction) || isa<StoreInst>(instruction)) {
127-
changed = visitLoadStoreInst(instruction);
128-
}
118+
for (auto* loadStoreInst : loadStoreInstructions) {
119+
modified |= visitLoadStoreInst(*loadStoreInst);
120+
}
129121

130-
if (changed) {
131-
modified = true;
132-
break;
133-
}
122+
// iterate over all newly generated load/stores
123+
while (!generatedLoadStores.empty()) {
124+
llvm::SmallVector<Instruction*, 32> newInstructions = generatedLoadStores;
125+
generatedLoadStores.clear();
126+
for (auto* loadStoreInst : newInstructions) {
127+
modified |= visitLoadStoreInst(*loadStoreInst);
134128
}
135-
} while (changed);
129+
}
136130

137131
if (m_numAdditionalControlFlows)
138132
{
@@ -155,8 +149,7 @@ bool GenericAddressDynamicResolution::runOnFunction(Function& F)
155149

156150
Type* GenericAddressDynamicResolution::getPointerAsIntType(LLVMContext& ctx, const unsigned AS)
157151
{
158-
Module* pModule = getModule();
159-
DataLayout dataLayout = pModule->getDataLayout();
152+
DataLayout dataLayout = m_module->getDataLayout();
160153
unsigned ptrBits(dataLayout.getPointerSizeInBits(AS));
161154
return IntegerType::get(ctx, ptrBits);
162155
}
@@ -168,11 +161,11 @@ bool GenericAddressDynamicResolution::visitLoadStoreInst(Instruction& I)
168161
Value* pointerOperand = nullptr;
169162
unsigned int pointerAddressSpace = ADDRESS_SPACE_NUM_ADDRESSES;
170163

171-
if (LoadInst * load = dyn_cast<LoadInst>(&I)) {
164+
if (auto* load = dyn_cast<LoadInst>(&I)) {
172165
pointerOperand = load->getPointerOperand();
173166
pointerAddressSpace = load->getPointerAddressSpace();
174167
}
175-
else if (StoreInst * store = dyn_cast<StoreInst>(&I)) {
168+
else if (auto* store = dyn_cast<StoreInst>(&I)) {
176169
pointerOperand = store->getPointerOperand();
177170
pointerAddressSpace = store->getPointerAddressSpace();
178171
}
@@ -241,7 +234,7 @@ void GenericAddressDynamicResolution::resolveGAS(Instruction& I, Value* pointerO
241234
// with the corresponding address space.
242235

243236
IGCLLVM::IRBuilder<> builder(&I);
244-
PointerType* pointerType = dyn_cast<PointerType>(pointerOperand->getType());
237+
auto* pointerType = dyn_cast<PointerType>(pointerOperand->getType());
245238
IGC_ASSERT( pointerType != nullptr );
246239
ConstantInt* privateTag = builder.getInt64(1); // tag 001
247240
ConstantInt* localTag = builder.getInt64(2); // tag 010
@@ -275,16 +268,21 @@ void GenericAddressDynamicResolution::resolveGAS(Instruction& I, Value* pointerO
275268
builder.SetInsertPoint(BB);
276269
PointerType* ptrType = IGCLLVM::getWithSamePointeeType(pointerType, addressSpace);
277270
Value* ptr = builder.CreateAddrSpaceCast(pointerOperand, ptrType);
271+
Instruction* generatedLoadStore = nullptr;
278272

279-
if (LoadInst* LI = dyn_cast<LoadInst>(&I))
273+
if (auto* LI = dyn_cast<LoadInst>(&I))
280274
{
281275
load = builder.CreateAlignedLoad(LI->getType(), ptr, getAlign(*LI), LI->isVolatile(), LoadName);
276+
generatedLoadStore = cast<Instruction>(load);
282277
}
283-
else if (StoreInst* SI = dyn_cast<StoreInst>(&I))
278+
else if (auto* SI = dyn_cast<StoreInst>(&I))
284279
{
285-
builder.CreateAlignedStore(I.getOperand(0), ptr, getAlign(*SI), SI->isVolatile());
280+
generatedLoadStore = builder.CreateAlignedStore(I.getOperand(0), ptr, getAlign(*SI), SI->isVolatile());
281+
}
282+
if (generatedLoadStore != nullptr)
283+
{
284+
generatedLoadStores.push_back(generatedLoadStore);
286285
}
287-
288286
builder.CreateBr(convergeBlock);
289287
return BB;
290288
};
@@ -345,23 +343,28 @@ void GenericAddressDynamicResolution::resolveGAS(Instruction& I, Value* pointerO
345343
void GenericAddressDynamicResolution::resolveGASWithoutBranches(Instruction& I, Value* pointerOperand)
346344
{
347345
IGCLLVM::IRBuilder<> builder(&I);
348-
PointerType* pointerType = dyn_cast<PointerType>(pointerOperand->getType());
346+
auto* pointerType = dyn_cast<PointerType>(pointerOperand->getType());
349347
IGC_ASSERT( pointerType != nullptr );
350348

351349
Value* nonLocalLoad = nullptr;
352350

353351
PointerType* ptrType = IGCLLVM::getWithSamePointeeType(pointerType, ADDRESS_SPACE_GLOBAL);
354352
Value* globalPtr = builder.CreateAddrSpaceCast(pointerOperand, ptrType);
353+
Instruction* generatedLoadStore = nullptr;
355354

356-
if (LoadInst* LI = dyn_cast<LoadInst>(&I))
355+
if (auto* LI = dyn_cast<LoadInst>(&I))
357356
{
358357
nonLocalLoad = builder.CreateAlignedLoad(LI->getType(), globalPtr, getAlign(*LI), LI->isVolatile(), "globalOrPrivateLoad");
358+
generatedLoadStore = cast<Instruction>(nonLocalLoad);
359359
}
360-
else if (StoreInst* SI = dyn_cast<StoreInst>(&I))
360+
else if (auto* SI = dyn_cast<StoreInst>(&I))
361361
{
362-
builder.CreateAlignedStore(I.getOperand(0), globalPtr, getAlign(*SI), SI->isVolatile());
362+
generatedLoadStore = builder.CreateAlignedStore(I.getOperand(0), globalPtr, getAlign(*SI), SI->isVolatile());
363+
}
364+
if (generatedLoadStore != nullptr)
365+
{
366+
generatedLoadStores.push_back(generatedLoadStore);
363367
}
364-
365368
if (nonLocalLoad != nullptr)
366369
{
367370
I.replaceAllUsesWith(nonLocalLoad);
@@ -386,12 +389,12 @@ bool GenericAddressDynamicResolution::visitIntrinsicCall(CallInst& I)
386389
{
387390
IGC_ASSERT(IGCLLVM::getNumArgOperands(&I) == 1);
388391
Value* arg = I.getArgOperand(0);
389-
PointerType* dstType = dyn_cast<PointerType>(I.getType());
392+
auto* dstType = dyn_cast<PointerType>(I.getType());
390393
IGC_ASSERT( dstType != nullptr );
391394
const unsigned targetAS = cast<PointerType>(I.getType())->getAddressSpace();
392395

393396
IGCLLVM::IRBuilder<> builder(&I);
394-
PointerType* pointerType = dyn_cast<PointerType>(arg->getType());
397+
auto* pointerType = dyn_cast<PointerType>(arg->getType());
395398
IGC_ASSERT( pointerType != nullptr );
396399
ConstantInt* globalTag = builder.getInt64(0); // tag 000/111
397400
ConstantInt* privateTag = builder.getInt64(1); // tag 001
@@ -411,7 +414,7 @@ bool GenericAddressDynamicResolution::visitIntrinsicCall(CallInst& I)
411414
// Force distinguishing private and global pointers if a kernel uses explicit casts.
412415
// For more details please refer to section "Generic Address Space Explicit Casts" in
413416
// documentation directory under igc/generic-pointers/generic-pointers.md
414-
auto ClContext = static_cast<OpenCLProgramContext*>(m_ctx);
417+
auto* ClContext = static_cast<OpenCLProgramContext*>(m_ctx);
415418
ClContext->setDistinguishBetweenPrivateAndGlobalPtr(true);
416419
}
417420

0 commit comments

Comments
 (0)