improve performance of Module Analysis stage in the part of processing "other instructions" #76047
Conversation
…g 'other instructions'
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-backend-spir-v Author: Vyacheslav Levytskyy (VyacheslavLevytskyy) ChangesThe 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. 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 The fix changes underlying data structure to be Full diff: https://github.com/llvm/llvm-project/pull/76047.diff 1 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 779036016560e8..9d0c01363f44f0 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -263,34 +263,6 @@ void SPIRVModuleAnalysis::processDefInstrs(const Module &M) {
[](const SPIRV::DTSortableEntry *E) { return E->getIsFunc(); }, true);
}
-// True if there is an instruction in the MS list with all the same operands as
-// the given instruction has (after the given starting index).
-// TODO: maybe it needs to check Opcodes too.
-static bool findSameInstrInMS(const MachineInstr &A,
- SPIRV::ModuleSectionType MSType,
- SPIRV::ModuleAnalysisInfo &MAI,
- unsigned StartOpIndex = 0) {
- for (const auto *B : MAI.MS[MSType]) {
- const unsigned NumAOps = A.getNumOperands();
- if (NumAOps != B->getNumOperands() || A.getNumDefs() != B->getNumDefs())
- continue;
- bool AllOpsMatch = true;
- for (unsigned i = StartOpIndex; i < NumAOps && AllOpsMatch; ++i) {
- if (A.getOperand(i).isReg() && B->getOperand(i).isReg()) {
- Register RegA = A.getOperand(i).getReg();
- Register RegB = B->getOperand(i).getReg();
- AllOpsMatch = MAI.getRegisterAlias(A.getMF(), RegA) ==
- MAI.getRegisterAlias(B->getMF(), RegB);
- } else {
- AllOpsMatch = A.getOperand(i).isIdenticalTo(B->getOperand(i));
- }
- }
- if (AllOpsMatch)
- return true;
- }
- return false;
-}
-
// Look for IDs declared with Import linkage, and map the corresponding function
// to the register defining that variable (which will usually be the result of
// an OpFunction). This lets us call externally imported functions using
@@ -319,15 +291,43 @@ void SPIRVModuleAnalysis::collectFuncNames(MachineInstr &MI,
}
}
+using InstrSignature = SmallVector<size_t>;
+using InstrTraces = std::set<InstrSignature>;
+
+// Returns a representation of an instruction as a vector of MachineOperand
+// hash values, see llvm::hash_value(const MachineOperand &MO) for details.
+// This creates a signature of the instruction with the same content
+// that MachineOperand::isIdenticalTo uses for comparison.
+static InstrSignature instrToSignature(MachineInstr &MI,
+ SPIRV::ModuleAnalysisInfo &MAI) {
+ InstrSignature ret;
+ for (unsigned i = 0; i < MI.getNumOperands(); ++i) {
+ const MachineOperand &MO = MI.getOperand(i);
+ size_t h;
+ if (MO.isReg()) {
+ Register RegAlias = MAI.getRegisterAlias(MI.getMF(), MO.getReg());
+ // mimic llvm::hash_value(const MachineOperand &MO)
+ h = hash_combine(MO.getType(), (unsigned)RegAlias,
+ MO.getSubReg(), MO.isDef());
+ }
+ else
+ h = hash_value(MO);
+ ret.push_back(h);
+ }
+ return ret;
+}
+
// Collect the given instruction in the specified MS. We assume global register
// numbering has already occurred by this point. We can directly compare reg
// arguments when detecting duplicates.
static void collectOtherInstr(MachineInstr &MI, SPIRV::ModuleAnalysisInfo &MAI,
- SPIRV::ModuleSectionType MSType,
+ SPIRV::ModuleSectionType MSType, InstrTraces& IS,
bool Append = true) {
MAI.setSkipEmission(&MI);
- if (findSameInstrInMS(MI, MSType, MAI))
+ InstrSignature MISign = instrToSignature(MI, MAI);
+ if (auto FoundMI = IS.find(MISign); FoundMI != IS.end())
return; // Found a duplicate, so don't add it.
+ IS.insert(MISign);
// No duplicates, so add it.
if (Append)
MAI.MS[MSType].push_back(&MI);
@@ -338,6 +338,7 @@ static void collectOtherInstr(MachineInstr &MI, SPIRV::ModuleAnalysisInfo &MAI,
// Some global instructions make reference to function-local ID regs, so cannot
// be correctly collected until these registers are globally numbered.
void SPIRVModuleAnalysis::processOtherInstrs(const Module &M) {
+ InstrTraces IS;
for (auto F = M.begin(), E = M.end(); F != E; ++F) {
if ((*F).isDeclaration())
continue;
@@ -349,20 +350,20 @@ void SPIRVModuleAnalysis::processOtherInstrs(const Module &M) {
continue;
const unsigned OpCode = MI.getOpcode();
if (OpCode == SPIRV::OpName || OpCode == SPIRV::OpMemberName) {
- collectOtherInstr(MI, MAI, SPIRV::MB_DebugNames);
+ collectOtherInstr(MI, MAI, SPIRV::MB_DebugNames, IS);
} else if (OpCode == SPIRV::OpEntryPoint) {
- collectOtherInstr(MI, MAI, SPIRV::MB_EntryPoints);
+ collectOtherInstr(MI, MAI, SPIRV::MB_EntryPoints, IS);
} else if (TII->isDecorationInstr(MI)) {
- collectOtherInstr(MI, MAI, SPIRV::MB_Annotations);
+ collectOtherInstr(MI, MAI, SPIRV::MB_Annotations, IS);
collectFuncNames(MI, &*F);
} else if (TII->isConstantInstr(MI)) {
// Now OpSpecConstant*s are not in DT,
// but they need to be collected anyway.
- collectOtherInstr(MI, MAI, SPIRV::MB_TypeConstVars);
+ collectOtherInstr(MI, MAI, SPIRV::MB_TypeConstVars, IS);
} else if (OpCode == SPIRV::OpFunction) {
collectFuncNames(MI, &*F);
} else if (OpCode == SPIRV::OpTypeForwardPointer) {
- collectOtherInstr(MI, MAI, SPIRV::MB_TypeConstVars, false);
+ collectOtherInstr(MI, MAI, SPIRV::MB_TypeConstVars, IS, false);
}
}
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@michalpaszkowski This is one possible way to fix the only timeout case from test cases of https://github.com/KhronosGroup/SPIRV-LLVM-Translator/tree/main/test |
|
Thank you @VyacheslavLevytskyy very much for the patch! This is a very useful improvement! I will add review comments in a moment. |
|
Thank you @VyacheslavLevytskyy for the patch! Merging as discussed in the email thread. |
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.spvtto 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::setto trace instructions with identical operands instead of the existing approach of thefindSameInstrInMS()function.