Issue with EliminateDeadOutputStores when reading from out variable

[mbechard]

Hey,
If I have a GLSL shader that is reading from an 'out' variable such as

out vec4 pos;
void main()
{
    pos = vec4(0.0);
    gl_Position = pos;
}

And then I run the resulting SPIRV through the EliminateDeadOutputStores() pass, I get an assert saying 'unexpected use of output variable'.
This should be legal code since "During shader execution they will behave as normal unqualified global variables" as per the GLSL spec.
@jeremy-lunarg can you take a look since you took over maintenance of this feature?
Thanks!
Malcolm

[mbechard]

Sorry to ping this, it's affecting a lot of my users due to this being a common writing workflow in shaders it seems.

[mbechard]

With the recent paper highlighting this workflow, I'd again like to ping this one

[greg-lunarg]

Hi Malcolm, Very sorry about missing this. I am semi-retired and not doing as good a job at monitoring email, issues, etc. I will make sure someone starts working on this. Best regards, Greg Greg Fischer Senior Engineer LunarG, Inc. - 3D Driver Innovations ***@***.*** ***@***.***>

…

On Mon, Apr 1, 2024 at 1:59 PM Malcolm Bechard ***@***.***> wrote: With the recent paper highlighting this workflow, I'd again like to ping this one — Reply to this email directly, view it on GitHub <#5516 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABZPVK672XVP2QGF46VXJLDY3G4BRAVCNFSM6AAAAABBKPFLBSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZQGQ3DANBYGE> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[jeremy-lunarg]

@mbechard Sorry. This fell through the cracks. I'll start looking at this tomorrow.

[jeremy-lunarg]

This looks like it's going to be non-trivial. I think pos should be eliminated but currently store-load elimination does not operate on output variables and the shader would be broken if we eliminated pos. We can do store-load elimination on pos but it would require writing a new pass. At the very least we shouldn't assert and we should silently ignore pos during EliminateDeadOutputStores. I think I'll work on that first.

[mbechard]

Thanks. right so more specifically this occurs if 'pos' is not consumed by the pixel shader, and thus it tries to be eliminated from the vertex shader during the call to SpirvToolsEliminateDeadOutputStores(), which causes this issue.

Ideally if the pass is trying to eliminate an output variable that is being used for a store-load, it should be converted to a temporary variable. Leaving it as-is would work for standalone outputs, but would cause an interface mismatch if the output is part of a block, where it's been eliminated from the block in the pixel but getting left as-is in the vertex