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.
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
[IA][RISCV] Support VP loads/stores in InterleavedAccessPass #120490
[IA][RISCV] Support VP loads/stores in InterleavedAccessPass #120490
Changes from 3 commits
156661a
c869bec
3688672
32d8de5
6b7f8e9
f7242df
0e4bf69
1afd2cc
16a88d9
5cd5be4
16915c9
725dfad
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This returns a different sized mask from the interleaved case, but it looks like the interleave intrinsics don't currently check the mask type which is why this works?
E.g. I tried out
And it seems to compile. I think we're missing a constraint on the intrinsics.
I think we need to return a new constant splat vector with the segment type here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see it's because after the tuple types in #97992 we don't have another vector to add attach a
LLVMScalarOrSameVectorWidth
type to. I'm not sure if there's an easy fix for this cc) @4vtomatThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A potential solution could be allowing us to supply a custom code fragment to
LLVMScalarOrSameVectorWidth
to retrieve the vector type we want to match against. For instance, if the first overloading type is ariscv.vector.tuple
, we can use this to construct the mask type:where
Ty
is an implicitType *
variable representing the original intrinsic parameter type.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds good. I think that's out of the scope for this PR though, if we can just manually fix WideMask to return the segment type here I think that's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I just added that,
along with checking if the (non-all-ones) mask has the desired vector lengthnvm, we don't need such check.