Skip to content

Commit e7346ac

Browse files
tex3dtex3d
authored
Fix dxc version printing and fix external validator testing (#8075)
Version print didn't reveal details from available version interfaces, nor did it print file version for DXIL.dll when available. Changes: - Clean up code a bit - No longer skip QI for IDxcVersionInfo2 based on build, since that's for implementer, not user. - Print CustomVersionString from IDxcVersionInfo3 if available. - Remove ifdefs for WIN32 around file/product version calls, because that's handled in the function. - Only print product version string if available for dxil.dll, since that's what it does for dxcompiler.dll. - Removes old dev build detection (separate commit in PR so it's easy to reverse if desired) - Fixes unconditional print of '(dev' as if it's an old dev build on non-Win32 targets - Implement IDxcVersionInfo(2,3) in the IDxcCompiler wrapper interface, so it doesn't look like version 1.0 - Only print version information for external validator when selected - Otherwise, don't print info for a validator we found that won't be used - Fix dxc version parsing in lit.cfg - Add built external validator testing check-extdxil - Select the only clang/test subdirs that could even test the external validator - Skip LitDXILValidation tests when testing compiler output using released validators
1 parent 1866977 commit e7346ac

14 files changed

Lines changed: 224 additions & 106 deletions

File tree

‎azure-pipelines.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ stages:
4646
call utils\hct\hctstart.cmd %HLSL_SRC_DIR% %HLSL_BLD_DIR%
4747
call utils\hct\hcttest.cmd -$(configuration) exec
4848
displayName: 'DXIL Execution Tests'
49+
- script: |
50+
call utils\hct\hctstart.cmd %HLSL_SRC_DIR% %HLSL_BLD_DIR%
51+
call utils\hct\hcttest.cmd -$(configuration) extdxil
52+
displayName: 'External DXIL Validator Tests'
53+
condition: succeededOrFailed()
4954
- script: |
5055
call utils\hct\hctstart.cmd %HLSL_SRC_DIR% %HLSL_BLD_DIR%
5156
call utils\hct\hcttest.cmd -$(configuration) compat-suite v1.6.2112
@@ -165,6 +170,8 @@ stages:
165170
displayName: 'Smoke tests'
166171
- bash: $(CHECK_ALL_ENV) ninja -C build check-all
167172
displayName: 'DXC tests'
173+
- bash: $(CHECK_ALL_ENV) ninja -C build check-extdxil
174+
displayName: 'External DXIL Validator tests'
168175
- task: PublishTestResults@2
169176
inputs:
170177
testResultsFormat: 'JUnit'

‎lib/DxcSupport/dxcapi.extval.cpp‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,10 @@ class ExtValidationArgHelper {
171171
// '-Vd' and sets the default validator version with '-validator-version'.
172172
// After a successful compilation, it uses the provided IDxcValidator to
173173
// perform validation when it would normally be performed.
174-
class ExternalValidationCompiler : public IDxcCompiler2, public IDxcCompiler3 {
174+
class ExternalValidationCompiler : public IDxcCompiler2,
175+
public IDxcCompiler3,
176+
public IDxcVersionInfo2,
177+
public IDxcVersionInfo3 {
175178
public:
176179
ExternalValidationCompiler(IMalloc *Malloc, IDxcValidator *OtherValidator,
177180
IUnknown *OtherCompiler)
@@ -203,8 +206,10 @@ class ExternalValidationCompiler : public IDxcCompiler2, public IDxcCompiler3 {
203206
// calls.
204207
CComPtr<ExternalValidationCompiler> NewWrapper(
205208
Alloc(m_pMalloc, Validator, TempCompiler));
206-
return DoBasicQueryInterface<IDxcCompiler, IDxcCompiler2, IDxcCompiler3>(
207-
NewWrapper.p, Iid, ResultObject);
209+
return DoBasicQueryInterface<IDxcCompiler, IDxcCompiler2, IDxcCompiler3,
210+
IDxcVersionInfo, IDxcVersionInfo2,
211+
IDxcVersionInfo3>(NewWrapper.p, Iid,
212+
ResultObject);
208213
} catch (...) {
209214
return E_FAIL;
210215
}
@@ -305,6 +310,28 @@ class ExternalValidationCompiler : public IDxcCompiler2, public IDxcCompiler3 {
305310
return cast<IDxcCompiler3>()->Disassemble(Object, Riid, ResultObject);
306311
}
307312

313+
// IDxcVersionInfo implementation
314+
HRESULT STDMETHODCALLTYPE GetVersion(_Out_ UINT32 *pMajor,
315+
_Out_ UINT32 *pMinor) override {
316+
return cast<IDxcVersionInfo>()->GetVersion(pMajor, pMinor);
317+
}
318+
HRESULT STDMETHODCALLTYPE GetFlags(_Out_ UINT32 *pFlags) override {
319+
return cast<IDxcVersionInfo>()->GetFlags(pFlags);
320+
}
321+
322+
// IDxcVersionInfo2 implementation
323+
HRESULT STDMETHODCALLTYPE
324+
GetCommitInfo(_Out_ UINT32 *pCommitCount,
325+
_Outptr_result_z_ char **pCommitHash) override {
326+
return cast<IDxcVersionInfo2>()->GetCommitInfo(pCommitCount, pCommitHash);
327+
}
328+
329+
// IDxcVersionInfo3 implementation
330+
HRESULT STDMETHODCALLTYPE
331+
GetCustomVersionString(_Outptr_result_z_ char **pVersionString) override {
332+
return cast<IDxcVersionInfo3>()->GetCustomVersionString(pVersionString);
333+
}
334+
308335
private:
309336
CComPtr<IDxcValidator> Validator;
310337

‎tools/clang/test/CMakeLists.txt‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -179,21 +179,25 @@ foreach(i RANGE 0 ${loop_end})
179179
# Path to dxil.dll for this release
180180
set(DXC_PATH ${CMAKE_BINARY_DIR}/tools/clang/test/dxc_releases/${version}/${name}/bin/${TARGET_ARCH}/dxil.dll)
181181

182+
# The only sub-dirs that make sense to run with the released external dxil
183+
# validator setting are CodeGenDXIL, DXC, and HLSLFileCheckLit.
184+
# It would be much better to run Unit/TAEF tests too, but those tests don't
185+
# currently respect the DXC_DXIL_DLL_PATH environment variable.
186+
# LitDXILValidation is also skipped, since it is a regression test for the
187+
# latest validator only.
188+
182189
# Create a lit target for this release
183-
add_lit_target("check-dxilcompat-${name}" "Running clang regression tests with dxil validator from ${name}\n"
184-
${CMAKE_CURRENT_SOURCE_DIR}
190+
add_lit_target("check-dxilcompat-${name}"
191+
"Running clang regression tests with dxil validator from ${name}"
192+
${CMAKE_CURRENT_SOURCE_DIR}/CodeGenDXIL
193+
${CMAKE_CURRENT_SOURCE_DIR}/DXC
194+
${CMAKE_CURRENT_SOURCE_DIR}/HLSLFileCheckLit
185195
PARAMS ${CLANG_TEST_PARAMS}
186196
DXC_DXIL_DLL_PATH=${DXC_PATH}
187197
DXC_RELEASE_VERSION_TAG=${version}
188-
skip_taef_exec=True
189198
DEPENDS ${CLANG_TEST_DEPS} ${name}
190199
ARGS ${CLANG_TEST_EXTRA_ARGS}
191200
)
192-
193-
# Hook into check-all
194-
if (WIN32 AND TARGET check-all)
195-
add_dependencies(check-all "check-dxilcompat-${name}")
196-
endif()
197201
endif()
198202
endforeach()
199203
# -------------------------------------------------------------------------
@@ -248,4 +252,27 @@ if(CMAKE_CONFIGURATION_TYPES)
248252
endif()
249253
endif()
250254

255+
# -------------------------------------------------------------------------
256+
# External Validator Testing
257+
# -------------------------------------------------------------------------
258+
259+
# The only sub-dirs that make sense to run with the external dxil validator
260+
# setting are CodeGenDXIL, DXC, HLSLFileCheckLit, and LitDXILValidation.
261+
# It would be much better to run Unit/TAEF tests too, but those tests don't
262+
# currently respect the DXC_DXIL_DLL_PATH environment variable.
263+
264+
add_lit_target("check-extdxil"
265+
"Running clang regression tests with built external dxil validator"
266+
${CMAKE_CURRENT_SOURCE_DIR}/CodeGenDXIL
267+
${CMAKE_CURRENT_SOURCE_DIR}/DXC
268+
${CMAKE_CURRENT_SOURCE_DIR}/HLSLFileCheckLit
269+
${CMAKE_CURRENT_SOURCE_DIR}/LitDXILValidation
270+
PARAMS ${CLANG_TEST_PARAMS}
271+
# Set path to external validator build target
272+
"DXC_DXIL_DLL_PATH=$<TARGET_FILE:dxildll>"
273+
DEPENDS ${CLANG_TEST_DEPS}
274+
ARGS ${CLANG_TEST_EXTRA_ARGS}
275+
)
276+
# -------------------------------------------------------------------------
277+
251278
# HLSL Change End

‎tools/clang/test/CodeGenSPIRV/lit.local.cfg‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
config.suffixes = ['.hlsl', '.hlsl2spv']
2-
# Skip when SPIR-V disabled or during down-level DXIL testing
3-
if not config.spirv or "dxc_dxil_dll_path" in config.available_features:
2+
# Skip when SPIR-V is disabled.
3+
if not config.spirv:
44
config.unsupported = True

tools/clang/test/LitDXILValidation/Environment.test renamed to tools/clang/test/DXC/Environment.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
# REQUIRES: dxc_dxil_dll_path
22
# RUN: python -c "import os; print(os.environ['DXC_DXIL_DLL_PATH'])" | FileCheck %s
3-
# CHECK: dxil.dll
3+
# CHECK: dxil.{{dll|so|dylib}}

‎tools/clang/test/DXC/Passes/lit.local.cfg‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
# Skip during down-level DXIL testing
1+
# Skip during external DXIL validator testing
22
config.unsupported = "dxc_dxil_dll_path" in config.available_features

tools/clang/test/LitDXILValidation/validate_1_6_2112.hlsl renamed to tools/clang/test/DXC/validate_1_6_2112.test

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
// REQUIRES: dxilval-v1.6.2112
22

3+
// Verify that the version info is correctly reported when selecting external validator.
4+
// RUN: dxc --version | FileCheck %s --check-prefix=VERSION
5+
// VERSION: dxil.{{dll|so|dylib}}: 1.6
6+
37
// Verify that the older DLL is being loaded, and that
48
// due to it being older, it errors on something that would
59
// be accepted in newer validators.
610
// Specifically, the v1.6.2112 validator would emit the
711
// below validation error, while validators newer than 1.6
812
// would be able to validate the module.
9-
10-
// RUN: not dxc -T cs_6_0 -validator-version 1.7 %s 2>&1 | FileCheck %s
13+
// RUN: not dxc /T ps_6_0 %S/Inputs/smoke.hlsl -validator-version 1.7 2>&1 | FileCheck %s
1114
// CHECK: error: Validator version in metadata (1.7) is not supported; maximum: (1.6).
12-
[numthreads(1, 1, 1)]
13-
void main() {}

‎tools/clang/test/DXC/validate_1_7_2308.test‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// REQUIRES: dxilval-v1.7.2308
2+
3+
// Verify that the version info is correctly reported when selecting external validator.
4+
// RUN: dxc --version | FileCheck %s --check-prefix=VERSION
5+
// VERSION: dxil.{{dll|so|dylib}}: 1.7
6+
7+
// Verify that the older DLL is being loaded, and that
8+
// due to it being older, it errors on something that would
9+
// be accepted in newer validators.
10+
// Specifically, the v1.7.2308 validator would emit the
11+
// below validation error, while validators newer than 1.7
12+
// would be able to validate the module.
13+
// RUN: not dxc /T ps_6_0 %S/Inputs/smoke.hlsl -validator-version 1.8 2>&1 | FileCheck %s
14+
// CHECK: error: Validator version in metadata (1.8) is not supported; maximum: (1.7).

‎tools/clang/test/DXC/validate_1_8_2502.test‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// REQUIRES: dxilval-v1.8.2502
2+
3+
// Verify that the version info is correctly reported when selecting external validator.
4+
// RUN: dxc --version | FileCheck %s --check-prefix=VERSION
5+
// VERSION: dxil.{{dll|so|dylib}}: 1.8
6+
7+
// Verify that the older DLL is being loaded, and that
8+
// due to it being older, it errors on something that would
9+
// be accepted in newer validators.
10+
// Specifically, the v1.8.2502 validator would emit the
11+
// below validation error, while validators newer than 1.8
12+
// would be able to validate the module.
13+
// RUN: not dxc /T ps_6_0 %S/Inputs/smoke.hlsl -validator-version 1.9 2>&1 | FileCheck %s
14+
// CHECK: error: Validator version in metadata (1.9) is not supported; maximum: (1.8).

‎tools/clang/test/DXC/version_interface.test‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// RUN: %dxc --version | FileCheck %s
2+
3+
// Verify that the external validator can support the
4+
// IDxcVersionInfo* interfaces, and can accurately report
5+
// that the dxcompiler.dll version is not 1.0.
6+
// The version is 1.0 only when the DxcVersionInfo interface
7+
// is unsupported. Otherwise, it should usually match the
8+
// dxil version or it will be at least 1.1
9+
// CHECK-NOT: dxcompiler.{{dll|so|dylib}}: 1.0

0 commit comments

Comments
 (0)