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

[Arc] Keep just one parameter if it's given multiple times #7284

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

elhewaty
Copy link
Member

@elhewaty elhewaty commented Jul 5, 2024

This pull request adds a new canonical form for the arc.vectorize operation.
Given the following IR

%1:4 = arc.vectorize(%0#0, %0#1, %0#2, %0#3), (%a, %b, %c, %d), (%0#0, %0#1, %0#2, %0#3)

We can remove the repeated input to simplify it as:

%1:4 = arc.vectorize(%0#0, %0#1, %0#2, %0#3), (%a, %b, %c, %d)

This can reduce the time of the unpacking of vector elements.

@elhewaty
Copy link
Member Author

elhewaty commented Jul 5, 2024

This can be generalized to remove any repeated pattern in the input, what do you think?
If the input patterns are scalars that won't be useful (I think), but if they are vectors maybe we can save more extraction time?

@elhewaty elhewaty requested review from TaoBi22 and Moxinilian July 5, 2024 11:13
@elhewaty elhewaty changed the title [ARC] Keep just one result as a parameter if the result is given multiple times [Arc] Keep just one result as a parameter if the result is given multiple times Jul 5, 2024
Copy link
Member

@maerhart maerhart left a comment

Choose a reason for hiding this comment

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

Yes, I think this is a useful canonicalization regardless of how much you can save in vector packing. 👍
It can reduce the use-count of values and thus enable more applications of your other canonicalization pattern, for example.

lib/Dialect/Arc/Transforms/ArcCanonicalizer.cpp Outdated Show resolved Hide resolved
lib/Dialect/Arc/Transforms/ArcCanonicalizer.cpp Outdated Show resolved Hide resolved
lib/Dialect/Arc/Transforms/ArcCanonicalizer.cpp Outdated Show resolved Hide resolved
test/Dialect/Arc/arc-canonicalizer.mlir Outdated Show resolved Hide resolved
@elhewaty elhewaty force-pushed the EraseMultipleVecUses branch 2 times, most recently from cb7bafb to ab92a68 Compare July 5, 2024 17:25
@elhewaty elhewaty requested a review from maerhart July 5, 2024 17:26
@elhewaty elhewaty changed the title [Arc] Keep just one result as a parameter if the result is given multiple times [ARC] Keep just one parameter if it's given multiple times Jul 5, 2024
@elhewaty elhewaty force-pushed the EraseMultipleVecUses branch from ab92a68 to c046812 Compare July 5, 2024 18:24
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

This is a very useful canonicalization! LGTM, besides a minor comment about SmallVector maybe not being necessary.

lib/Dialect/Arc/Transforms/ArcCanonicalizer.cpp Outdated Show resolved Hide resolved
@elhewaty elhewaty force-pushed the EraseMultipleVecUses branch from c046812 to a38f140 Compare July 9, 2024 19:47
@elhewaty elhewaty requested a review from fabianschuiki July 9, 2024 19:47
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

LGTM!

@elhewaty elhewaty changed the title [ARC] Keep just one parameter if it's given multiple times [Arc] Keep just one parameter if it's given multiple times Jul 9, 2024
@elhewaty elhewaty merged commit ff02e7a into llvm:main Jul 9, 2024
4 checks passed
@elhewaty elhewaty deleted the EraseMultipleVecUses branch July 9, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants