Disallow structs, arrays, and overwide vectors from typed buffers #7130
+171
−785
Conversation
This file contains 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
…didates The param array populated by MatchArguments only gets modifiers such as references applied when the intrinsic is new and needs to be added to the cache. By using the type of the parameter in the found candidate function regardless of whether it was created and added or just retrieved maintains the modifier for the sake of error messages giving the correct result for failed conversions of the same intrinsic after the first.
|
The first four commits are intended to facilitate reviewing of mostly independent parts:
|
These never reliably generated the right results, so they are being disallowed. The check takes place in SemaHLSL and replaces a few places that checked for such things in special cases. To facilitate the check, the definition of the texture bit was expanded to include RW textures. This is consistent with the description and since the bit was never actually used before, has no other consequences. Some code that executed after Sema that either produced errors or processed resources with these now forbidden types was removed. Most of this is from CodeGen and HLOperationLower where code was generated or expanded for load/store operations on such resources. Additionally moved the check for overlarge elements in typed resources to enable keeping some tests around that tested these side by side as well as adding a new one. I changed the nature of the check a bit to limit the types to four elements and also that they be no larger than 4 32-bit elements. This means that 64-bit elements are limited to two. While there wasn't an error that caught this before, there was an assert that fired. Incidentally fixes an issue with error reporting for modified types after the first mismatch of that intrinsic is encountered. Because the insertion code changed them in the temp array and the stored function type, but printed from the temp array and the later encounters didn't add the modifiers to the temp array, later errors would incorrectly leave off the reference. By retrieving the type from the function type, the reference is always preserved. This required changing a fair number of tests that relied on this behavior including removing test cases and entire test files that existed solely for such tests. Fixes microsoft#7080
pow2clk
force-pushed
the
no_tybuf_aggs
branch
from
4633d6b to
04f476a
Compare
I forgot about 2DMS
tex3d
reviewed
Feb 11, 2025
tools/clang/lib/Sema/SemaHLSL.cpp
Outdated
| @@ -14178,6 +14180,35 @@ bool Sema::DiagnoseHLSLDecl(Declarator &D, DeclContext *DC, Expr *BitWidth, | |||
|
|
|||
| ArBasicKind basicKind = hlslSource->GetTypeElementKind(qt); | |||
|
|
|||
| // Check for invalid template params in typed buffer/texture declarations. | |||
There was a problem hiding this comment.
I think this checking needs to be done under CheckTemplateArgumentListForHLSL, like by updating IsValidTemplateArgumentType, rather than on the DiagnoseHLSLDecl.
There was a problem hiding this comment.
I moved it to CheckTemplateArgumentListForHLSL. I was tempted to move it to IsValidTemplateArgumentType, but it would need to add some params to that and then would just be an else there instead of outside where we can already identify textures (though I'm not thrilled with how we need to identify them there either)
Not fond of the way we detect Texture templates, but I don't see another one.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
These never reliably generated the right results, so they are being disallowed. The check takes place in SemaHLSL and replaces a few places that checked for such things in special cases. To facilitate the check, the definition of the texture bit was expanded to include RW textures. This is consistent with the description and since the bit was never actually used before, has no other consequences.
Some code that executed after Sema that either produced errors or processed resources with these now forbidden types was removed. Most of this is from CodeGen and HLOperationLower where code was generated or expanded for load/store operations on such resources.
Additionally moved the check for overlarge elements in typed resources to enable keeping some tests around that tested these side by side as well as adding a new one. I changed the nature of the check a bit to limit the types to four elements and also that they be no larger than 4 32-bit elements. This means that 64-bit elements are limited to two. While there wasn't an error that caught this before, there was an assert that fired.
Incidentally fixes an issue with error reporting for modified types after the first mismatch of that intrinsic is encountered. Because the insertion code changed them in the temp array and the stored function type, but printed from the temp array and the later encounters didn't add the modifiers to the temp array, later errors would incorrectly leave off the reference. By retrieving the type from the function type, the reference is always preserved.
This required changing a fair number of tests that relied on this behavior including removing test cases and entire test files that existed solely for such tests.
Fixes #7080