-
Notifications
You must be signed in to change notification settings - Fork 79
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
dialects: stencil: Allow loading and storing from a same field. #3071
Conversation
…he specific lowering assuming it.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3071 +/- ##
==========================================
- Coverage 89.90% 89.90% -0.01%
==========================================
Files 418 418
Lines 52913 52898 -15
Branches 8202 8200 -2
==========================================
- Hits 47571 47556 -15
Misses 4015 4015
Partials 1327 1327 ☔ View full report in Codecov by Sentry. |
|
||
// CHECK: "stencil.store"(%2, %0) {"bounds" = #stencil.bounds<[0, 0, 0], [64, 64, 64]>} : (!stencil.temp<[0,64]x[0,64]x[0,64]xindex>, !stencil.field<[0,64]x[0,64]x[0,64]xindex>) -> () | ||
// CHECK-NEXT: ^^^^^^^^^^^^^^^------------------------------------------------------------------------------------------------------------------------- | ||
// CHECK-NEXT: | Error while applying pattern: Cannot lower directly if storing to the same field multiple times! Try running `stencil-bufferize` before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, we still keep this, as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kinda, as a transition; This is now only an error to try and use the old lowering on such a case!
So you can now express it without hacks, and in most cases lower it simply by first running stencil-bufferize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Where before, it was considered an error to just write it!)
Now that
stencil-bufferize
can deal with those cases, lift this restriction at the stencil dialect level.Keep the check in the specific old conversion patterns still assuming this property for least surprise. Point user to use
stencil-bufferize
, which if successful makes those operations disappear.