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

Add structs to eliminate dead input components #4894

Merged
merged 1 commit into from
Aug 16, 2022

Conversation

greg-lunarg
Copy link
Contributor

Will eliminate all trailing members of input struct that are not
referenced.

@greg-lunarg greg-lunarg requested a review from Keenuts August 12, 2022 21:46
Will eliminate all trailing members of input struct that are not
referenced.
Copy link
Contributor

@Keenuts Keenuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello!

How would that change affect struct array stride?
Looks like we define the max size by walking the use, and if it's an OpAccessChain with constant indice, truncate the struct.
What would happen if the struct is part of a uniform buffer?
Or if the type is used into another type?

If those cases are handled, would it be useful to add tests for them?

@Keenuts Keenuts requested a review from s-perron August 15, 2022 14:45
@s-perron
Copy link
Collaborator

How would that change affect struct array stride?

In this case, the variable is of type struct, so there is no array of structs.

Looks like we define the max size by walking the use, and if it's an OpAccessChain with constant indice, truncate the struct.
What would happen if the struct is part of a uniform buffer?

This pass looks specifically for variables that are in the input storage class. If it wants to change the size of the variable, it creates a new type, and changes the type of the variable. Other variables will not be changed. Maybe the commnets are not clear enough that it changes the type of the variable.

However, there is another pass (dead member elimination) that removes any unused members from uniforms. The members of the struct do not determine the layout of the struct. The layout is determined by the decorations. See decorations are used for that. See ArrayStride and Offset.

When you "remove" the member of a struct, all we are doing is removing a way of referencing the member. We are not changing the layout.

Or if the type is used into another type?

Again, the old type is not deleted. A new type is created.

@greg-lunarg
Copy link
Contributor Author

@Keenuts Do you concur with @s-perron 's approval? If so, can you please approve?

@Keenuts
Copy link
Contributor

Keenuts commented Aug 16, 2022

Sorry, yes of course approved 😀 Thanks for the explanations Steven!

@s-perron s-perron merged commit 71b2aee into KhronosGroup:master Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants