microsoft / DirectXShaderCompiler

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

[Feature Request][SPIR-V] Allow removing the `-T xx_Y_Y` flag when building for SPIR-V? #5733

Closed Keenuts closed 2 weeks ago

Keenuts commented 12 months ago

Part 1: removing the ShaderModel version for SPIR-V

DirectX uses ShaderModel to determine some level of hardware support. But there is no clear rule to match ShaderModel to Vulkan/SPIR-V versions.

This can cause the following behavior:

$ dxc ~/shader.hlsl -T cs_6_6 -spirv -fspv-target-env=vulkan1.0
error: Vulkan 1.1 is required for Wave Operation but not permitted to use
  if (WaveIsFirstLane()) {

This can be counter intuitive as I asked for ShaderModel 6.6, but still cannot use this intrinsic because Vulkan doesn't support it. Since this ShaderModel version doesn't bring any value, I'd suggest allowing removal when SPIR-V is requested:

dxc ~/shader.hlsl -T cs -spirv -fspv-target-env=vulkan1.3

Part 2: make -T optional

Now that the version is removed, this flag only tells us if the shader is a compute shader, a library, or anything else. But we do have an attribute for that: [shader("anyhit")].

This attribute is mainly used for raytracing shaders, but why limiting it to those?

This works today:

[shader("compute")]
[numthreads(10, 1, 1)]
void main() { }
dxc ~/shader.hlsl -T lib_6_6 -spirv -fspv-target-env=vulkan1.3

The annotation makes us generate the valid SPIR-V for a compute shader. So what about making the whole -T xx_Y_Y flag optional for SPIR-V?

The current issue is that library keeps all the entrypoints, as it is designed for this. But maybe the default behavior with no flags would be to compute only the specified entrypoint (with -E).

So in such case:

[shader("compute")]
[numthreads(10, 1, 1)]
void shader1() {
}

[shader("pixel")]
float4 shader2() : SV_TARGET {
  return float4(1, 1, 1, 1);
}
llvm-beanz commented 11 months ago

I have no objection to this, but is it a better user interface to make -T optional, or should we support new option values for -T to expose the correct SPIR-V targets?

I could imagine -T vulkan_1_3 being a simpler user interface.

Keenuts commented 11 months ago

That would also be fine for me, deprecating -fspv-target-env.

On the Clang side, we will probably have 2 ways to specify the versions:

Maybe that's the opportunity for alignment there.

The command I use to compile HLSL in clang is clang -cc1 -triple dxil-unknown-shadermodel6.3-compute, do you plan to add a flag to the clang driver other than the triple for dxil?

s-perron commented 2 weeks ago

I don't think we should change anything in DXC. We can discuss the clang interface in another forum.