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

SpirvToolsEliminateDeadInputComponents() doesn't seem to be trimming array size down #5632

Open
mbechard opened this issue Apr 3, 2024 · 1 comment

Comments

@mbechard
Copy link

mbechard commented Apr 3, 2024

The following code does not end up with a reduced array size for arrInOut. The whitepaper (and my recollection of early tests of this feature with @greg-lunarg) seems to suggest that array size should get reduced to arrInOut[2]. Since the resources available for in/out variables between shader stages are very limited, this is an important optimization.
arrIn is correctly reduced to arrIn[2] though.

Vertex:

out Vertex
{
	vec4 c;    
	vec3 n;
	
} oVert;
in vec3 arrIn[5];
out vec3 arrInOut[5];
void main() 
{
	arrInOut[1] = arrIn[1];
	oVert.c = vec4(1, 0, 0, 1);
	oVert.n = vec3(1);
	gl_Position = vec4(0);
}

Fragment

in Vertex
{
    vec4 c;
    vec3 n;
    
} iVert;
in vec3 arrInOut[5];
out vec4 fragColor;
void main()
{
	vec4 color = vec4(iVert.c + arrInOut[1].xyzz);
	fragColor = color;
}

Thanks!

@greg-lunarg
Copy link
Contributor

Some input and output arrays are not trimmed by EliminateDead*Components because sometimes it is not safe to do so given the current design.

Input arrays in vertex shaders can always be safely trimmed as with arrIn[] pointed out by @mbechard . Same for output arrays in frag shaders. Any others can potentially break a shader interface given the current design.

EliminateDeadInputComponents (EDIC) and EliminateDeadOutputComponents (EDOC) only operate on one shader at a time. So the workflow has to depend on EDOC and EDIC trimming the same elements on both sides of a shader interface so to not break the interface. In the workflow this can always be guaranteed for structs, but not for arrays. This is because structs can only be indexed with constants, but arrays can be indexed with variables. ED*C cannot trim an array that is indexed with a variable because it has to assume any element might be referenced. So, if on one side of an interface an array is indexed only with constants and on the other side is indexed with a variable, the array may be trimmed differently on either side and break the interface. For this reason EDIC and EDOC do not trim arrays except for the special cases mentioned.

My best idea is to create a new all_array mode in the EDOC and EDOC passes which would trim all arrays with only constant indices. This mode would be controlled with a bool argument. The default would be false.

In order to make sure it is safe to use the all_array mode, I propose new passes called EliminateDeadInputComponentsArraySafe(bool &safe) and EliminateDeadOutputComponentsArraySafe(bool &safe) . These passes would return true if no Input/Output arrays are indexed with variables in the shader. In the workflow, when optimizing shaders A & B, it would be safe to use this new all_array mode if both A & B are ArraySafe. The new all_array mode would need to be used on both shaders.

The control here is fairly coarse-grained (per shader), but since (I believe) it is fairly rare for input/output arrays to be indexed with variables, I don't think this would be too prohibitive. Someone can add more fine-grained control (per array) if it is needed. My design above would be sufficient for this issue.

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