DirectXShaderCompiler icon indicating copy to clipboard operation
DirectXShaderCompiler copied to clipboard

[SPIR-V] mixed entry points reference unsupported BuiltIns in interface list

Open natevm opened this issue 3 years ago • 2 comments
trafficstars

I'm trying to replicate a CUDA/OptiX workflow with HLSL, and I ran into this issue while mixing a compute entry point and a closest hit entry point together in the same device program...

A bit of background, I want to use the compute entry point to fill in the instance buffer required for a TLAS build, so that I can update instance transforms in real time. Then, I use a closest hit program to interpolate vertex data of whatever instance was hit, and then that program returns a color via a ray payload.

When both of these stages are compiled into the same .spv file, it appears that certain built-in functions, like OpDecorate %7 BuiltIn WorldRayDirectionNV, are incorrectly listed in the interface list for the compute entry point (which does not reference this built-in function) :

OpEntryPoint GLCompute %vkrtFillInstanceData "vkrtFillInstanceData" ... %7 ...

This causes vkCreateComputePipelines to crash on AMD cards, in the driver and with no errors reported in any of the validation layers.

natevm avatar Aug 28 '22 00:08 natevm

it’s possible that this pass in SPIRV-Tools can repair the incorrect SPIR-V entry points generated by DXC, but I’m not sure.

https://github.com/KhronosGroup/SPIRV-Tools/pull/4275

#3789 is a good reference issue that appears to be running into a similar limitation with DXC.

@jaebaek since you were on the previous issue, do you know what might be required to correct the interfaces of each entry point to avoid referencing unsupported builtIn functions? Is there anything that can be done at compilation time rather than as a repair pass afterwards?

natevm avatar Aug 28 '22 18:08 natevm

It seems this repair pass is actually unavailable in SPIRV-Tools, so this is still a breaking issue with dxc unfortunately.

natevm avatar Aug 30 '22 19:08 natevm

The aggressive dead code elimination pass should remove unused input. Can you provide a sample that I can double check?

s-perron avatar Nov 03 '23 16:11 s-perron

Sure, @s-perron here's a minimal reproducer on godbolt: https://godbolt.org/z/6T4P391fj

Notice that on line 6 of the emitted SPIR-V, we get the following: OpEntryPoint GLCompute %computeShader "computeShader" %2 %g_renderTarget %payload

where %2 maps to the following built-in : OpDecorate %2 BuiltIn WorldRayDirectionNV

This is invalid SPIRV, since a Compute shader entrypoint type does not support the WorldRayDirectionNV built-in. Despite never actually calling or using this built in from the compute shader, ADCE fails to remove WorldRayDirectionNV from the input list.

natevm avatar Nov 03 '23 18:11 natevm

Similar issues occur in more realistic use cases, where a ray generation program and a closest hit program might exist in the same library and the closest hit uses a built in like WorldRayDirection which is unavailable to ray generation entry points.

Here's a godbolt reproducer of this scenario as well: https://godbolt.org/z/8fdsaKKjq

natevm avatar Nov 03 '23 18:11 natevm

I finally got around to looking at this. I see why DCE is not removing. We generally think of our optimizations working on single shader modules, because that use to be the only common case. If a shader did not reference an input it would be removed from the module completely by DCE, which includes removing it from the OpEntryPoint.

In this case, there are two entry points. Since one of them references the input, DCE does not remove the input. Extending DCE to remove the dead input from one entry point is not inline with its algorithm. It does not keep track of which entry points could execute different instructions.

The remove unused interface variables does work, but is not run as part of the DXC legalization passes. I will add it to the legalization passes in spir-v opt.

s-perron avatar Feb 14 '24 15:02 s-perron