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
[LLVM] Add
llvm.experimental.vector.compress
intrinsic #92289[LLVM] Add
llvm.experimental.vector.compress
intrinsic #92289Changes from 28 commits
3a7b064
75abf0b
0329bc9
73bfebb
e4423a1
3e99678
b686f83
73cc28f
8a613f3
a4df959
17004b9
984cad1
0ea2415
1dc79b4
c8515ca
8353b2d
a9aba29
b0af320
d9587c7
c04da9b
a60523c
9279e5e
b48dada
d443671
d45f61b
4366a43
808709f
d5fca0f
00b64d2
60f9c61
3faed13
6aa480c
6549386
5589519
f7e4b48
9f291c8
c733d6b
9222b54
616c142
daa784f
3e4673c
4b12b32
6c9c969
069eb24
457cbdf
5e3189b
6b3a3b8
50cce23
89c3e9f
995c863
adb9d9c
ba2939e
11e1742
32cc27f
99610a8
efa2e92
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.
The StackID is not the frame index, This just happens to work because the default stack ID is 0 and the one object you have is also a 0. The ABI for this is unfortunately awkward.
The API here could probably use work. createStackTemporary returns the G_FRAME_INDEX which contains the index you really want. We could maybe return the instruction and the actual index as a pair from createStackTemporary?
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.
Just to validate my understanding: it is possible for
CreateStackObject
to change the alignment, so we technically have no guarantee that the alignment that we pass in is the stack's alignment that we actually use?I've seen two other approaches to this in
LegalizerHelper
.pass the alignment to
createStackTemporary()
and then just reuse that alignment object later on (somewhat similar to my first approach but without the redundant call to get the alignment). If my understanding is correct, this could technically lead to wrong alignment if the requested alignment is larger thanMachineFrameInfo::StackAlignment
.a somewhat awkward
FIDef->getOperand(1).getIndex()
inlowerMemset()
, which I think could easily break if something changes.llvm-project/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
Line 8615 in d5fca0f
I think changing the API to make it easier to get the real alignment makes sense. But this is probably a separate PR, as this would mean each caller must think about what happens when the input alignment is
!=
the frame's alignment. In light of this, I'd suggest to useVecAlign
here, as done in approach 1) with a TODO to fix this when the API changes.