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

transformations: Implement shape-inference pass #3047

Merged
merged 12 commits into from
Aug 17, 2024

Conversation

PapyChacal
Copy link
Collaborator

@PapyChacal PapyChacal commented Aug 16, 2024

Implement a trait-based generic shape-inference pass, following the style of canonicalize, allowing for shape inference patterns to interoperate amongst dialect and all; Just hook existing stencil shape inference patterns in there for now.

Stacked on:

@PapyChacal PapyChacal added core xDSL core (ir, textual format, ...) transformations Changes or adds a transformatio labels Aug 16, 2024
@PapyChacal PapyChacal self-assigned this Aug 16, 2024
Copy link

codecov bot commented Aug 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.89%. Comparing base (5568380) to head (1f7b66a).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3047   +/-   ##
=======================================
  Coverage   89.89%   89.89%           
=======================================
  Files         415      416    +1     
  Lines       52120    52173   +53     
  Branches     8060     8063    +3     
=======================================
+ Hits        46852    46903   +51     
- Misses       3976     3977    +1     
- Partials     1292     1293    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PapyChacal PapyChacal marked this pull request as ready for review August 16, 2024 13:03
Copy link
Member

@superlopuh superlopuh left a comment

Choose a reason for hiding this comment

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

Very nice! When I did this in the Toy tutorial, I followed the MLIR version's approach of just modifying the type directly, instead of using a rewrite pattern: https://mlir.llvm.org/docs/Tutorials/Toy/Ch-4/
I'm now not sure which approach is prefereable. It feels like the one used in Toy is more straightforward, why not use that directly?

@PapyChacal
Copy link
Collaborator Author

PapyChacal commented Aug 16, 2024

Very nice! When I did this in the Toy tutorial, I followed the MLIR version's approach of just modifying the type directly, instead of using a rewrite pattern: https://mlir.llvm.org/docs/Tutorials/Toy/Ch-4/ I'm now not sure which approach is prefereable. It feels like the one used in Toy is more straightforward, why not use that directly?

I belive it's less generic, right? I need some level of worklist management for my pass to work independently of the application order. stencil shape inference also works backwards, inferring operand sizes from results. So to interoperate both direction, I think we need that?

I would argue for sugar to hook a simpler case like the Toy one in the bigger case if you really want to keep Toy simple yet integrate in this one. I also think it might be okay to just have a simple one on Toy for demo purposes, and it doesn't have to imply constraints on the core one?

Copy link
Collaborator

@n-io n-io left a comment

Choose a reason for hiding this comment

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

Looks great, thanks

Base automatically changed from emilien/shape-inference to main August 17, 2024 12:14
@PapyChacal PapyChacal merged commit b12d2a5 into main Aug 17, 2024
9 checks passed
@PapyChacal PapyChacal deleted the emilien/shape-inference-pass branch August 17, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core xDSL core (ir, textual format, ...) transformations Changes or adds a transformatio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants