Skip to content

Commit 829edfa

Browse files
lsatanovigcbot
authored andcommitted
[IGC VC] GenXVerify pass, corrections.
Avoid unexpecded failures when being run with -genx-verify-terminate=no on broken IRs. Added -genx-verify-set configuration option for pass usage out of the default pipeline. Converted -genx-verify-terminate to enum-option.
1 parent d9a05cd commit 829edfa

File tree

6 files changed

+145
-70
lines changed

6 files changed

+145
-70
lines changed

IGC/VectorCompiler/lib/GenXCodeGen/GenX.h

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,23 @@ enum class BuiltinFunctionKind {
6868
// GenX IR may have different sets of validity invariants for different stages
6969
// in pipeline.
7070
enum class GenXVerifyInvariantSet {
71-
PreIrAdaptors,
72-
PreGenXLegalization,
73-
All,
71+
PostSPIRVReader,
72+
PostIrAdaptors,
73+
PostGenXLowering,
74+
PostGenXLegalization,
75+
// --- Add sets above, in respective optimization pipeline order ---
76+
End_,
77+
Default_ = static_cast<int>(GenXVerifyInvariantSet::End_) -
78+
1 // Must be set to the latest one.
7479
};
7580

81+
static_assert(GenXVerifyInvariantSet::PostSPIRVReader <
82+
GenXVerifyInvariantSet::PostIrAdaptors &&
83+
GenXVerifyInvariantSet::PostIrAdaptors <
84+
GenXVerifyInvariantSet::PostGenXLowering &&
85+
GenXVerifyInvariantSet::PostGenXLowering <
86+
GenXVerifyInvariantSet::PostGenXLegalization);
87+
7688
FunctionPass *createGenXPrinterPass(raw_ostream &O, const std::string &Banner);
7789
ModulePass *createGenXGroupPrinterPass(raw_ostream &O,
7890
const std::string &Banner);
@@ -128,7 +140,7 @@ ModulePass *createGenXRematerializationWrapperPass();
128140
ModulePass *createGenXCoalescingWrapperPass();
129141
ModulePass *createGenXGVClobberCheckerPass();
130142
ModulePass *createGenXVerifyPass(
131-
GenXVerifyInvariantSet PipelineStage = GenXVerifyInvariantSet::All);
143+
GenXVerifyInvariantSet PipelineStage = GenXVerifyInvariantSet::Default_);
132144
ModulePass *createGenXAddressCommoningWrapperPass();
133145
ModulePass *createGenXArgIndirectionWrapperPass();
134146
FunctionPass *createGenXTidyControlFlowPass();

IGC/VectorCompiler/lib/GenXCodeGen/GenXTargetMachine.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -361,8 +361,8 @@ bool GenXTargetMachine::addPassesToEmitFile(PassManagerBase &PM,
361361

362362
if (!DisableVerify) {
363363
vc::addPass(PM, createVerifierPass());
364-
vc::addPass(
365-
PM, createGenXVerifyPass(GenXVerifyInvariantSet::PreGenXLegalization));
364+
vc::addPass(PM,
365+
createGenXVerifyPass(GenXVerifyInvariantSet::PostIrAdaptors));
366366
}
367367

368368
// Run passes to generate vISA.
@@ -486,8 +486,8 @@ bool GenXTargetMachine::addPassesToEmitFile(PassManagerBase &PM,
486486

487487
if (!DisableVerify) {
488488
vc::addPass(PM, createVerifierPass());
489-
vc::addPass(
490-
PM, createGenXVerifyPass(GenXVerifyInvariantSet::PreGenXLegalization));
489+
vc::addPass(PM,
490+
createGenXVerifyPass(GenXVerifyInvariantSet::PostGenXLowering));
491491
}
492492

493493
/// .. include:: GenXRegionCollapsing.cpp
@@ -504,8 +504,8 @@ bool GenXTargetMachine::addPassesToEmitFile(PassManagerBase &PM,
504504

505505
if (!DisableVerify) {
506506
vc::addPass(PM, createVerifierPass());
507-
vc::addPass(
508-
PM, createGenXVerifyPass(GenXVerifyInvariantSet::PreGenXLegalization));
507+
vc::addPass(PM,
508+
createGenXVerifyPass(GenXVerifyInvariantSet::PostGenXLowering));
509509
}
510510

511511
/// .. include:: GenXExtractVectorizer.cpp
@@ -554,7 +554,8 @@ bool GenXTargetMachine::addPassesToEmitFile(PassManagerBase &PM,
554554

555555
if (!DisableVerify) {
556556
vc::addPass(PM, createVerifierPass());
557-
vc::addPass(PM, createGenXVerifyPass());
557+
vc::addPass(
558+
PM, createGenXVerifyPass(GenXVerifyInvariantSet::PostGenXLegalization));
558559
}
559560

560561
/// EarlyCSE
@@ -569,7 +570,8 @@ bool GenXTargetMachine::addPassesToEmitFile(PassManagerBase &PM,
569570

570571
if (!DisableVerify) {
571572
vc::addPass(PM, createVerifierPass());
572-
vc::addPass(PM, createGenXVerifyPass());
573+
vc::addPass(
574+
PM, createGenXVerifyPass(GenXVerifyInvariantSet::PostGenXLegalization));
573575
}
574576

575577
/// LICM
@@ -660,7 +662,8 @@ bool GenXTargetMachine::addPassesToEmitFile(PassManagerBase &PM,
660662
".regalloc"));
661663
if (!DisableVerify) {
662664
vc::addPass(PM, createVerifierPass());
663-
vc::addPass(PM, createGenXVerifyPass());
665+
vc::addPass(
666+
PM, createGenXVerifyPass(GenXVerifyInvariantSet::PostGenXLegalization));
664667
}
665668

666669
/// .. include:: GenXCisaBuilder.cpp
@@ -699,7 +702,7 @@ void GenXTargetMachine::adjustPassManager(PassManagerBuilder &PMBuilder) {
699702
auto AddPacketize = [](const PassManagerBuilder &Builder,
700703
PassManagerBase &PM) {
701704
#ifndef NDEBUG
702-
PM.add(createGenXVerifyPass(GenXVerifyInvariantSet::PreIrAdaptors));
705+
PM.add(createGenXVerifyPass(GenXVerifyInvariantSet::PostSPIRVReader));
703706
#endif
704707
PM.add(createGenXTranslateIntrinsicsPass());
705708
PM.add(createGenXTranslateSPIRVBuiltinsPass());

IGC/VectorCompiler/lib/GenXCodeGen/GenXVerify.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,17 @@ bool GenXVerify::ensure(const bool Cond, const Twine &Msg, const Instruction &I,
3131
const IsFatal IsFatal_) {
3232
if (LLVM_LIKELY(Cond))
3333
return true;
34-
vc::diagnose(I.getContext(),
35-
DbgPrefix + (IsFatal_ == IsFatal::No
36-
? " (non-fatal, spec review required)"
37-
: ""),
38-
Msg, DS_Warning, vc::WarningName::Generic, &I);
34+
if (LLVM_LIKELY(static_cast<bool>(IsFatal_) || OptAllFatal ||
35+
!OptQuietNonFatal))
36+
vc::diagnose(I.getContext(),
37+
DbgPrefix + (IsFatal_ == IsFatal::No
38+
? " (non-fatal, spec review required)"
39+
: ""),
40+
Msg, DS_Warning, vc::WarningName::Generic, &I);
3941
if ((LLVM_LIKELY(!OptAllFatal) && IsFatal_ == IsFatal::Yes) ||
4042
LLVM_UNLIKELY(OptAllFatal)) {
4143
IsBroken = true;
42-
if (OptTerminateOnFirstError)
44+
if (OptTerminationPolicy == Terminate::OnFirstError)
4345
terminate();
4446
}
4547
return false;
@@ -51,7 +53,7 @@ bool GenXVerify::ensure(const bool Cond, const Twine &Msg, const Instruction &I,
5153

5254
bool GenXVerify::runOnModule(Module &M) {
5355
visit(M);
54-
if (OptTerminate && IsBroken)
56+
if (OptTerminationPolicy != Terminate::No && IsBroken)
5557
terminate();
5658
return false;
5759
}

IGC/VectorCompiler/lib/GenXCodeGen/GenXVerify.h

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,52 @@ class GenXVerify : public ModulePass,
3131
public IDMixin<GenXVerify>,
3232
public InstVisitor<GenXVerify> {
3333
private:
34-
static inline cl::opt<bool> OptTerminateOnFirstError{
35-
"genx-verify-terminate-on-first-error", cl::init(false), cl::Hidden,
36-
cl::desc("Terminate execution on first error found.")};
34+
enum class Terminate { No = 0, Yes, OnFirstError };
35+
static_assert(Terminate::No < Terminate::Yes &&
36+
Terminate::Yes < Terminate::OnFirstError);
3737

38-
static inline cl::opt<bool> OptTerminate{
39-
"genx-verify-terminate", cl::init(true), cl::Hidden,
40-
cl::desc(
41-
"Terminate execution after pass completion if any errors found.")};
38+
static inline cl::opt<GenXVerifyInvariantSet> OptInvSet{
39+
"genx-verify-set",
40+
cl::desc("Check for this pipeline-dependent invariant set."),
41+
cl::init(GenXVerifyInvariantSet::Default_), cl::NotHidden,
42+
cl::values(
43+
clEnumValN(GenXVerifyInvariantSet::PostSPIRVReader,
44+
"post-spirv-reader",
45+
"Checks valid for early stage of IR acquired right after "
46+
"SPIRV->LLVM translation."),
47+
clEnumValN(GenXVerifyInvariantSet::PostIrAdaptors, "post-ir-adaptors",
48+
"Checks valid after IR adaptors run."),
49+
clEnumValN(GenXVerifyInvariantSet::PostGenXLowering,
50+
"post-genx-lowering",
51+
"Checks valid after GenXLowering pass."),
52+
clEnumValN(GenXVerifyInvariantSet::PostGenXLegalization,
53+
"post-genx-legalization",
54+
"Checks valid after GenXLegalization pass."),
55+
clEnumValN(
56+
GenXVerifyInvariantSet::Default_, "default",
57+
"Default checks set (must be a checks set used for the late "
58+
"pipeline stages)"))};
59+
60+
static inline cl::opt<Terminate> OptTerminationPolicy{
61+
"genx-verify-terminate", cl::desc("Execution termination policy."),
62+
cl::init(Terminate::Yes), cl::NotHidden,
63+
cl::values(
64+
clEnumValN(Terminate::No, "no",
65+
"Do not terminate if error(s) found."),
66+
clEnumValN(Terminate::Yes, "yes",
67+
"Terminate after pass completion if any errors found."),
68+
clEnumValN(Terminate::OnFirstError, "first",
69+
"Terminate on first error found."))};
4270

4371
static inline cl::opt<bool> OptAllFatal{
44-
"genx-verify-all-fatal", cl::init(false), cl::Hidden,
72+
"genx-verify-all-fatal", cl::init(false), cl::NotHidden,
4573
cl::desc("Ignore IsFatal::No flag used for assetions requiring spec "
4674
"clarification, making all of the checks fatal on failure.")};
4775

76+
static inline cl::opt<bool> OptQuietNonFatal{
77+
"genx-verify-quiet-non-fatal", cl::init(true), cl::NotHidden,
78+
cl::desc("Do not display non-fatal warnings.")};
79+
4880
static inline const StringRef DbgPrefix = "GenXVerify";
4981
LLVMContext *Ctx;
5082
bool IsBroken = false;
@@ -57,7 +89,7 @@ class GenXVerify : public ModulePass,
5789
[[noreturn]] static void terminate();
5890

5991
public:
60-
explicit GenXVerify(GenXVerifyInvariantSet IS = GenXVerifyInvariantSet::All)
92+
explicit GenXVerify(GenXVerifyInvariantSet IS = OptInvSet)
6193
: InvariantSet(IS), ModulePass(ID) {}
6294
StringRef getPassName() const override;
6395
void getAnalysisUsage(AnalysisUsage &) const override;

IGC/VectorCompiler/lib/GenXCodeGen/GenXVerify_Regioning.cpp

Lines changed: 57 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,18 @@ void GenXVerify::verifyRegioning(const CallInst &CI,
2323
auto ensureNumEls4_8_16_and_offsetIsAMultipleOf =
2424
[&](const auto NumElts, const Twine NumEltsArgDesc,
2525
unsigned OffsetOpndNum) -> void {
26-
if (InvariantSet <= GenXVerifyInvariantSet::PreGenXLegalization)
26+
if (InvariantSet < GenXVerifyInvariantSet::PostGenXLegalization)
27+
return;
28+
if (!ensure(NumElts == 4 || NumElts == 8 || NumElts == 16,
29+
NumEltsArgDesc + " must be 4, 8 or 16", CI))
2730
return;
28-
ensure(NumElts == 4 || NumElts == 8 || NumElts == 16,
29-
NumEltsArgDesc + " must be 4, 8 or 16", CI);
3031
const auto *OffsetInElementsConstInt =
3132
llvm::dyn_cast<llvm::ConstantInt>(CI.getOperand(OffsetOpndNum));
32-
ensure(OffsetInElementsConstInt,
33-
"OffsetInElements must be a constant integer", CI);
33+
if (!ensure(OffsetInElementsConstInt,
34+
"offset in elements must be a constant integer", CI))
35+
return;
3436
ensure(!(OffsetInElementsConstInt->getZExtValue() % NumElts),
35-
"OffsetInElements must be multiple of the number of elements of " +
37+
"offset in elements must be multiple of the number of elements of " +
3638
NumEltsArgDesc,
3739
CI);
3840
};
@@ -58,30 +60,50 @@ void GenXVerify::verifyRegioning(const CallInst &CI,
5860
if (!ensure(IGCLLVM::getNumArgOperands(&CI) == 2,
5961
"rdpredregion* intrinsics must have 2 operands", CI))
6062
return;
61-
const auto *RetT = dyn_cast<IGCLLVM::FixedVectorType>(
63+
const auto *RetVT = dyn_cast<IGCLLVM::FixedVectorType>(
6264
CI.getCalledFunction()->getReturnType());
63-
ensure(RetT, "return type must be a vector", CI);
64-
ensureNumEls4_8_16_and_offsetIsAMultipleOf(RetT->getNumElements(),
65+
if (!ensure(RetVT, "return type must be a vector", CI))
66+
return;
67+
ensureNumEls4_8_16_and_offsetIsAMultipleOf(RetVT->getNumElements(),
6568
"returned vector", 1);
6669
}
6770
return;
68-
case GenXIntrinsic::genx_wrpredpredregion:
71+
case GenXIntrinsic::genx_wrpredpredregion: {
6972
if (!ensure(IGCLLVM::getNumArgOperands(&CI) == 4,
7073
"wrpredpredregion* intrinsics must have 4 operands", CI))
7174
return;
75+
76+
// The constant offset indexes both the vector itself and the predicate.
77+
// This intrinsic is valid only if the predicate is an EM value, and the
78+
// subvector operand is the result of a cmp (which is then baled in).
7279
ensure(isa<CmpInst>(CI.getOperand(1)),
7380
"subvector to write must be the direct result of a cmp instruction",
7481
CI);
82+
83+
const auto *SubvectorToWriteT =
84+
dyn_cast<IGCLLVM::FixedVectorType>(CI.getOperand(1)->getType());
85+
86+
if (!ensure(SubvectorToWriteT, "subvector to write must be a fixed vector",
87+
CI))
88+
return;
89+
ensureNumEls4_8_16_and_offsetIsAMultipleOf(
90+
SubvectorToWriteT->getNumElements(), "subvector to write", 2);
91+
7592
// TODO: This intrinsic is valid only if the predicate is an EM value
7693
// CI.getOperand(3)
77-
LLVM_FALLTHROUGH;
94+
}
95+
return;
7896
case GenXIntrinsic::genx_wrpredregion: {
7997
if (!ensure(IGCLLVM::getNumArgOperands(&CI) == 3,
8098
"wrpredregion* intrinsics must have 3 operands", CI))
8199
return;
100+
82101
const auto *SubvectorToWriteT =
83102
dyn_cast<IGCLLVM::FixedVectorType>(CI.getOperand(1)->getType());
84-
ensure(SubvectorToWriteT, "subvector to write must be a fixed vector", CI);
103+
104+
if (!ensure(SubvectorToWriteT, "subvector to write must be a fixed vector",
105+
CI))
106+
return;
85107
ensureNumEls4_8_16_and_offsetIsAMultipleOf(
86108
SubvectorToWriteT->getNumElements(), "subvector to write", 2);
87109
}
@@ -134,15 +156,14 @@ void GenXVerify::verifyRegioning(const CallInst &CI,
134156

135157
const llvm::Value *WidthOpnd = CI.getOperand(SrcOpndIdx + 2);
136158
const auto WidthConstantInt = dyn_cast<llvm::ConstantInt>(WidthOpnd);
137-
ensure(WidthConstantInt, "width must be a constant int", CI);
138159

139-
const unsigned Width =
140-
WidthConstantInt ? WidthConstantInt->getZExtValue()
141-
: 1 /*a value making following checks happy (for
142-
non-failing LIT tests runs). If width was not a
143-
constant, we have already reported above.*/
144-
;
145-
ensure(Width, "the width must be non-zero.", CI);
160+
if (!ensure(WidthConstantInt, "width must be a constant int", CI))
161+
return;
162+
163+
const unsigned Width = WidthConstantInt->getZExtValue();
164+
165+
if (!ensure(Width, "the width must be non-zero.", CI))
166+
return;
146167

147168
const unsigned TotalElCount_ExecSize =
148169
GenXIntrinsic::isRdRegion(IntrinsicId)
@@ -198,6 +219,12 @@ void GenXVerify::verifyRegioning(const CallInst &CI,
198219
const auto *DstSrcOpndT = DstSrcOpnd->getType();
199220
const auto *DstSrcOpndMaybeVT =
200221
dyn_cast<IGCLLVM::FixedVectorType>(DstSrcOpndT);
222+
223+
// TODO:spec review: some IRs have arg0 as a scalar with undef value.
224+
ensure(DstSrcOpndMaybeVT,
225+
"destination-source (arg0) must be a fixed vector.", CI,
226+
IsFatal::No);
227+
201228
// TODO:spec review: some IRs have arg0 as a scalar with undef value.
202229
ensure(DstSrcOpndMaybeVT,
203230
"source-destination (arg0) operand must be a fixed vector.", CI,
@@ -207,26 +234,22 @@ void GenXVerify::verifyRegioning(const CallInst &CI,
207234
"The arg1 subvector must have the same element type as the arg0 vector",
208235
CI);
209236

210-
if (InvariantSet > GenXVerifyInvariantSet::PreIrAdaptors) {
211-
// TODO:spec review: some IRs have arg0 as a scalar with undef value.
237+
if (InvariantSet >= GenXVerifyInvariantSet::PostIrAdaptors) {
212238
if (SrcOpndMaybeVT && DstSrcOpndMaybeVT)
213239
ensure(SrcOpndMaybeVT->getNumElements() <=
214240
DstSrcOpndMaybeVT->getNumElements(),
215-
"The arg1 subvector must be no lardger than arg0 vector", CI);
216-
}
241+
"The arg1 subvector must be no larger than arg0 vector", CI);
217242

218-
// After lowering, the arg1 subvector to write can be a scalar of the
219-
// same type as an element of arg0, indicating that the region has one
220-
// element. (Lowering lowers an insertelement to this type of wrregion.)
221-
if (InvariantSet > GenXVerifyInvariantSet::PreIrAdaptors) {
222-
ensure(SrcOpndMaybeVT || (!SrcOpndMaybeVT && Width == 1),
223-
"subregion to write may be a scalar if the number of elements in "
243+
// After lowering, the arg1 subvector to write can be a scalar of the
244+
// same type as an element of arg0, indicating that the region has one
245+
// element. (Lowering lowers an insertelement to this type of wrregion.)
246+
ensure(SrcOpndMaybeVT ||
247+
(InvariantSet >= GenXVerifyInvariantSet::PostGenXLowering &&
248+
Width == 1),
249+
"after genx lowering subregion to write may be a scalar if the "
250+
"number of elements in "
224251
"subregion is 1.",
225252
CI);
226-
// TODO:spec review: some IRs have arg0 as a scalar with undef value.
227-
ensure(DstSrcOpndMaybeVT,
228-
"destination-source (arg0) must be a fixed vector.", CI,
229-
IsFatal::No);
230253
}
231254

232255
ensure(SrcOpndMaybeVT || SrcOpndT->getScalarType(),

0 commit comments

Comments
 (0)