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

Implicit LOD functions can be supported when compute shader derivatives are supported #5674

Open
EpicJeanNoeMorissette opened this issue May 17, 2024 · 6 comments

Comments

@EpicJeanNoeMorissette
Copy link
Contributor

IsFragmentShaderOnlyInstruction(inst)) {

When using DXC, the SPIRV is correctly generated prior: the proper extensions (SPV_NV_compute_shader_derivatives) and execution mode (DerivativeGroupLinearNV or Quad) are specified, but then this pass goes through and removes all the instructions.

We simply added "model != spv::ExecutionModel::GLCompute" in our local fork, but it would probably be cleaner to also ensure that the extension is active and rename the function.

EpicJeanNoeMorissette added a commit to EpicGames/SPIRV-Tools that referenced this issue May 17, 2024
Allow for implicit texture sampling from compute since we support DerivativeGroupLinearNV (SPIRV-Tools issue KhronosGroup#5674)
@s-perron
Copy link
Collaborator

I remember adding this pass. I forget the details, but I sort-of regret adding the pass. Before I update it, I'd like to revisit if it is really needed. Do you have a sample that you can share where an instruction must be removed to be valid? I'd like to understand why the instruction is not dead. It scares me a little that we simply replace an instruction that is still used. If needed, I will update the pass.

@EpicJeanNoeMorissette
Copy link
Contributor Author

To be honest, I feel exactly the same way you do: I was surprised to see this pass simply remove instructions that were in use (although there were no side effects for what it's worth). I would sort of expect a compilation error with a detailed message (this only gave me a warning which I didn't notice at first).

@s-perron
Copy link
Collaborator

I would sort of expect a compilation error with a detailed message (this only gave me a warning which I didn't notice at first).

If you disable the pass in your workflow, you will get a validation error in the shaders that contain the invalid instructions. I'll close the issue for now. Reopen if you decide you need the pass updated.

@EpicJeanNoeMorissette
Copy link
Contributor Author

Reopen if you decide you need the pass updated.

I'm not sure I understand? :) Left unchanged, this pass removes all implicit LOD functions from my compute shaders when using compute shader derivatives. This breaks my compute shaders, that much I am certain about.

@s-perron
Copy link
Collaborator

What happens if you do not use the pass at all for any shaders? That is what I wanted to know. It is possible the pass is no longer needed. If it is still needed, can we improve optimizations so that we can deprecate the pass. I believe you are the only users.

@s-perron s-perron reopened this May 29, 2024
@EpicJeanNoeMorissette
Copy link
Contributor Author

I'll give it a try and report back!

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

2 participants