Skip to content

Commit 4281f29

Browse files
[SPIR-V] Duplicates Tracker accounts for possible changes in Constant usage after optimization (#110835)
This PR introduces changes into processing of internal/service data in SPIRV Backend so that Duplicates Tracker accounts for possible changes in Constant usage after optimization, namely this PR fixes the case when a Constant register stored in Duplicates Tracker after all passes is represented by a non-constant expression. In this case we may be sure that it neither is able to create a duplicate nor is in need of a special placement as a Constant instruction. This PR doesn't introduce a new feature, and in this case we rely on existing set of test cases in the SPIRV Backend test suite to ensure that this PR doesn't break existing assumptions without introducing new test cases. There is a reproducer of the issue available as part of SYCL CTS test suite, however it's a binary of several MB's size. Given the subtlety of the issue, reduction of the reproducer to a reasonable site for inclusion into the SPIRV Backend test suite doesn't seem realistic.
1 parent 45817aa commit 4281f29

File tree

4 files changed

+40
-29
lines changed

4 files changed

+40
-29
lines changed

llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.cpp

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,37 +12,39 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "SPIRVDuplicatesTracker.h"
15+
#include "SPIRVInstrInfo.h"
1516

1617
#define DEBUG_TYPE "build-dep-graph"
1718

1819
using namespace llvm;
1920

2021
template <typename T>
2122
void SPIRVGeneralDuplicatesTracker::prebuildReg2Entry(
22-
SPIRVDuplicatesTracker<T> &DT, SPIRVReg2EntryTy &Reg2Entry) {
23+
SPIRVDuplicatesTracker<T> &DT, SPIRVReg2EntryTy &Reg2Entry,
24+
const SPIRVInstrInfo *TII) {
2325
for (auto &TPair : DT.getAllUses()) {
2426
for (auto &RegPair : TPair.second) {
2527
const MachineFunction *MF = RegPair.first;
2628
Register R = RegPair.second;
2729
MachineInstr *MI = MF->getRegInfo().getUniqueVRegDef(R);
28-
if (!MI)
30+
if (!MI || (TPair.second.getIsConst() && !TII->isConstantInstr(*MI)))
2931
continue;
3032
Reg2Entry[&MI->getOperand(0)] = &TPair.second;
3133
}
3234
}
3335
}
3436

3537
void SPIRVGeneralDuplicatesTracker::buildDepsGraph(
36-
std::vector<SPIRV::DTSortableEntry *> &Graph,
38+
std::vector<SPIRV::DTSortableEntry *> &Graph, const SPIRVInstrInfo *TII,
3739
MachineModuleInfo *MMI = nullptr) {
3840
SPIRVReg2EntryTy Reg2Entry;
39-
prebuildReg2Entry(TT, Reg2Entry);
40-
prebuildReg2Entry(CT, Reg2Entry);
41-
prebuildReg2Entry(GT, Reg2Entry);
42-
prebuildReg2Entry(FT, Reg2Entry);
43-
prebuildReg2Entry(AT, Reg2Entry);
44-
prebuildReg2Entry(MT, Reg2Entry);
45-
prebuildReg2Entry(ST, Reg2Entry);
41+
prebuildReg2Entry(TT, Reg2Entry, TII);
42+
prebuildReg2Entry(CT, Reg2Entry, TII);
43+
prebuildReg2Entry(GT, Reg2Entry, TII);
44+
prebuildReg2Entry(FT, Reg2Entry, TII);
45+
prebuildReg2Entry(AT, Reg2Entry, TII);
46+
prebuildReg2Entry(MT, Reg2Entry, TII);
47+
prebuildReg2Entry(ST, Reg2Entry, TII);
4648

4749
for (auto &Op2E : Reg2Entry) {
4850
SPIRV::DTSortableEntry *E = Op2E.second;
@@ -65,20 +67,19 @@ void SPIRVGeneralDuplicatesTracker::buildDepsGraph(
6567
if (MI->getOpcode() == SPIRV::OpConstantFunctionPointerINTEL && i == 2)
6668
continue;
6769
MachineOperand *RegOp = &VRegDef->getOperand(0);
68-
LLVM_DEBUG({
69-
if (Reg2Entry.count(RegOp) == 0 &&
70-
(MI->getOpcode() != SPIRV::OpVariable || i != 3)) {
71-
dbgs() << "Unexpected pattern while building a dependency "
72-
"graph.\nInstruction: ";
73-
MI->print(dbgs());
74-
dbgs() << "Operand: ";
75-
Op.print(dbgs());
76-
dbgs() << "\nOperand definition: ";
77-
VRegDef->print(dbgs());
78-
}
79-
});
80-
assert((MI->getOpcode() == SPIRV::OpVariable && i == 3) ||
81-
Reg2Entry.count(RegOp));
70+
if (Reg2Entry.count(RegOp) == 0 &&
71+
(MI->getOpcode() != SPIRV::OpVariable || i != 3)) {
72+
std::string DiagMsg;
73+
raw_string_ostream OS(DiagMsg);
74+
OS << "Unexpected pattern while building a dependency "
75+
"graph.\nInstruction: ";
76+
MI->print(OS);
77+
OS << "Operand: ";
78+
Op.print(OS);
79+
OS << "\nOperand definition: ";
80+
VRegDef->print(OS);
81+
report_fatal_error(DiagMsg.c_str());
82+
}
8283
if (Reg2Entry.count(RegOp))
8384
E->addDep(Reg2Entry[RegOp]);
8485
}

llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
namespace llvm {
2828
namespace SPIRV {
29+
class SPIRVInstrInfo;
2930
// NOTE: using MapVector instead of DenseMap because it helps getting
3031
// everything ordered in a stable manner for a price of extra (NumKeys)*PtrSize
3132
// memory and expensive removals which do not happen anyway.
@@ -35,8 +36,9 @@ class DTSortableEntry : public MapVector<const MachineFunction *, Register> {
3536
struct FlagsTy {
3637
unsigned IsFunc : 1;
3738
unsigned IsGV : 1;
39+
unsigned IsConst : 1;
3840
// NOTE: bit-field default init is a C++20 feature.
39-
FlagsTy() : IsFunc(0), IsGV(0) {}
41+
FlagsTy() : IsFunc(0), IsGV(0), IsConst(0) {}
4042
};
4143
FlagsTy Flags;
4244

@@ -45,8 +47,10 @@ class DTSortableEntry : public MapVector<const MachineFunction *, Register> {
4547
// require hoisting of params as well.
4648
bool getIsFunc() const { return Flags.IsFunc; }
4749
bool getIsGV() const { return Flags.IsGV; }
50+
bool getIsConst() const { return Flags.IsConst; }
4851
void setIsFunc(bool V) { Flags.IsFunc = V; }
4952
void setIsGV(bool V) { Flags.IsGV = V; }
53+
void setIsConst(bool V) { Flags.IsConst = V; }
5054

5155
const SmallVector<DTSortableEntry *, 2> &getDeps() const { return Deps; }
5256
void addDep(DTSortableEntry *E) { Deps.push_back(E); }
@@ -162,6 +166,10 @@ template <typename KeyTy> class SPIRVDuplicatesTrackerBase {
162166
typename std::remove_const<
163167
typename std::remove_pointer<KeyTy>::type>::type>())
164168
Storage[V].setIsGV(true);
169+
if (std::is_same<Constant,
170+
typename std::remove_const<
171+
typename std::remove_pointer<KeyTy>::type>::type>())
172+
Storage[V].setIsConst(true);
165173
}
166174

167175
Register find(KeyTy V, const MachineFunction *MF) const {
@@ -213,11 +221,12 @@ class SPIRVGeneralDuplicatesTracker {
213221

214222
template <typename T>
215223
void prebuildReg2Entry(SPIRVDuplicatesTracker<T> &DT,
216-
SPIRVReg2EntryTy &Reg2Entry);
224+
SPIRVReg2EntryTy &Reg2Entry,
225+
const SPIRVInstrInfo *TII);
217226

218227
public:
219228
void buildDepsGraph(std::vector<SPIRV::DTSortableEntry *> &Graph,
220-
MachineModuleInfo *MMI);
229+
const SPIRVInstrInfo *TII, MachineModuleInfo *MMI);
221230

222231
void add(const Type *Ty, const MachineFunction *MF, Register R) {
223232
TT.add(unifyPtrType(Ty), MF, R);

llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,9 @@ class SPIRVGlobalRegistry {
152152
}
153153

154154
void buildDepsGraph(std::vector<SPIRV::DTSortableEntry *> &Graph,
155+
const SPIRVInstrInfo *TII,
155156
MachineModuleInfo *MMI = nullptr) {
156-
DT.buildDepsGraph(Graph, MMI);
157+
DT.buildDepsGraph(Graph, TII, MMI);
157158
}
158159

159160
void setBound(unsigned V) { Bound = V; }

llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ void SPIRVModuleAnalysis::collectGlobalEntities(
282282
void SPIRVModuleAnalysis::processDefInstrs(const Module &M) {
283283
std::vector<SPIRV::DTSortableEntry *> DepsGraph;
284284

285-
GR->buildDepsGraph(DepsGraph, SPVDumpDeps ? MMI : nullptr);
285+
GR->buildDepsGraph(DepsGraph, TII, SPVDumpDeps ? MMI : nullptr);
286286

287287
collectGlobalEntities(
288288
DepsGraph, SPIRV::MB_TypeConstVars,

0 commit comments

Comments
 (0)