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

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

Open
danginsburg opened this issue Jul 23, 2024 · 4 comments
Open

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

danginsburg opened this issue Jul 23, 2024 · 4 comments

Comments

@danginsburg
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

jeremy-lunarg commented Jul 25, 2024

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

@greg-lunarg
Copy link
Contributor

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?

jeremy-lunarg added a commit to jeremy-lunarg/SPIRV-Tools that referenced this issue Aug 26, 2024
jeremy-lunarg added a commit to jeremy-lunarg/SPIRV-Tools that referenced this issue Sep 5, 2024
jeremy-lunarg added a commit to jeremy-lunarg/SPIRV-Tools that referenced this issue Sep 9, 2024
jeremy-lunarg added a commit to jeremy-lunarg/SPIRV-Tools that referenced this issue Sep 9, 2024
jeremy-lunarg added a commit to jeremy-lunarg/SPIRV-Tools that referenced this issue Sep 10, 2024
jeremy-lunarg added a commit to jeremy-lunarg/SPIRV-Tools that referenced this issue Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants