search

KhronosGroup / SPIRV-Tools

Apache License 2.0
1.08k stars 555 forks source link

spirv-opt unable to eliminate dead loads from raytracing library #5745

Open danginsburg opened 2 months ago

danginsburg commented 2 months ago

I have a Raytracing library that has two anyhit entrypoints (generated from slang using SLANG_TARGET_FLAG_GENERATE_WHOLE_PROGRAM). When the second entrypoint is enabled, spirv-opt -Os is unable to eliminate a large number of loads to the ByteAddressBuffer g_MaterialData in the shader. Downstream of this, our use of spirv-reflect SpvReflectDescriptorBinding::byte_address_buffer_offsets contains offsets that should have been dead code eliminated.

I am able to workaround this by not using SLANG_TARGET_FLAG_GENERATE_WHOLE_PROGRAM and instead generating SPIR-V for each entrypoint and then using spirv-link to link them together. However, it would be great if spirv-opt would still be able to do dead load elimination in this case. The SPIR-V library that contains the issue is attached.

test.zip

jeremy-lunarg commented 2 months ago

Typically, private variables are converted to a local variable, which can be optimized further. However, if there are two functions referencing the same private variable, then the PrivateToLocal pass will bail out. This is because it's considered unsafe. Fortunately, if both functions are used exclusively in entry points, then I think it would be safe to create local copies in each function. This will take a bit of work.

danginsburg commented 2 months ago

Your analysis sounds correct. The high level shader has a static global variable that both entrypoints are assigning to. According to @csyonghe it could also be possible for slang to do an optimization pass over the shader prior to generating SPIR-V that does the conversion to local variable. It perhaps would be good though if spirv-opt could still handle the case though even if slang implements that pass?

jeremy-lunarg commented 2 months ago

I agree. I'd like to see it in spirv-opt. I'm working on a patch for this. Thanks Dan.

greg-lunarg commented 2 months ago

I am concerned with replicated optimizations in slang and spirv-tools. Redundancy is a maintenance burden for the whole community. Is there a good reason to also put this in slang if we are fixing this in spirv-tools?