[SM6.9] Allow native vectors longer than 4 #7143
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
added 2 commits
February 17, 2025 21:49
Remove errors in Sema diagnostics for vectors longer than 4 in 6.9. Test for failures using long vectors in unspported contexts and for correct codegen in supported contexts. Verify errors persist in pre-6.9 shader models The type buffer cache expects a max vector size of 4. By just skipping the cache for longer vectors, we don't overrun and store float7 vectors in the double3 slot or retrieve the double3 in place of float7. Testing is for acceptance, mangling and basic copying that takes place at the high level to ensure they are being accepted and recognized correctly. The intent is not to tully test the passing of data as that requires enabling vector operations to do properly. This test is used to verify that these same constructs are disallowed in 6.8 and earlier. A separate test verifies that disallowed contexts produce the appropriate errors Fixes microsoft#7117
Disallow long vectors, and arrays or structs containing long vectors in cbuffers, entry functions, node records, tessellation patchs, or special intrinsic parameters with user-defined struct parameters.
This got lost somewhere
alsepkow
reviewed
Feb 18, 2025
alsepkow
reviewed
Feb 19, 2025
llvm-beanz
reviewed
Feb 19, 2025
tex3d
reviewed
Feb 19, 2025
llvm-beanz
reviewed
Feb 19, 2025
tex3d
reviewed
Feb 19, 2025
tex3d
reviewed
Feb 19, 2025
pow2clk
commented
Feb 20, 2025
tools/clang/test/SemaHLSL/hlsl/types/invalid_longvec_decls.hlsl
Outdated
Show resolved
Hide resolved
added 5 commits
March 3, 2025 09:40
Expand resource attribute to all resource types by adding reskind and resclass arguments indicating the specific resource type. Change detection in HlslTypes to use these attribute arguments. Similarly add vertex number arguments to output stream attribute and a boolean indicator of input or output for tessellation patches. Add geomstream attr to detect those objects Use attribute to detect tesselation patches Removes template arg counts and startswith stirngs to identify tesslations patches and distinguish them from multisampled textures
Add setting for max vec size. Determine long vector presence using DefinitionData bit? OR Rename testing for long vecs function? Add attribute for geometry streams, produce and test errors for long vectors there. Add and test errors for > 1024 element vectors. Add vector size to error messages good test changes
Go for consistent test filename formatting. most LLVM tests have dashes, so dashes it is. Remove redundant sm68 test
Expand existing tests to different target and contexts. Add thorough testing for geometry streams and tessellation patches. Add toolong vector test. Verify that vectors that are over the maximum for 6.9 fail. Add subobjects and template classes to tests. These are unfortunately disabled because the code to make them work causes other tests to fail.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Use RequireCompleteType to force specialization of templates encountered in global and other scopes where finding long vectors is necessary where possible. This populates the definitiondata which contains the base class chain needed to detect when a base class has disqualifying long vectors. It was also needed to detect when dependent types in a template class result in long vectors. Work graph node types didn't check their base classes for failures. This affects base classes with longvectors that have sub classes used for node objects which should fail for having long vector members. Respond to feedback about iterating through fields in clunky manner which got left out of the last reviewer feedback response
I guess it was about time. Should simplify some things later as well as at present and it was too easy to not do. Specifically, I was going to need to add another string check to the template instantiation code to identify longvectors. This is cleaner. Incidentally convert another feedback texture string check to use attribs. Incidentally resort the recently-added attribs to not break up the node shader attribs.
Vector types can be cached in a 2D array that has a column for lenghts 1-4. This uses the added contant to indicate the length and for the checks that confirm it isn't exceeded.
By setting the bit when the vector template is instantiated and then propagating it when members, be they standard members or base classes, the bit will be set correctly for any struct or struct-like type. For arrays, the arrays are pealed away in a utility function to get at the elements. Decided to separate the check for completeness from the check for long vectors. Even though the latter almost always requires the former, they are separate concepts and embedding the first in the second would be unexpected
Output Streams, Tessellation patches, and global variables should be complete when receiving other correctness checks. If they cannot be made complete, they should produce an error. This was omitted for various of these including non-template globals, which was fine, but it meant that redundant errors were produced for templates, but not standard globals likely just because that was what was tested. This removes that distinction and adds testing for all of the above to the existing incomplete-type.hlsl test.
alsepkow
reviewed
Mar 6, 2025
tex3d
reviewed
Mar 7, 2025
remove some stale elements. Add some HLSL type helper functions and add some new ones. Make resource type retreiveals type-safe. Add some parameter comments and names to make clearer what the effect of them are. Pass resource attribute to cbuffer/tbuffer creation. Clean up and clarify error messages. Remove redundant type canonization from type queries. Correct resclass of tbuffers. Use multimatch utility of verify to condense checks
llvm-beanz
reviewed
Mar 10, 2025
tex3d
reviewed
Mar 10, 2025
tex3d
reviewed
Mar 10, 2025
Correct UAVness of consume/append buffers. Add HLSL notes for changes to DeclCXX Share more code in IsHLSLVecMatType
llvm-beanz
approved these changes
Mar 10, 2025
alsepkow
reviewed
Mar 11, 2025
| // RUN: %dxc -T ps_6_9 -DTYPE=float -DNUM=1025 -verify %s | ||
|
|
||
| // A test to verify that declarations of longvecs are permitted in all the accepted places. | ||
| // Only tests for acceptance, most codegen is ignored for now. |
There was a problem hiding this comment.
When you say "most codegen is ignored for now" do you mean until long vecs are implemented? Do we intend to use this hlsl file to verify codegen at some point? Just wondering if this comment is going to become stale.
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.
Remove errors in Sema diagnostics for vectors longer than 4 in 6.9.
Test for failures using long vectors in unspported contexts and for correct codegen in
supported contexts. Verify errors persist in pre-6.9 shader models
The type buffer cache expects a max vector size of 4. By just skipping the cache for longer vectors, we don't overrun and store float7 vectors in the double3 slot or retrieve the double3 in place of float7.
Testing is for acceptance, mangling and basic copying that takes place
at the high level to ensure they are being accepted and recognized
correctly. The intent is not to tully test the passing of data as that
requires enabling vector operations to do properly. This test is used to
verify that these same constructs are disallowed in 6.8 and earlier.
A separate test verifies that disallowed contexts produce the
appropriate errors
Fixes #7117