Skip to content

Commit dbb9c97

Browse files
authored
Fix auto-dxilver logic in FileCheckerTest (microsoft#6368)
In FileCheckerTest.cpp, the -T target of a %dxc part is checked for compatibility. However, if you use -Vd, it skips the version check for the validator. This isn't quite right since the validator version will be set from the attached validator, unless you explicitly set -validator-version or use -select-validator internal. Also, if -validator-version is explicitly set, we should check the validator against that version, not just the one based on shader model. This change fixes the initial check logic to only skip validator check when using -select-validator internal or -validator-version. It also checks against an explicitly requested -validator-version when an external validator will be used. Fixes microsoft#6367
1 parent a932cf4 commit dbb9c97

File tree

2 files changed

+51
-3
lines changed

2 files changed

+51
-3
lines changed
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Make sure each run line either works, or implicitly requires dxilver 1.8.
2+
// If an implicit dxilver check is missing, we should see one of these errors.
3+
4+
// RUN: %dxc -T vs_6_8 %s | FileCheck %s
5+
// This should implicitly require dxilver 1.8.
6+
7+
// RUN: %dxc -T vs_6_8 -Vd %s | FileCheck %s
8+
// Even though this is using -Vd, the validator version is set by the available
9+
// validator. If that isn't version 1.8 or above, we'll see an error.
10+
// The implicit dxilver logic should not skip the check when -Vd is used.
11+
// CHECK-NOT: error: validator version {{.*}} does not support target profile.
12+
13+
// RUN: %dxc -T vs_6_0 -validator-version 1.8 %s | FileCheck %s
14+
// Even though target is 6.0, the explicit -validator-version should add an
15+
// implicit dxilver 1.8 requirement.
16+
// CHECK-NOT: error: The module cannot be validated by the version of the validator currently attached.
17+
18+
// This error would occur if run against wrong compiler.
19+
// CHECK-NOT: error: invalid profile
20+
21+
// Catch any other unexpected error cases.
22+
// CHECK-NOT: error
23+
24+
// RUN: %dxc -T vs_6_8 -select-validator internal %s | FileCheck %s
25+
// This should always be run, and always succeed.
26+
// CHECK: define void @main()
27+
28+
void main() {}

tools/clang/unittests/HLSLTestLib/FileCheckerTest.cpp

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -518,12 +518,32 @@ FileRunCommandPart::RunDxc(dxc::DxcDllSupport &DllSupport,
518518
if (RequiredDxilMinor != 0xF && stage.compare("rootsig") != 0) {
519519
// Convert stage to minimum dxil/validator version:
520520
RequiredDxilMajor = std::max(RequiredDxilMajor, (unsigned)6) - 5;
521+
522+
bool bInternalValidator =
523+
opts.SelectValidator == hlsl::options::ValidatorSelection::Internal;
524+
bool bValVerExplicit = opts.ValVerMajor != UINT_MAX;
525+
526+
// Normally we must check the validator version as well, but there are
527+
// two scenarios where the validator version doesn't need to be checked
528+
// against the version based on the shader model:
529+
// 1. The test selects internal validator.
530+
// 2. The test explicitly requests a specific validator version.
521531
FileRunCommandResult result =
522532
CheckDxilVer(DllSupport, RequiredDxilMajor, RequiredDxilMinor,
523-
!opts.DisableValidation);
524-
if (result.AbortPipeline) {
533+
!(bInternalValidator || bValVerExplicit));
534+
if (result.AbortPipeline)
535+
return result;
536+
537+
// Additionally, if the test explicitly requests a specific non-zero
538+
// validator version, and doesn't select internal validator or disable
539+
// validation, we must check that the validator version is at least as
540+
// high as the requested version.
541+
// When ValVerMajor is 0, validation cannot be run against the module.
542+
if (bValVerExplicit && opts.ValVerMajor != 0 &&
543+
!(bInternalValidator || opts.DisableValidation))
544+
result = CheckDxilVer(DllSupport, opts.ValVerMajor, opts.ValVerMinor);
545+
if (result.AbortPipeline)
525546
return result;
526-
}
527547
}
528548
}
529549

0 commit comments

Comments
 (0)