transformations: Add support for strided MemRefs in 'load' and 'store' #2854
Conversation
Co-authored-by: Markus Böck <markus.boeck02@gmail.com>
The lowering previously assumed an identity layout and errored if a strided layout was present. Strided layouts are unfortunately very common as they are a direct result of tiling. This PR therefore adds support for them in both the store and load operations. As a side-effect, the `memref.subview` implementation could also be heavily simplified.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2854 +/- ##
==========================================
- Coverage 89.89% 89.88% -0.01%
==========================================
Files 398 398
Lines 49659 49626 -33
Branches 7633 7634 +1
==========================================
- Hits 44641 44607 -34
- Misses 3821 3822 +1
Partials 1197 1197 ☔ View full report in Codecov by Sentry. |
|
Yeah this looks much better. I already pushed to the other branch, causing some conflicts. It seems like a strict improvement over my PR, so maybe you could retarget your changes to main, and then I can rebase whatever other positive changes I have in my PR on top of this one? |
That's a good idea! I think your PR should then also support subviews of strided MemRefs. Note that I actually found a discrepency in the generated code (final offset being added) between our two PRs that I have not yet figured out whose at fault. But we can figure this out in your PR. |
|
Sounds good. Please ping me when you've retargeted to main and the PR is ready for review. I'll then retarget mine. Can try to hunt for bugs once everything is in order :) |
The lowering previously assumed an identity layout and errored if a strided layout was present. Strided layouts are unfortunately very common as they are a direct result of tiling. This PR therefore adds support for them in both the store and load operations. As a side-effect, the `memref.subview` implementation could also be heavily simplified.
Should be good to go now. Some small utils (the |
strides are not allowed to cause "internal" aliasing.
The lowering previously assumed an identity layout and errored if a strided layout was present. Strided layouts are unfortunately very common as they are a direct result of tiling. This PR therefore adds support for them in both the store and load operations.