Fix CB vector to scalar array translation generating invalid IR (#6777)#6800
Merged
python3kgae merged 1 commit intomicrosoft:release-1.8.2407from Jul 22, 2024
Merged
Conversation
…osoft#6777) In the special code to handle the memcpy pattern where a constant buffer contains a vector array that initializes a local (or static global) scalar array for use by the shader, an invalid assumption was made that if the memcpy dest was global, that the src is global as well. This was not the case, and when expecting to generate constant expressions to index the src, these generated orphaned instructions instead, leading to invalid IR. This fixes the issue by leveraging ReplaceConstantWithInst, and setting the insertion point for the Builder. Now, replacement *could* fail, if src instructions don't dominate replacement uses, so bool for replaced all is returned from replaceScalarArrayWithVectorArray. Another issue was that it would replace the dest for the original memcpy with src along the way. Now, if we don't replace all uses, this turns the memcpy into a no-op and any remaining uses are no longer coming from src, but an undef dest instead. This was also fixed to skip this replacement, then clean up this use if all other uses have been successfully replaced. Fixes microsoft#6510 (cherry picked from commit 5cfefc7)
damyanp
approved these changes
Jul 18, 2024
bob80905
reviewed
Jul 18, 2024
Collaborator
|
Is there a way to shrink the sizes of the 3 tests? |
Contributor
Author
We could remove the debug info. |
bob80905
approved these changes
Jul 22, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the special code to handle the memcpy pattern where a constant buffer contains a vector array that initializes a local (or static global) scalar array for use by the shader, an invalid assumption was made that if the memcpy dest was global, that the src is global as well.
This was not the case, and when expecting to generate constant expressions to index the src, these generated orphaned instructions instead, leading to invalid IR.
This fixes the issue by leveraging ReplaceConstantWithInst, and setting the insertion point for the Builder. Now, replacement could fail, if src instructions don't dominate replacement uses, so bool for replaced all is returned from replaceScalarArrayWithVectorArray.
Another issue was that it would replace the dest for the original memcpy with src along the way. Now, if we don't replace all uses, this turns the memcpy into a no-op and any remaining uses are no longer coming from src, but an undef dest instead. This was also fixed to skip this replacement, then clean up this use if all other uses have been successfully replaced.
Fixes #6510
(cherry picked from commit 5cfefc7)