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: (stencil-tensorize-z-dim) Support loops #3118

Merged
merged 2 commits into from
Sep 2, 2024

Conversation

n-io
Copy link
Collaborator

@n-io n-io commented Aug 30, 2024

Add support for enclosing unknown ups such as scf.for

@n-io n-io added the transformations Changes or adds a transformatio label Aug 30, 2024
@n-io n-io requested review from PapyChacal and dk949 August 30, 2024 09:02
@n-io n-io self-assigned this Aug 30, 2024
Copy link

codecov bot commented Aug 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.95%. Comparing base (37702db) to head (8bd0fa7).
Report is 20 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3118      +/-   ##
==========================================
+ Coverage   89.90%   89.95%   +0.04%     
==========================================
  Files         421      425       +4     
  Lines       53306    53568     +262     
  Branches     8263     8297      +34     
==========================================
+ Hits        47927    48186     +259     
- Misses       4041     4043       +2     
- Partials     1338     1339       +1     

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

@dk949
Copy link
Collaborator

dk949 commented Sep 2, 2024

Could you please add a description as to what this PR aims to achieve?

Copy link
Collaborator

@dk949 dk949 left a comment

Choose a reason for hiding this comment

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

I don't fully understand what this change does. Is it to relax the restrictions on what can be tensorized?

@n-io
Copy link
Collaborator Author

n-io commented Sep 2, 2024

I don't fully understand what this change does. Is it to relax the restrictions on what can be tensorized?

The type conversion pass helps dealing with unknown ops. Notably, including scf.for. Running the type conversion pass first has the effect of the old checks "input is tensorised and output is not tensorised" no longer working. So the ExternalLoadOp conversion was not needed at all, and some other rewrite passes needed their checks (esp for output) revisited.

@dk949
Copy link
Collaborator

dk949 commented Sep 2, 2024

I don't fully understand what this change does. Is it to relax the restrictions on what can be tensorized?

The type conversion pass helps dealing with unknown ops. Notably, including scf.for. Running the type conversion pass first has the effect of the old checks "input is tensorised and output is not tensorised" no longer working. So the ExternalLoadOp conversion was not needed at all, and some other rewrite passes needed their checks (esp for output) revisited.

Makes sense, thank you

@n-io n-io merged commit 765496c into main Sep 2, 2024
14 checks passed
@n-io n-io deleted the nicolai/csl-tensorize-with-loops branch September 2, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
transformations Changes or adds a transformatio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants