Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPIR-V] SV_Position can no longer be declared to be min10float4, min16float4, float16_t4 or half4 after fix to issue 3736 (works with DXIL) #4262

Closed
Dredhog opened this issue Feb 15, 2022 · 1 comment · Fixed by #4275
Assignees
Labels
spirv Work related to SPIR-V

Comments

@Dredhog
Copy link

Dredhog commented Feb 15, 2022

Title

[SPIR-V] SV_Position can no longer be declared to be min10float4, min16float4, float16_t4 or half4 after fix to issue 3736

Functional impact

  • Vertex shaders using these types for SV_Position written for FXC will not be compatible with DXC
  • Cannot declare SV_Position to be relaxed or true 16-bit precision in the SPIR-V backend (can with the DXIL backend)

Minimal repro steps

  1. Compile any of the following vertex shaders with with dxc.exe -T vs_6_0 -E Vert -spirv repro_shader.txt:
min10float4 Vert () : SV_Position
{
    return 1;
}
min16float4 Vert () : SV_Position
{
    return 1;
}
half4 Vert () : SV_Position
{
    return 1;
}
  1. Compile the following vertex shader using explicit fp16 support dxc.exe -T vs_6_2 -E Vert -spirv -enable-16bit-types repro_shader.txt:
float16_t4 Vert () : SV_Position
{
    return 1;
}

Expected result

Compilation succeeds

Actual result

The compiler produces the corresponding error messages:

repro_shader.txt:1:24: error: semantic Position must be float4 or a composite type recursively including only float4
min10float4  Vert () : SV_Position
                       ^
repro_shader.txt:1:24: error: semantic Position must be float4 or a composite type recursively including only float4
min16float4  Vert () : SV_Position
                       ^
repro_shader.txt:1:18: error: semantic Position must be float4 or a composite type recursively including only float4
half4  Vert () : SV_Position
                 ^

The vertex shader with -enable-16-bit-types produces the following error:

repro_shader4.txt:1:22: error: semantic Position must be float4 or a composite type recursively including only float4
float16_t4 Vert () : SV_Position
                     ^

Further technical details

  • Seems to have been caused by the fix to issue 3736. The fix to only allow float4 seems to have been overly conservative.
  • Used dxc.exe downloaded from release v1.6.2112 (Dec 09, 2021).
  • Note, the DXIL validator appears to have a small typo, where it prints error: SV_Position must be float4. when the component count is not 4 for SV_Position, which seems to not account for the other 4 component floating-point types that are supported.
@jaebaek jaebaek added the spirv Work related to SPIR-V label Feb 15, 2022
@jaebaek jaebaek self-assigned this Feb 15, 2022
@jaebaek
Copy link
Collaborator

jaebaek commented Feb 15, 2022

Thank you for reporting this issue. We will take a look. Since now we are experiencing intense workloads, it would take some time.

sudonatalie added a commit to sudonatalie/DirectXShaderCompiler that referenced this issue Feb 17, 2022
A valid vertex shader output variable with SV_Position semantics may be
constructed from any HLSL BuiltinType that translates to a 32-bit
floating point type in the SPIR-V backend, so relax the requirements to
allow the use of additonal types (such as half4) when
-enable-16bit-types is false.

Fixes microsoft#4262
@sudonatalie sudonatalie assigned sudonatalie and unassigned jaebaek Feb 17, 2022
sudonatalie added a commit that referenced this issue Feb 22, 2022
A valid vertex shader output variable with SV_Position semantics may be
constructed from any HLSL BuiltinType that translates to a 32-bit
floating point type in the SPIR-V backend, so relax the requirements to
allow the use of additonal types (such as half4) when
-enable-16bit-types is false.

Fixes #4262
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spirv Work related to SPIR-V
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants