Skip to content

Commit 4e5d61e

Browse files
alsepkowCopilot
alsepkow
and
Copilot
authored
Make PSV/DebugBreak unit tests robust to validator version (#8625)
The PSV and DebugBreak unit tests hard-code the newest PSV runtime-info version, which is a latent fragility unrelated to any single release. - `ValidationTest` PSV tests hard-coded `sizeof(PSVRuntimeInfo4)` (and the literal string `56`). That silently assumes the validator always emits the newest PSV struct. It is wrong for any validator `< 1.10` (which emits `PSVRuntimeInfo3`), including downlevel/external-validator runs and any future shader-model cap. The expected size is now derived from the validator version via a `GetExpectedPSVRuntimeInfoSize()` helper. - `PixTest::DebugBreakInstrumentation_{Basic,Multiple}` hard-code `cs_6_10`; they now skip when Dxil/Validator `< 1.10`. This is behavior-preserving on `main` (validator is 1.10 there, so PSV v4 is still expected and the tests still run), while removing the fragility. The latent bug was introduced in #7871, which added `PSVRuntimeInfo4` and the `< 1.10 -> v3` gating together; main has always run validator 1.10 since, so the `v3` path was never exercised. Intended to be cherry-picked to `release-1.9.2607` (where #8618 caps the validator to 1.9), unblocking the SM 6.9 cap. Assisted by co-pilot --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c2f95cc commit 4e5d61e

2 files changed

Lines changed: 42 additions & 24 deletions

File tree

‎tools/clang/unittests/HLSL/PixTest.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3389,6 +3389,8 @@ void RaygenInternalName()
33893389
}
33903390

33913391
TEST_F(PixTest, DebugBreakInstrumentation_Basic) {
3392+
if (m_ver.SkipDxilVersion(1, 10))
3393+
return;
33923394

33933395
const char *source = R"x(
33943396
[numthreads(1, 1, 1)]
@@ -3426,6 +3428,8 @@ void main() {
34263428
}
34273429

34283430
TEST_F(PixTest, DebugBreakInstrumentation_Multiple) {
3431+
if (m_ver.SkipDxilVersion(1, 10))
3432+
return;
34293433

34303434
const char *source = R"x(
34313435
RWByteAddressBuffer buf : register(u0);

‎tools/clang/unittests/HLSL/ValidationTest.cpp‎

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4882,6 +4882,15 @@ TEST_F(ValidationTest, CacheInitWithLowPrec) {
48824882
TestCheck(L"..\\DXILValidation\\val-dx-type-lowprec.ll");
48834883
}
48844884

4885+
// PSVRuntimeInfo4 adds NumBytesGroupSharedMemory and is only emitted for
4886+
// validator version >= 1.10; earlier validators emit PSVRuntimeInfo3.
4887+
static uint32_t GetExpectedPSVRuntimeInfoSize(const VersionSupportInfo &ver) {
4888+
bool HasV4 =
4889+
ver.m_ValMajor > 1 || (ver.m_ValMajor == 1 && ver.m_ValMinor >= 10);
4890+
return HasV4 ? static_cast<uint32_t>(sizeof(PSVRuntimeInfo4))
4891+
: static_cast<uint32_t>(sizeof(PSVRuntimeInfo3));
4892+
}
4893+
48854894
TEST_F(ValidationTest, PSVStringTableReorder) {
48864895
if (!m_ver.m_InternalValidator)
48874896
if (m_ver.SkipDxilVersion(1, 8))
@@ -4916,7 +4925,7 @@ TEST_F(ValidationTest, PSVStringTableReorder) {
49164925
const uint32_t *PSVPtr = (const uint32_t *)GetDxilPartData(pPSVPart);
49174926

49184927
uint32_t PSVRuntimeInfo_size = *(PSVPtr++);
4919-
VERIFY_ARE_EQUAL(sizeof(PSVRuntimeInfo4), PSVRuntimeInfo_size);
4928+
VERIFY_ARE_EQUAL(GetExpectedPSVRuntimeInfoSize(m_ver), PSVRuntimeInfo_size);
49204929
PSVRuntimeInfo4 *PSVInfo =
49214930
const_cast<PSVRuntimeInfo4 *>((const PSVRuntimeInfo4 *)PSVPtr);
49224931
VERIFY_ARE_EQUAL(2u, PSVInfo->SigInputElements);
@@ -5108,7 +5117,7 @@ TEST_F(ValidationTest, PSVSemanticIndexTableReorder) {
51085117
const uint32_t *PSVPtr = (const uint32_t *)GetDxilPartData(pPSVPart);
51095118

51105119
uint32_t PSVRuntimeInfo_size = *(PSVPtr++);
5111-
VERIFY_ARE_EQUAL(sizeof(PSVRuntimeInfo4), PSVRuntimeInfo_size);
5120+
VERIFY_ARE_EQUAL(GetExpectedPSVRuntimeInfoSize(m_ver), PSVRuntimeInfo_size);
51125121
PSVRuntimeInfo4 *PSVInfo =
51135122
const_cast<PSVRuntimeInfo4 *>((const PSVRuntimeInfo4 *)PSVPtr);
51145123
VERIFY_ARE_EQUAL(PSVInfo->SigInputElements, 3u);
@@ -5443,17 +5452,18 @@ struct SimplePSV {
54435452
llvm::MutableArrayRef<uint32_t> PCInputToOutputTable;
54445453
llvm::MutableArrayRef<uint32_t> ViewIDOutputMask[DXIL::kNumOutputStreams];
54455454
llvm::MutableArrayRef<uint32_t> ViewIDPCOutputMask;
5446-
SimplePSV(const DxilPartHeader *pPSVPart);
5455+
SimplePSV(const DxilPartHeader *pPSVPart, uint32_t ExpectedRuntimeInfoSize);
54475456
};
54485457

5449-
SimplePSV::SimplePSV(const DxilPartHeader *pPSVPart) {
5458+
SimplePSV::SimplePSV(const DxilPartHeader *pPSVPart,
5459+
uint32_t ExpectedRuntimeInfoSize) {
54505460
uint32_t PartSize = pPSVPart->PartSize;
54515461
uint32_t *PSVPtr =
54525462
const_cast<uint32_t *>((const uint32_t *)GetDxilPartData(pPSVPart));
54535463
const uint32_t *PSVPtrEnd = PSVPtr + PartSize / 4;
54545464

54555465
uint32_t PSVRuntimeInfoSize = *(PSVPtr++);
5456-
VERIFY_ARE_EQUAL(sizeof(PSVRuntimeInfo4), PSVRuntimeInfoSize);
5466+
VERIFY_ARE_EQUAL(ExpectedRuntimeInfoSize, PSVRuntimeInfoSize);
54575467
PSVRuntimeInfo4 *PSVInfo4 =
54585468
const_cast<PSVRuntimeInfo4 *>((const PSVRuntimeInfo4 *)PSVPtr);
54595469
PSVInfo = PSVInfo4;
@@ -5581,7 +5591,7 @@ TEST_F(ValidationTest, PSVContentValidationVS) {
55815591
VERIFY_ARE_NOT_EQUAL(hlsl::end(pHeader), pPartIter);
55825592

55835593
const DxilPartHeader *pPSVPart = (const DxilPartHeader *)(*pPartIter);
5584-
SimplePSV PSV(pPSVPart);
5594+
SimplePSV PSV(pPSVPart, GetExpectedPSVRuntimeInfoSize(m_ver));
55855595

55865596
// Update PSV.
55875597
PSV.SigInput[0].InterpolationMode = 20;
@@ -5737,7 +5747,7 @@ TEST_F(ValidationTest, PSVContentValidationHS) {
57375747
VERIFY_ARE_NOT_EQUAL(hlsl::end(pHeader), pPartIter);
57385748

57395749
const DxilPartHeader *pPSVPart = (const DxilPartHeader *)(*pPartIter);
5740-
SimplePSV PSV(pPSVPart);
5750+
SimplePSV PSV(pPSVPart, GetExpectedPSVRuntimeInfoSize(m_ver));
57415751

57425752
// Update PSV.
57435753
PSV.SigPatchConstOrPrim[0].InterpolationMode = 20;
@@ -5887,7 +5897,7 @@ TEST_F(ValidationTest, PSVContentValidationDS) {
58875897
VERIFY_ARE_NOT_EQUAL(hlsl::end(pHeader), pPartIter);
58885898

58895899
const DxilPartHeader *pPSVPart = (const DxilPartHeader *)(*pPartIter);
5890-
SimplePSV PSV(pPSVPart);
5900+
SimplePSV PSV(pPSVPart, GetExpectedPSVRuntimeInfoSize(m_ver));
58915901

58925902
// Update PSV.
58935903
PSV.SigPatchConstOrPrim[0].InterpolationMode = 20;
@@ -6044,7 +6054,7 @@ TEST_F(ValidationTest, PSVContentValidationGS) {
60446054
VERIFY_ARE_NOT_EQUAL(hlsl::end(pHeader), pPartIter);
60456055

60466056
const DxilPartHeader *pPSVPart = (const DxilPartHeader *)(*pPartIter);
6047-
SimplePSV PSV(pPSVPart);
6057+
SimplePSV PSV(pPSVPart, GetExpectedPSVRuntimeInfoSize(m_ver));
60486058
// Update PSV.
60496059
PSV.PSVInfo->MaxVertexCount = 2;
60506060

@@ -6132,7 +6142,7 @@ TEST_F(ValidationTest, PSVContentValidationPS) {
61326142
VERIFY_ARE_NOT_EQUAL(hlsl::end(pHeader), pPartIter);
61336143

61346144
const DxilPartHeader *pPSVPart = (const DxilPartHeader *)(*pPartIter);
6135-
SimplePSV PSV(pPSVPart);
6145+
SimplePSV PSV(pPSVPart, GetExpectedPSVRuntimeInfoSize(m_ver));
61366146

61376147
// Update PSV.
61386148
PSV.PSVInfo->PS.DepthOutput = 1;
@@ -6217,7 +6227,7 @@ TEST_F(ValidationTest, PSVContentValidationCS) {
62176227
VERIFY_ARE_NOT_EQUAL(hlsl::end(pHeader), pPartIter);
62186228

62196229
const DxilPartHeader *pPSVPart = (const DxilPartHeader *)(*pPartIter);
6220-
SimplePSV PSV(pPSVPart);
6230+
SimplePSV PSV(pPSVPart, GetExpectedPSVRuntimeInfoSize(m_ver));
62216231
// Update PSV.
62226232
PSV.PSVInfo->NumThreadsX = 1;
62236233

@@ -6299,7 +6309,7 @@ TEST_F(ValidationTest, PSVContentValidationMS) {
62996309
VERIFY_ARE_NOT_EQUAL(hlsl::end(pHeader), pPartIter);
63006310

63016311
const DxilPartHeader *pPSVPart = (const DxilPartHeader *)(*pPartIter);
6302-
SimplePSV PSV(pPSVPart);
6312+
SimplePSV PSV(pPSVPart, GetExpectedPSVRuntimeInfoSize(m_ver));
63036313
// Update PSV.
63046314
memset(PSV.ViewIDOutputMask[0].data(), 0,
63056315
PSV.ViewIDOutputMask[0].size() * sizeof(uint32_t));
@@ -6366,7 +6376,7 @@ TEST_F(ValidationTest, PSVContentValidationAS) {
63666376
VERIFY_ARE_NOT_EQUAL(hlsl::end(pHeader), pPartIter);
63676377

63686378
const DxilPartHeader *pPSVPart = (const DxilPartHeader *)(*pPartIter);
6369-
SimplePSV PSV(pPSVPart);
6379+
SimplePSV PSV(pPSVPart, GetExpectedPSVRuntimeInfoSize(m_ver));
63706380

63716381
// Update PSV.
63726382
PSV.PSVInfo->AS.PayloadSizeInBytes = 0;
@@ -6560,7 +6570,7 @@ TEST_F(ValidationTest, WrongPSVSizeOnZeros) {
65606570
const uint32_t *PSVPtr = (const uint32_t *)GetDxilPartData(pPSVPart);
65616571

65626572
uint32_t PSVRuntimeInfo_size = *(PSVPtr++);
6563-
VERIFY_ARE_EQUAL(sizeof(PSVRuntimeInfo4), PSVRuntimeInfo_size);
6573+
VERIFY_ARE_EQUAL(GetExpectedPSVRuntimeInfoSize(m_ver), PSVRuntimeInfo_size);
65646574
PSVRuntimeInfo4 *PSVInfo =
65656575
const_cast<PSVRuntimeInfo4 *>((const PSVRuntimeInfo4 *)PSVPtr);
65666576
VERIFY_ARE_EQUAL(2u, PSVInfo->SigInputElements);
@@ -6791,11 +6801,14 @@ TEST_F(ValidationTest, WrongPSVVersion) {
67916801
VERIFY_IS_NOT_NULL(p60WithPSV68Result);
67926802
VERIFY_SUCCEEDED(p60WithPSV68Result->GetStatus(&status));
67936803
VERIFY_FAILED(status);
6794-
CheckOperationResultMsgs(
6795-
p60WithPSV68Result,
6796-
{"DXIL container mismatch for 'PSVRuntimeInfoSize' between 'PSV0' "
6797-
"part:('56') and DXIL module:('24')"},
6798-
/*maySucceedAnyway*/ false, /*bRegex*/ false);
6804+
std::string ExpectedPSVSizeStr =
6805+
std::to_string(GetExpectedPSVRuntimeInfoSize(m_ver));
6806+
std::string Msg60WithPSV68 =
6807+
"DXIL container mismatch for 'PSVRuntimeInfoSize' between 'PSV0' "
6808+
"part:('" +
6809+
ExpectedPSVSizeStr + "') and DXIL module:('24')";
6810+
CheckOperationResultMsgs(p60WithPSV68Result, {Msg60WithPSV68.c_str()},
6811+
/*maySucceedAnyway*/ false, /*bRegex*/ false);
67996812

68006813
// Create a new Blob.
68016814
CComPtr<IDxcBlobEncoding> pProgram68WithPSV60;
@@ -6809,9 +6822,10 @@ TEST_F(ValidationTest, WrongPSVVersion) {
68096822
VERIFY_IS_NOT_NULL(p68WithPSV60Result);
68106823
VERIFY_SUCCEEDED(p68WithPSV60Result->GetStatus(&status));
68116824
VERIFY_FAILED(status);
6812-
CheckOperationResultMsgs(
6813-
p68WithPSV60Result,
6814-
{"DXIL container mismatch for 'PSVRuntimeInfoSize' between 'PSV0' "
6815-
"part:('24') and DXIL module:('56')"},
6816-
/*maySucceedAnyway*/ false, /*bRegex*/ false);
6825+
std::string Msg68WithPSV60 =
6826+
"DXIL container mismatch for 'PSVRuntimeInfoSize' between 'PSV0' "
6827+
"part:('24') and DXIL module:('" +
6828+
ExpectedPSVSizeStr + "')";
6829+
CheckOperationResultMsgs(p68WithPSV60Result, {Msg68WithPSV60.c_str()},
6830+
/*maySucceedAnyway*/ false, /*bRegex*/ false);
68176831
}

0 commit comments

Comments
 (0)