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

[Flow] Add pattern to canonicalize consecutive pads #17878

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

qedawkins
Copy link
Contributor

This allows fusing pad(pad) into a single pad with the same padding
value. This is added here due to the inclusion of the Affine dialect and
upstream tensor not depending on Affine.

@qedawkins qedawkins marked this pull request as ready for review July 14, 2024 19:59
Copy link
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

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

Q: do we move the pattern to GlobalOpt?

@qedawkins
Copy link
Contributor Author

Q: do we move the pattern to GlobalOpt?

Currently GlobalOpt and Flow are sharing the same "CanonicalizeExt" pass, so this is the place that makes the most sense (other than upstream, this pattern should probably go upstream). To move it to GlobalOpt would require making a GlobalOpt specific canonicalization pass, and this pattern fits well in both places.

@qedawkins qedawkins requested a review from hanhanW August 5, 2024 23:18
@IanWood1
Copy link
Contributor

IanWood1 commented Aug 6, 2024

Q: do we move the pattern to GlobalOpt?

Currently GlobalOpt and Flow are sharing the same "CanonicalizeExt" pass, so this is the place that makes the most sense (other than upstream, this pattern should probably go upstream). To move it to GlobalOpt would require making a GlobalOpt specific canonicalization pass, and this pattern fits well in both places.

Would this canonicalization pass be used in the planned dispatch partitioning/preprocessing "phase" #18099?

@qedawkins
Copy link
Contributor Author

I would assume so, yes. Flow could potentially just use a default canonicalization pass after that split also.

This allows fusing `pad(pad)` into a single pad with the same padding
value. This is added here due to the inclusion of the Affine dialect and
upstream tensor not depending on Affine.
@qedawkins qedawkins enabled auto-merge (squash) August 7, 2024 19:04
@qedawkins qedawkins merged commit e341692 into iree-org:main Aug 7, 2024
44 checks passed
@qedawkins qedawkins deleted the compose_pads branch August 7, 2024 19:54
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.

4 participants