Skip to content

Commit 83c1d00

Browse files
[SPIR-V] Overhaul module analysis to improve translation speed and simplify the underlying logics (llvm#120415)
This PR is to address legacy issues with module analysis that currently uses a complicated and not so efficient approach to trace dependencies between SPIR-V id's via a duplicate tracker data structures and an explicitly built dependency graph. Even a quick performance check without any specialized benchmarks points to this part of the implementation as a biggest bottleneck. This PR specifically: * eliminates a need to build a dependency graph as a data structure, * updates the test suite (mainly, by fixing incorrect CHECK's referring to a hardcoded order of definitions, contradicting the spec requirement to allow certain definitions to go "in any order", see https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_logical_layout_of_a_module), * improves function pointers implementation so that it now passes EXPENSIVE_CHECKS (thus removing 3 XFAIL's in the test suite). As a quick sanity check of whether goals of the PR are achieved, we can measure time of translation for any big LLVM IR. While testing the PR in the local development environment, improvements of the x5 order have been observed. For example, the SYCL test case "group barrier" that is a ~1Mb binary IR input shows the following values of the naive performance metric that we can nevertheless apply here to roughly estimate effects of the PR. before the PR: ``` $ time llc -O0 -mtriple=spirv64v1.6-unknown-unknown _group_barrier_phi.bc -o 1 --filetype=obj real 3m33.241s user 3m14.688s sys 0m18.530s ``` after the PR ``` $ time llc -O0 -mtriple=spirv64v1.6-unknown-unknown _group_barrier_phi.bc -o 1 --filetype=obj real 0m42.031s user 0m38.834s sys 0m3.193s ``` Next work should probably address Duplicate Tracker further, as it needs analysis now from the perspective of what parts of it are not necessary now, after changing the approach to implementation of the module analysis step.
1 parent cf23549 commit 83c1d00

35 files changed

+425
-439
lines changed

llvm/lib/Target/SPIRV/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ add_llvm_target(SPIRVCodeGen
2020
SPIRVCallLowering.cpp
2121
SPIRVInlineAsmLowering.cpp
2222
SPIRVCommandLine.cpp
23-
SPIRVDuplicatesTracker.cpp
2423
SPIRVEmitIntrinsics.cpp
2524
SPIRVGlobalRegistry.cpp
2625
SPIRVInstrInfo.cpp

llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ void SPIRVAsmPrinter::emitInstruction(const MachineInstr *MI) {
274274
}
275275

276276
void SPIRVAsmPrinter::outputModuleSection(SPIRV::ModuleSectionType MSType) {
277-
for (MachineInstr *MI : MAI->getMSInstrs(MSType))
277+
for (const MachineInstr *MI : MAI->getMSInstrs(MSType))
278278
outputInstruction(MI);
279279
}
280280

@@ -326,7 +326,7 @@ void SPIRVAsmPrinter::outputOpMemoryModel() {
326326
void SPIRVAsmPrinter::outputEntryPoints() {
327327
// Find all OpVariable IDs with required StorageClass.
328328
DenseSet<Register> InterfaceIDs;
329-
for (MachineInstr *MI : MAI->GlobalVarList) {
329+
for (const MachineInstr *MI : MAI->GlobalVarList) {
330330
assert(MI->getOpcode() == SPIRV::OpVariable);
331331
auto SC = static_cast<SPIRV::StorageClass::StorageClass>(
332332
MI->getOperand(2).getImm());
@@ -336,14 +336,14 @@ void SPIRVAsmPrinter::outputEntryPoints() {
336336
// declaring all global variables referenced by the entry point call tree.
337337
if (ST->isAtLeastSPIRVVer(VersionTuple(1, 4)) ||
338338
SC == SPIRV::StorageClass::Input || SC == SPIRV::StorageClass::Output) {
339-
MachineFunction *MF = MI->getMF();
339+
const MachineFunction *MF = MI->getMF();
340340
Register Reg = MAI->getRegisterAlias(MF, MI->getOperand(0).getReg());
341341
InterfaceIDs.insert(Reg);
342342
}
343343
}
344344

345345
// Output OpEntryPoints adding interface args to all of them.
346-
for (MachineInstr *MI : MAI->getMSInstrs(SPIRV::MB_EntryPoints)) {
346+
for (const MachineInstr *MI : MAI->getMSInstrs(SPIRV::MB_EntryPoints)) {
347347
SPIRVMCInstLower MCInstLowering;
348348
MCInst TmpInst;
349349
MCInstLowering.lower(MI, TmpInst, MAI);
@@ -381,9 +381,8 @@ void SPIRVAsmPrinter::outputGlobalRequirements() {
381381

382382
void SPIRVAsmPrinter::outputExtFuncDecls() {
383383
// Insert OpFunctionEnd after each declaration.
384-
SmallVectorImpl<MachineInstr *>::iterator
385-
I = MAI->getMSInstrs(SPIRV::MB_ExtFuncDecls).begin(),
386-
E = MAI->getMSInstrs(SPIRV::MB_ExtFuncDecls).end();
384+
auto I = MAI->getMSInstrs(SPIRV::MB_ExtFuncDecls).begin(),
385+
E = MAI->getMSInstrs(SPIRV::MB_ExtFuncDecls).end();
387386
for (; I != E; ++I) {
388387
outputInstruction(*I);
389388
if ((I + 1) == E || (*(I + 1))->getOpcode() == SPIRV::OpFunction)

llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ bool SPIRVCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
418418
.addImm(FuncControl)
419419
.addUse(GR->getSPIRVTypeID(FuncTy));
420420
GR->recordFunctionDefinition(&F, &MB.getInstr()->getOperand(0));
421+
GR->addGlobalObject(&F, &MIRBuilder.getMF(), FuncVReg);
421422

422423
// Add OpFunctionParameter instructions
423424
int i = 0;
@@ -431,6 +432,7 @@ bool SPIRVCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
431432
.addUse(GR->getSPIRVTypeID(ArgTypeVRegs[i]));
432433
if (F.isDeclaration())
433434
GR->add(&Arg, &MIRBuilder.getMF(), ArgReg);
435+
GR->addGlobalObject(&Arg, &MIRBuilder.getMF(), ArgReg);
434436
i++;
435437
}
436438
// Name the function.

llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.cpp

Lines changed: 0 additions & 136 deletions
This file was deleted.

llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.h

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -211,23 +211,7 @@ class SPIRVGeneralDuplicatesTracker {
211211
SPIRVDuplicatesTracker<MachineInstr> MT;
212212
SPIRVDuplicatesTracker<SPIRV::SpecialTypeDescriptor> ST;
213213

214-
// NOTE: using MOs instead of regs to get rid of MF dependency to be able
215-
// to use flat data structure.
216-
// NOTE: replacing DenseMap with MapVector doesn't affect overall correctness
217-
// but makes LITs more stable, should prefer DenseMap still due to
218-
// significant perf difference.
219-
using SPIRVReg2EntryTy =
220-
MapVector<MachineOperand *, SPIRV::DTSortableEntry *>;
221-
222-
template <typename T>
223-
void prebuildReg2Entry(SPIRVDuplicatesTracker<T> &DT,
224-
SPIRVReg2EntryTy &Reg2Entry,
225-
const SPIRVInstrInfo *TII);
226-
227214
public:
228-
void buildDepsGraph(std::vector<SPIRV::DTSortableEntry *> &Graph,
229-
const SPIRVInstrInfo *TII, MachineModuleInfo *MMI);
230-
231215
void add(const Type *Ty, const MachineFunction *MF, Register R) {
232216
TT.add(unifyPtrType(Ty), MF, R);
233217
}

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,7 @@ Register SPIRVGlobalRegistry::buildGlobalVariable(
721721
}
722722
Reg = MIB->getOperand(0).getReg();
723723
DT.add(GVar, &MIRBuilder.getMF(), Reg);
724+
addGlobalObject(GVar, &MIRBuilder.getMF(), Reg);
724725

725726
// Set to Reg the same type as ResVReg has.
726727
auto MRI = MIRBuilder.getMRI();

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ class SPIRVGlobalRegistry {
8989
// Intrinsic::spv_assign_ptr_type instructions.
9090
DenseMap<Value *, CallInst *> AssignPtrTypeInstr;
9191

92+
// Maps OpVariable and OpFunction-related v-regs to its LLVM IR definition.
93+
DenseMap<std::pair<const MachineFunction *, Register>, const Value *> Reg2GO;
94+
9295
// Add a new OpTypeXXX instruction without checking for duplicates.
9396
SPIRVType *createSPIRVType(const Type *Type, MachineIRBuilder &MIRBuilder,
9497
SPIRV::AccessQualifier::AccessQualifier AQ =
@@ -151,15 +154,17 @@ class SPIRVGlobalRegistry {
151154
return DT.find(F, MF);
152155
}
153156

154-
void buildDepsGraph(std::vector<SPIRV::DTSortableEntry *> &Graph,
155-
const SPIRVInstrInfo *TII,
156-
MachineModuleInfo *MMI = nullptr) {
157-
DT.buildDepsGraph(Graph, TII, MMI);
158-
}
159-
160157
void setBound(unsigned V) { Bound = V; }
161158
unsigned getBound() { return Bound; }
162159

160+
void addGlobalObject(const Value *V, const MachineFunction *MF, Register R) {
161+
Reg2GO[std::make_pair(MF, R)] = V;
162+
}
163+
const Value *getGlobalObject(const MachineFunction *MF, Register R) {
164+
auto It = Reg2GO.find(std::make_pair(MF, R));
165+
return It == Reg2GO.end() ? nullptr : It->second;
166+
}
167+
163168
// Add a record to the map of function return pointer types.
164169
void addReturnType(const Function *ArgF, TypedPointerType *DerivedTy) {
165170
FunResPointerTypes[ArgF] = DerivedTy;

llvm/lib/Target/SPIRV/SPIRVInstrInfo.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,19 @@ bool SPIRVInstrInfo::isConstantInstr(const MachineInstr &MI) const {
4747
}
4848
}
4949

50+
bool SPIRVInstrInfo::isSpecConstantInstr(const MachineInstr &MI) const {
51+
switch (MI.getOpcode()) {
52+
case SPIRV::OpSpecConstantTrue:
53+
case SPIRV::OpSpecConstantFalse:
54+
case SPIRV::OpSpecConstant:
55+
case SPIRV::OpSpecConstantComposite:
56+
case SPIRV::OpSpecConstantOp:
57+
return true;
58+
default:
59+
return false;
60+
}
61+
}
62+
5063
bool SPIRVInstrInfo::isInlineAsmDefInstr(const MachineInstr &MI) const {
5164
switch (MI.getOpcode()) {
5265
case SPIRV::OpAsmTargetINTEL:

llvm/lib/Target/SPIRV/SPIRVInstrInfo.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class SPIRVInstrInfo : public SPIRVGenInstrInfo {
3030
const SPIRVRegisterInfo &getRegisterInfo() const { return RI; }
3131
bool isHeaderInstr(const MachineInstr &MI) const;
3232
bool isConstantInstr(const MachineInstr &MI) const;
33+
bool isSpecConstantInstr(const MachineInstr &MI) const;
3334
bool isInlineAsmDefInstr(const MachineInstr &MI) const;
3435
bool isTypeDeclInstr(const MachineInstr &MI) const;
3536
bool isDecorationInstr(const MachineInstr &MI) const;

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,6 +1117,7 @@ bool SPIRVInstructionSelector::selectMemOperation(Register ResVReg,
11171117
Constant::getNullValue(LLVMArrTy));
11181118
Register VarReg = MRI->createGenericVirtualRegister(LLT::scalar(64));
11191119
GR.add(GV, GR.CurMF, VarReg);
1120+
GR.addGlobalObject(GV, GR.CurMF, VarReg);
11201121

11211122
Result &=
11221123
BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(SPIRV::OpVariable))
@@ -3509,18 +3510,25 @@ bool SPIRVInstructionSelector::selectGlobalValue(
35093510
// References to a function via function pointers generate virtual
35103511
// registers without a definition. We will resolve it later, during
35113512
// module analysis stage.
3513+
Register ResTypeReg = GR.getSPIRVTypeID(ResType);
35123514
MachineRegisterInfo *MRI = MIRBuilder.getMRI();
3513-
Register FuncVReg = MRI->createGenericVirtualRegister(LLT::scalar(64));
3514-
MRI->setRegClass(FuncVReg, &SPIRV::iIDRegClass);
3515-
MachineInstrBuilder MB =
3515+
Register FuncVReg =
3516+
MRI->createGenericVirtualRegister(GR.getRegType(ResType));
3517+
MRI->setRegClass(FuncVReg, &SPIRV::pIDRegClass);
3518+
MachineInstrBuilder MIB1 =
3519+
BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpUndef))
3520+
.addDef(FuncVReg)
3521+
.addUse(ResTypeReg);
3522+
MachineInstrBuilder MIB2 =
35163523
BuildMI(BB, I, I.getDebugLoc(),
35173524
TII.get(SPIRV::OpConstantFunctionPointerINTEL))
35183525
.addDef(NewReg)
3519-
.addUse(GR.getSPIRVTypeID(ResType))
3526+
.addUse(ResTypeReg)
35203527
.addUse(FuncVReg);
35213528
// mapping the function pointer to the used Function
3522-
GR.recordFunctionPointer(&MB.getInstr()->getOperand(2), GVFun);
3523-
return MB.constrainAllUses(TII, TRI, RBI);
3529+
GR.recordFunctionPointer(&MIB2.getInstr()->getOperand(2), GVFun);
3530+
return MIB1.constrainAllUses(TII, TRI, RBI) &&
3531+
MIB2.constrainAllUses(TII, TRI, RBI);
35243532
}
35253533
return BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpConstantNull))
35263534
.addDef(NewReg)

0 commit comments

Comments
 (0)