vkd3d-shader/spirv: Introduce a fixme on clip/cull inputs.
Currently the first geometry shader test in clip-cull-distance.shader_test segfaults on Mesa 25.0.5-arch1.1 radv when on shader model 6.0, after the HLSL is compiled to DXIL using DXC and converted to SPIR-V.
Running with validation layers and valgrind:
shader_runner.c:2103: Compiling SM6.0-SM6.2 shaders with dxcompiler and executing with vkd3d.
shader_runner.c:1957: tags: "d3d12".
shader_runner.c:1982: caps: depth-bounds float64 geometry-shader int64 rov rt-vp-array-index
shader_runner.c:1995: wave-ops.
shader_runner.c:2019: uav-load: 0x2 0x3 0x4 0xa 0xc 0xe 0x10 0x1c 0x1e 0x20 0x29 0x2a 0x2b 0x36 0x39
shader_runner.c:2033: 0x3b 0x3d 0x3e 0x40.
...
Validation Error: [ VUID-VkShaderModuleCreateInfo-pCode-08737 ] | MessageID = 0xa5625282
vkCreateShaderModule(): pCreateInfo->pCode (spirv-val produced an error):
OpAccessChain result type (OpTypeVector) does not match the type that results from indexing into the base <id> (OpTypeArray).
%77 = OpAccessChain %_ptr_Input_v2float %v1 %74
The Vulkan spec states: If pCode is a pointer to SPIR-V code, pCode must adhere to the validation rules described by the Validation Rules within a Module section of the SPIR-V Environment appendix (https://docs.vulkan.org/spec/latest/chapters/shaders.html#VUID-VkShaderModuleCreateInfo-pCode-08737)
...
==198473== Invalid read of size 1
==198473== at 0x1AC75A28: radv_shader_spirv_to_nir (radv_shader.c:381)
==198473== by 0x1AC5BC33: radv_graphics_shaders_compile (radv_pipeline_graphics.c:2621)
==198473== by 0x1AC5EAFC: radv_graphics_pipeline_compile (radv_pipeline_graphics.c:3020)
==198473== by 0x1AC60817: radv_graphics_pipeline_init (radv_pipeline_graphics.c:3408)
==198473== by 0x1AC61461: radv_graphics_pipeline_create (radv_pipeline_graphics.c:3469)
==198473== by 0x1AC61461: radv_CreateGraphicsPipelines (radv_pipeline_graphics.c:3620)
==198473== by 0x1D82E50C: vvl::dispatch::Device::CreateGraphicsPipelines(VkDevice_T*, VkPipelineCache_T*, unsigned int, VkGraphicsPipelineCreateInfo const*, VkAllocationCallbacks const*, VkPipeline_T**) (dispatch_object_manual.cpp:1002)
==198473== by 0x1D82266E: vulkan_layer_chassis::CreateGraphicsPipelines(VkDevice_T*, VkPipelineCache_T*, unsigned int, VkGraphicsPipelineCreateInfo const*, VkAllocationCallbacks const*, VkPipeline_T**) (chassis_manual.cpp:490)
==198473== by 0x4A7BF63: d3d12_pipeline_state_get_or_create_pipeline (state.c:4115)
==198473== by 0x49C040B: UnknownInlinedFun (command.c:2833)
==198473== by 0x49C040B: d3d12_command_list_begin_render_pass.lto_priv.0 (command.c:3468)
==198473== by 0x49C1596: d3d12_command_list_DrawInstanced.lto_priv.0 (command.c:3555)
==198473== Address 0x40 is not stack'd, malloc'd or (recently) free'd
==198473==
==198473==
==198473== Process terminating with default action of signal 11 (SIGSEGV): dumping core
...
Segmentation fault (core dumped)
This segfault seems to happen because we are generating invalid SPIR-V. When dumping this SPIR-V, we have the clip/cull input variables declared as 2-dimension arrays:
%v1 = OpVariable %_ptr_Input__arr__arr_float_uint_1_uint_3 Input
%v2 = OpVariable %_ptr_Input__arr__arr_float_uint_1_uint_3 Input
%v3 = OpVariable %_ptr_Input__arr__arr_float_uint_1_uint_3 Input
%v4 = OpVariable %_ptr_Input__arr__arr_float_uint_1_uint_3 Input
and then incorrectly accessed as 1-dimension arrays:
%77 = OpAccessChain %_ptr_Input_v2float %v1 %74
%78 = OpInBoundsAccessChain %_ptr_Input_float %77 %uint_0
%79 = OpLoad %float %78
This is a mishandling of input clip/cull builtin variables.
Currently spirv_compiler_emit_shader_signature_outputs() takes care of creating arrayed variables for clip and cull distances, calling spirv_compiler_emit_builtin_variable(). It only makes one array variable of the right size (up to one for clip, and one for cull) even if there are multiple clip or cull elements in the output signature.
Thanks to this function, in this shader, clip is float[5] and cull is float[3].
float2 clip : SV_ClipDistance; \ float[5]
float3 clip2 : SV_ClipDistance1; /
float2 cull2 : SV_CullDistance1; \ float[3]
float1 cull : SV_CullDistance; /
The clip/cull built-ins, also decorated as such by this function, are expected by spirv in this array form.
Input signature clip/cull elements are not handled in the previous function, instead one variable for every element is emitted by spirv_compiler_emit_input(), which detects they have a sysval and thus, that they are built-ins. Every input signature element is created as a two-dimensional array calling spirv_compiler_emit_builtin_variable_v().
float2 clip : SV_ClipDistance; -> float[3][1]
float3 clip2 : SV_ClipDistance1; -> float[3][1]
float2 cull2 : SV_CullDistance1; -> float[3][1]
float1 cull : SV_CullDistance; -> float[3][1]
Where the 3 is because of the compiler->input_control_point_count, it being a geometry shader.
The 2-dimensional array is wrong since the code only expects one array when dereferencing, in particular spirv_compiler_emit_dereference_register(). Also note that the component count is overwritten to be 1 which is also incorrect, and it happens for every type of shader.
If we instead ignore the sysval when emitting input clip/cull variables, spirv_compiler_emit_array_variable() gets called and we get:
float2 clip : SV_ClipDistance; -> float2[3]
float3 clip2 : SV_ClipDistance1; -> float3[3]
float2 cull2 : SV_CullDistance1; -> float2[3]
float1 cull : SV_CullDistance; -> float1[3]
which doesn't generate invalid spirv.
While the generated spirv is not invalid with this change, it is not completelly correct, as we still need to merge the clip/cull inputs into an array, as we do for the outputs, for these values to be properly received by the shader, otherwise the results of executing the shader vary.
For this reason, a fixme is introduced.
To make this transformation to array also available for other backends I am currently working on an a rewrite of the vsir pass made in Conor's !564 (closed), so that normalized vsir has clip/culls already in the array form, but it touches many complex things in vsir and the spirv backend (and potentially also other backends) I was not previously familiarized with, so it has taken me some time.