search

microsoft / DirectXShaderCompiler

This repo hosts the source for the DirectX Shader Compiler which is based on LLVM/Clang.
Other
3.08k stars 689 forks source link

[Feature Request] Hidden option -select-validator not shown in help #6324

Open pmkippes opened 8 months ago

pmkippes commented 8 months ago

If the dxil.dll is found in the path, the external validator will be defaulted. This creates a problem when compiling SM 6.8 HLSL as it fails to verify. The user would need to provide -select-validator internal to successfully compile.

Version fafbc4272540c09cd070ca8a7a8dcda2788bc108 of main and 476d34409411d06285097e611d933b962912507a of release-1.8.2403

Example commands:

> ..\hlsl.bin\Debug\bin\dxc.exe -T lib_6_8 -Vd tools/clang/test/HLSLFileCheck/hlsl/workgraph/wavesize.hlsl
[IR omitted, PASSED]

> set PATH=C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64;%PATH%

> ..\hlsl.bin\Debug\bin\dxc.exe -T lib_6_8 -Vd tools/clang/test/HLSLFileCheck/hlsl/workgraph/wavesize.hlsl
error: validator version 1,6 does not support target profile.

> ..\hlsl.bin\Debug\bin\dxc.exe -select-validator internal -T lib_6_8 -Vd tools/clang/test/HLSLFileCheck/hlsl/workgraph/wavesize.hlsl
[IR omitted, PASSED]

Listing this option in -help is suggested.

tex3d commented 8 months ago

This should only be used when compiling to a pre-release shader model. Selecting the internal validator won't sign the shader, and it will require developer mode with experimental shader models turned on to use the shaders with D3D12.

Another point is that -Vd should not be required, with -select-validator internal, or for the final release with the updated DXIL.dll. If -Vd is required for it to successfully compile in either of these scenarios, then illegal DXIL is being produced by the compiler, or the validator is failing something it shouldn't. Illegal DXIL may be produced by using something in a way the compiler is unable to make legal for the shader target, or it could be a bug in the compiler. If it's a bug in the compiler or validator, we'd like to have an issue to track it so we can try to fix it. It's much worse for people to "work around" these issues by using -Vd. If it's something the compiler is unable to make legal for the target, the HLSL code pattern needs to be changed, or the compiler might need to be updated to handle something it currently does not (another issue we want to know about).

In all of these scenarios using -Vd just silences a real issue that needs to be addressed, preferably before release.

pmkippes commented 8 months ago

This request was in the context of supporting SM6.8. With the target profile I provided in the example, I'm wanting the new WaveSize Range MD node to be created, but the versions of dxil.dll I have access to was kicking the shader out for the reasons given. Forcing the validator to the internal one seemed the best solution since -Vd wasn't permitting the IR to be generated.

It is sufficient that the --select validator internal option exists and is available. Feel free to close this PR if preference is to not document option in the command help list.

llvm-beanz commented 8 months ago

In the short term I think we're going to keep things as they are. I'm not going to close this issue right away though because we need to have some larger discussions about the validation infrastructure and how it will be used and maintained by Clang.