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

[AIE2] Check all divisions for shufflevector extraction #100

Open
wants to merge 1 commit into
base: vvandebe.shufflevector.add.intrinsics
Choose a base branch
from

Conversation

ValentijnvdBeek
Copy link
Collaborator

Optimizes the additional cases of the extraction intrinsics. These are the cases where the destination vector fits more than twice in the target vector.

// This optimization does not work if the target type is not a power of
// two, this can happen in some backends that support uneven vector types.
// We also need to make sure that the vector can be split into two.
if (SrcTy == DstTy || ((SrcNumElts / 2) % 2) != 0 ||
Copy link
Collaborator

@martien-de-jong martien-de-jong Jun 28, 2024

Choose a reason for hiding this comment

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

I don't understand the SrcNumElts condition. 5 will get rounded down by the division to an even number. Also, the comment says 'power of two', but I see no test for that. Perhaps you mean 'multiple of two' ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh yeah, I forgot that in C/C++ it is binary division. The reason why this still works is that the broken case is AMDGPU which uses 3 which does become uneven, but that could be done.

The wording is confusing, here we just need to check if the given type is even. I don't know how to word it precisely, but my intention is to communicate is that we assume that the architecture targets a power of two. This optimization breaks at the point whenever we get an uneven number of elements of our vector. The call itself can have uneven numbers provided it is able to split at least once cleanly.

@ValentijnvdBeek ValentijnvdBeek changed the base branch from vvandebe.shufflevector.intrinsics to vvandebe.shufflevector.add.intrinsics August 2, 2024 15:02
@ValentijnvdBeek ValentijnvdBeek force-pushed the vvandebe.shufflevector.check.all.intersections branch from 8ceb792 to 83f6f9d Compare August 2, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:aie2 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants