Skip to content

Commit 774b957

Browse files
[SPIR-V] improve performance of Module Analysis stage in the part of processing "other instructions" (#76047)
The goal of this PR is to fix an issue when Module Analysis stage is not able to complete processing of a really big LLVM source: #76048. There is an example of a bulky LLVM source: https://github.com/KhronosGroup/SPIRV-LLVM-Translator/blob/main/test/SpecConstants/long-spec-const-composite.ll Processing of this file with `llc -mtriple=spirv64-unknown-unknown -O0 long-spec-const-composite.ll -o long-spec-const-composite.spvt` to produce SPIR-V output using LLVM SPIR-V backend takes too long, and I've never been able to see it actually completes. After the patch from this PR applied elapsed time for me is ~30 sec. The fix changes underlying data structure to be `std::set` to trace instructions with identical operands instead of the existing approach of the `findSameInstrInMS()` function.
1 parent b75b9d8 commit 774b957

File tree

1 file changed

+37
-36
lines changed

1 file changed

+37
-36
lines changed

llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp

Lines changed: 37 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -263,34 +263,6 @@ void SPIRVModuleAnalysis::processDefInstrs(const Module &M) {
263263
[](const SPIRV::DTSortableEntry *E) { return E->getIsFunc(); }, true);
264264
}
265265

266-
// True if there is an instruction in the MS list with all the same operands as
267-
// the given instruction has (after the given starting index).
268-
// TODO: maybe it needs to check Opcodes too.
269-
static bool findSameInstrInMS(const MachineInstr &A,
270-
SPIRV::ModuleSectionType MSType,
271-
SPIRV::ModuleAnalysisInfo &MAI,
272-
unsigned StartOpIndex = 0) {
273-
for (const auto *B : MAI.MS[MSType]) {
274-
const unsigned NumAOps = A.getNumOperands();
275-
if (NumAOps != B->getNumOperands() || A.getNumDefs() != B->getNumDefs())
276-
continue;
277-
bool AllOpsMatch = true;
278-
for (unsigned i = StartOpIndex; i < NumAOps && AllOpsMatch; ++i) {
279-
if (A.getOperand(i).isReg() && B->getOperand(i).isReg()) {
280-
Register RegA = A.getOperand(i).getReg();
281-
Register RegB = B->getOperand(i).getReg();
282-
AllOpsMatch = MAI.getRegisterAlias(A.getMF(), RegA) ==
283-
MAI.getRegisterAlias(B->getMF(), RegB);
284-
} else {
285-
AllOpsMatch = A.getOperand(i).isIdenticalTo(B->getOperand(i));
286-
}
287-
}
288-
if (AllOpsMatch)
289-
return true;
290-
}
291-
return false;
292-
}
293-
294266
// Look for IDs declared with Import linkage, and map the corresponding function
295267
// to the register defining that variable (which will usually be the result of
296268
// an OpFunction). This lets us call externally imported functions using
@@ -319,15 +291,43 @@ void SPIRVModuleAnalysis::collectFuncNames(MachineInstr &MI,
319291
}
320292
}
321293

294+
using InstrSignature = SmallVector<size_t>;
295+
using InstrTraces = std::set<InstrSignature>;
296+
297+
// Returns a representation of an instruction as a vector of MachineOperand
298+
// hash values, see llvm::hash_value(const MachineOperand &MO) for details.
299+
// This creates a signature of the instruction with the same content
300+
// that MachineOperand::isIdenticalTo uses for comparison.
301+
static InstrSignature instrToSignature(MachineInstr &MI,
302+
SPIRV::ModuleAnalysisInfo &MAI) {
303+
InstrSignature Signature;
304+
for (unsigned i = 0; i < MI.getNumOperands(); ++i) {
305+
const MachineOperand &MO = MI.getOperand(i);
306+
size_t h;
307+
if (MO.isReg()) {
308+
Register RegAlias = MAI.getRegisterAlias(MI.getMF(), MO.getReg());
309+
// mimic llvm::hash_value(const MachineOperand &MO)
310+
h = hash_combine(MO.getType(), (unsigned)RegAlias, MO.getSubReg(),
311+
MO.isDef());
312+
} else {
313+
h = hash_value(MO);
314+
}
315+
Signature.push_back(h);
316+
}
317+
return Signature;
318+
}
319+
322320
// Collect the given instruction in the specified MS. We assume global register
323321
// numbering has already occurred by this point. We can directly compare reg
324322
// arguments when detecting duplicates.
325323
static void collectOtherInstr(MachineInstr &MI, SPIRV::ModuleAnalysisInfo &MAI,
326-
SPIRV::ModuleSectionType MSType,
324+
SPIRV::ModuleSectionType MSType, InstrTraces &IS,
327325
bool Append = true) {
328326
MAI.setSkipEmission(&MI);
329-
if (findSameInstrInMS(MI, MSType, MAI))
330-
return; // Found a duplicate, so don't add it.
327+
InstrSignature MISign = instrToSignature(MI, MAI);
328+
auto FoundMI = IS.insert(MISign);
329+
if (!FoundMI.second)
330+
return; // insert failed, so we found a duplicate; don't add it to MAI.MS
331331
// No duplicates, so add it.
332332
if (Append)
333333
MAI.MS[MSType].push_back(&MI);
@@ -338,6 +338,7 @@ static void collectOtherInstr(MachineInstr &MI, SPIRV::ModuleAnalysisInfo &MAI,
338338
// Some global instructions make reference to function-local ID regs, so cannot
339339
// be correctly collected until these registers are globally numbered.
340340
void SPIRVModuleAnalysis::processOtherInstrs(const Module &M) {
341+
InstrTraces IS;
341342
for (auto F = M.begin(), E = M.end(); F != E; ++F) {
342343
if ((*F).isDeclaration())
343344
continue;
@@ -349,20 +350,20 @@ void SPIRVModuleAnalysis::processOtherInstrs(const Module &M) {
349350
continue;
350351
const unsigned OpCode = MI.getOpcode();
351352
if (OpCode == SPIRV::OpName || OpCode == SPIRV::OpMemberName) {
352-
collectOtherInstr(MI, MAI, SPIRV::MB_DebugNames);
353+
collectOtherInstr(MI, MAI, SPIRV::MB_DebugNames, IS);
353354
} else if (OpCode == SPIRV::OpEntryPoint) {
354-
collectOtherInstr(MI, MAI, SPIRV::MB_EntryPoints);
355+
collectOtherInstr(MI, MAI, SPIRV::MB_EntryPoints, IS);
355356
} else if (TII->isDecorationInstr(MI)) {
356-
collectOtherInstr(MI, MAI, SPIRV::MB_Annotations);
357+
collectOtherInstr(MI, MAI, SPIRV::MB_Annotations, IS);
357358
collectFuncNames(MI, &*F);
358359
} else if (TII->isConstantInstr(MI)) {
359360
// Now OpSpecConstant*s are not in DT,
360361
// but they need to be collected anyway.
361-
collectOtherInstr(MI, MAI, SPIRV::MB_TypeConstVars);
362+
collectOtherInstr(MI, MAI, SPIRV::MB_TypeConstVars, IS);
362363
} else if (OpCode == SPIRV::OpFunction) {
363364
collectFuncNames(MI, &*F);
364365
} else if (OpCode == SPIRV::OpTypeForwardPointer) {
365-
collectOtherInstr(MI, MAI, SPIRV::MB_TypeConstVars, false);
366+
collectOtherInstr(MI, MAI, SPIRV::MB_TypeConstVars, IS, false);
366367
}
367368
}
368369
}

0 commit comments

Comments
 (0)