[SPIR-V] Fix image write to unknown format #6984
Merged
Conversation
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
When 'unknown' was used in vk::image_format, the value was ignored, and a format was still guessed. The fix revealed a bug with Write/ReadWithoutFormat capability gen: - image formats attributes are applied as a fix-up after initial codegen. - the variable is changed, but not all related operations are fixed. - capabilities visitor assumed types were coherent, and only looked at the OpImageWrite/Read operand type. Not the source variable. - legalization fixes those incoherences, but runs after the visitor. Using the capability trimming pass for this is an easy way to solve this issue. One test was checking for ReadWithoutFormat, but doesn't require it since no OpImageRead is done on a Unknown format. Signed-off-by: Nathan Gauër <[email protected]>
Signed-off-by: Nathan Gauër <[email protected]>
s-perron
approved these changes
Oct 28, 2024
cassiebeckley
approved these changes
Oct 28, 2024
SjMxr233
pushed a commit
to ShaderHelper/DirectXShaderCompiler
that referenced
this pull request
Jul 24, 2025
By default, texture format is guessed from the type. But the `vk::image_format` attribute can be added to specify it. The `unknown` value was used to convey `no format selected`, and require the guess to happen. This made selecting `unknown` as value impossible. Once the optional added, we have a new issue: - The format setup relied on the legalizer. - load/stores are done using the default format, then we fix it, then we use the legalizer to propagate the format fix to all users. The capability visitor is running before the legalizer, meaning it can look at a non-fixed load, which still carries the old type. The way to solve this is to remove this logic, and move the capability addition/deletion to the capability_trimming pass, run after legalization. Fixes microsoft#6981 --------- Signed-off-by: Nathan Gauër <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
By default, texture format is guessed from the type. But the
vk::image_formatattribute can be added to specify it.The
unknownvalue was used to conveyno format selected, and require the guess to happen.This made selecting
unknownas value impossible.Once the optional added, we have a new issue:
The capability visitor is running before the legalizer, meaning it can look at a non-fixed load, which still carries the old type.
The way to solve this is to remove this logic, and move the capability addition/deletion to the capability_trimming pass, run after legalization.
Fixes #6981