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

also search global sequences for live variables #2345

Merged
merged 1 commit into from
Jul 22, 2020

Conversation

mbechard
Copy link
Contributor

when traversing the AST to find live UBOs etc, also traverse
module-level variable assignment, not just function calls.

Otherwise for cases such as this at the module level:

float f = SomeUniform;

SomeUniform won't be determined to be 'live'.

Is checking for EOpSequence the correct OpType here, or is there something more appropriate?

@johnkslang
Copy link
Member

Why limit it to a nested EOpSequence? The code was looking inside each function, and you want to add looking outside each function, it just seems like you'd want to traverse anything that's not a function. It could be you picked them all up, just possibly fragile, if not everything stays two levels deep in the tree at global scope.

@mbechard
Copy link
Contributor Author

Should it just traverse everything then and remove the code that is selectively just doing functions?

@johnkslang
Copy link
Member

That sounds reasonable. I don't know if there's a subtlety, but the obvious thing now is having a traversal the just selects functions, and a traversal that just skips functions, wondering if it should all just be one traversal. But, whatever solves the actual problem.

when traversing the AST to find live UBOs etc, also traverse
references to global module-level variables, incase they are
being filled in from UBOs etc.
@mbechard
Copy link
Contributor Author

Went a different route. Now when global variables are visited it adds those to the list of things to traverse. That way globals that aren't referenced anywhere aren't going to end up tagging the variable source as 'live'.

@johnkslang johnkslang merged commit 1c42d4e into KhronosGroup:master Jul 22, 2020
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.

None yet

2 participants