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

[SPIR-V] Handle vectors passed to asuint #6953

Merged
merged 5 commits into from
Oct 29, 2024

Conversation

cassiebeckley
Copy link
Collaborator

@cassiebeckley cassiebeckley commented Oct 9, 2024

asuint should be able to take vectors in addition to scalar values. Previously, it would be lowered as a bitcast from the input value to a vector of uints with a width of 2, which is not large enough if the input value is larger than a scalar value. In order to handle, for example, an input value that is a double4, we instead perform a component-wise bitcast.

Fixes #6735

@cassiebeckley cassiebeckley requested a review from a team as a code owner October 9, 2024 23:22
@cassiebeckley
Copy link
Collaborator Author

@s-perron lmk if this approach looks correct to you. If it does, I think I'll need to update dead code elimination again, since the case where a temporary variable is being used is having the OpStore removed.

`asuint` should be able to take vectors in addition to scalar values.
Previously, it would be lowered as a bitcast from the input value to a
vector of uints with a width of 2, which is not large enough if the
input value is larger than a scalar value. In order to handle, for
example, an input value that is a `double4`, we instead perform a
component-wise bitcast.

Fixes microsoft#6735
Copy link
Contributor

github-actions bot commented Oct 14, 2024

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

@cassiebeckley cassiebeckley enabled auto-merge (squash) October 14, 2024 21:46
@cassiebeckley cassiebeckley enabled auto-merge (squash) October 14, 2024 21:46
Copy link
Collaborator

@s-perron s-perron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I would tests some more cases. I think they will work, but I'm not sure.

tools/clang/lib/SPIRV/SpirvEmitter.cpp Outdated Show resolved Hide resolved
tools/clang/lib/SPIRV/SpirvEmitter.cpp Outdated Show resolved Hide resolved
tools/clang/lib/SPIRV/SpirvEmitter.h Outdated Show resolved Hide resolved
tools/clang/lib/SPIRV/SpirvEmitter.cpp Outdated Show resolved Hide resolved
tools/clang/lib/SPIRV/SpirvEmitter.cpp Show resolved Hide resolved
@cassiebeckley cassiebeckley merged commit 5704c47 into microsoft:main Oct 29, 2024
12 checks passed
@cassiebeckley cassiebeckley deleted the asuint-vector branch October 30, 2024 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[SPIR-V] asuint fails on bit cast
3 participants