Skip to content
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
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

pow2clk
Copy link
Member

@pow2clk pow2clk commented Feb 18, 2025

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

@pow2clk pow2clk requested a review from a team as a code owner February 18, 2025 16:25
Copy link
Contributor

github-actions bot commented Feb 18, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@pow2clk
Copy link
Member Author

pow2clk commented Feb 18, 2025

@pow2clk pow2clk force-pushed the longvec_bab_ldst_pr branch from 1d601a1 to ed6fd02 Compare February 18, 2025 16:28
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.
@pow2clk pow2clk force-pushed the longvec_bab_ldst_pr branch from ed6fd02 to 2d35de9 Compare February 18, 2025 16:33
@pow2clk pow2clk force-pushed the longvec_bab_ldst_pr branch from 2d35de9 to 039284b Compare February 18, 2025 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New
Development

Successfully merging this pull request may close these issues.

[SM6.9] Templated Load and Store long vectors from and to byteaddressbuffers
1 participant