-
Notifications
You must be signed in to change notification settings - Fork 718
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
Create new raw buffer load lowering function #7144
Open
pow2clk
wants to merge
7
commits into
microsoft:main
Choose a base branch
from
pow2clk:longvec_bab_ldst_pr
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
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
✅ With the latest revision this PR passed the C/C++ code formatter. |
Note this has dependencies. only this range is new relevant code: https://github.com/microsoft/DirectXShaderCompiler/pull/7144/files/fad8db095f71d3cfa395348647ae64203c501df5..039284bcf6bd924cbb5242450f2f9f5c483b59f8 |
1d601a1
to
ed6fd02
Compare
The array type checks were for a type extracted from a vector. Arrays cannot be elements of vectors. This is leftover from when that was not the case. The code shows as never executed in coverage
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. Remove code rendered dead by disallowing typebuff aggregates Update tests for changes to allowed typed buffer elements 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. Fixes microsoft#7080
Disentangles the raw buffer lowering implmentation into an isolated function. Alters the various places that lowering took place to call into the common function. This function will be expanded to handle other lowering later. When raw buffers use a templated load with a struct, they reuse the subscript path also used for subscripted structured buffers. Such loads with structs containing vectors or matrices will invoke the load lowering from within this recursive call that traverses GEPs and other users of the original call to set up correct offsets etc. This adapts that code to use the common load lowering that enables long vectors within structs to be correctly loaded. Since the code expects byte address buffers, it is not (yet) adapted to structured buffers, so those code paths are kept as they were. This requires the ability to override the type used by the resloadhelper explicitly, so a member is added to accomodate the matrices vector representation that doesn't match the types of the load call. This also requires removing the bufIdx and offset swapping that was done, confusingly throughout the TranslateStructBufSubscriptUser code to account for the fact that byte address buffers have to represent offsets using the main coord parameter in favor of passing the Resource Kind down such that the right parameter can receive the incrementation when necessary for longer types such as matrices. This is enabled also by adding ResKind appropriate offset calculation in the ResLoadHelper. ResLoadHelper also gets an opcode set based on the ResKind for both overaloads in preparation for further expansion to different resource kinds.
ed6fd02
to
2d35de9
Compare
2d35de9
to
039284b
Compare
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.
Disentangles the raw buffer lowering implementation into an isolated
function. Alters the various places that lowering took place to call
into the common function. This function will be expanded to handle other
lowering later.
When raw buffers use a templated load with a struct, they reuse the
subscript path also used for subscripted structured buffers. Such loads
with structs containing vectors or matrices will invoke the load
lowering from within this recursive call that traverses GEPs and other
users of the original call to set up correct offsets etc.
This adapts that code to use the common load lowering that enables long
vectors within structs to be correctly loaded.
Since the code expects byte address buffers, it is not (yet) adapted to
structured buffers, so those code paths are kept as they were.
This requires the ability to override the type used by the resloadhelper
explicitly, so a member is added to accommodate the matrices vector
representation that doesn't match the types of the load call.
This also requires removing the bufIdx and offset swapping that was
done, confusingly throughout the TranslateStructBufSubscriptUser code to
account for the fact that byte address buffers have to represent offsets
using the main coord parameter in favor of passing the Resource Kind
down such that the right parameter can receive the incrementation when
necessary for longer types such as matrices. This is enabled also by
adding ResKind appropriate offset calculation in the ResLoadHelper.
ResLoadHelper also gets an opcode set based on the ResKind for both
overloads in preparation for further expansion to different resource
kinds.
Fixes #7118