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

Unused shader inputs are getting eliminated since the latest release #722

Closed
hrydgard opened this issue Aug 13, 2021 · 2 comments
Closed
Labels
t: bug Something isn't working

Comments

@hrydgard
Copy link
Contributor

hrydgard commented Aug 13, 2021

Somewhere between the rust-gpu .alpha.11 and .alpha.12 releases, it started entirely eliminating inputs like this:

#[spirv(push_constant)] _material: &Material,

if they are unused in the shader. This seems to be a bug.

(It's useful to keep the interface the same while developing a shader, sometimes you know you'll need an input but not right now)

@hrydgard hrydgard added the t: bug Something isn't working label Aug 13, 2021
@eddyb
Copy link
Contributor

eddyb commented Aug 13, 2021

This was likely caused (or rather, exposed) by #715 - however, inlining would've likely caused a similar issue.

My best guess is that DCE doesn't consider the OpEntryPoint interface variable list as a "root" that can keep its elements alive - I'm not sure having globals there without ever mentioning them inside functions will work, but we should try it at least.

(EDIT: my guess was wrong, @khyperia found the cause elsewhere - #691 + pre-1.4 SPIR-V making it impossible to tell how PushConstant interface variables map to entry-points - that seems unsound, doesn't it?)

@khyperia
Copy link
Contributor

Late follow up! After researching this a lot, I couldn't find any generic, edge-case-free way to forcibly mark a variable as used in pre-1.4 SPIR-V that covers all cases. So, I don't think this is solvable, sadly.

Some background:

In SPIR-V, there's a concept called an "interface", which is supposedly all the public variables that the shader module will interact with the outside world with. In SPIR-V versions 1.3 and earlier, this interface list only included Input and Output variables - the bindings to the previous/next shader stages. Because multiple entry points can be in the same file, if there's a uniform laying around the binary, it's impossible to tell if it's part of a shader or not, except for scanning through the code of each entry point and seeing if it references the uniform. So, if a uniform (or push constant, or whatever) is declared, but never used, it gets emitted into the binary, but never touched by code, and so it's impossible for the driver to know that it was supposed to be part of that entry point.

SPIR-V 1.4 realized their mistake and fixed this by making this "interface" actually list every global variable, including uniforms, push constants, etc. declared by the shader - so now, even if a uniform isn't used in code by a shader, the driver will still know that yes, this uniform is part of this shader.

So, because idk how to inject random useless references to variables in SPIR-V <1.4 to make them be included, unused shader inputs will effectively be removed in SPIR-V <1.4. I guess the "fix" is to update to 1.4 or later, where SPIR-V realized the design flaw and it got fixed.


(The reason this worked for a while in rust-gpu and then got broken with an update is because we were generating horrible code for this specific case that happened to reference the variable in a way that spirv-opt didn't nuke - I thiiink they might have since fixed that optimization bug - but other very similar cases got evaporated and we just never hit those)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants