dialects: Add memref.extract_strided_metadata op #2748
Conversation
JosseVanDelm
added
enhancement
| @staticmethod | ||
| def from_source_memref( | ||
| source: SSAValue | Operation, source_type: MemRefType[Attribute] | ||
| ): |
There was a problem hiding this comment.
Can you make this an __init__? That's what we are trying to move towards in xDSL
There was a problem hiding this comment.
Is this an init though? creating a bare op of this kind should not really include a source type, should it?
There was a problem hiding this comment.
I remember being torn by the very same thought on subview itself;
I propose you just do what's still done there, and @AntonLydike could file an issue or Zulip thread to discuss it independently?
There was a problem hiding this comment.
Hmmm, I don't know if I can follow. How else would you create this op if not from a source memref?
There was a problem hiding this comment.
Maybe you could clear this up by adding a docstring to the method saying exactly what it's for and what each operand does?
There was a problem hiding this comment.
May I ask why source and source_type are passed in separately? I'd expect source to come with a type
xdsl/dialects/memref.py
Outdated
| return ExtractStridedMetaDataOp.build( | ||
| operands=[source], result_types=[*return_type] | ||
| ) | ||
|
|
There was a problem hiding this comment.
In the init you can replace this with super().__init__(operands=[source], result_types=[*return_type])
There was a problem hiding this comment.
Also [*return_type] is the same as return_type
xdsl/dialects/memref.py
Outdated
| strides_type = [IndexType() for _ in source_shape] | ||
| sizes_type = [IndexType() for _ in source_shape] |
There was a problem hiding this comment.
You could write this as [IndexType()] * len(source_shape), but I don't know if you want to^^
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2748 +/- ##
=======================================
Coverage 89.90% 89.91%
=======================================
Files 367 367
Lines 47324 47355 +31
Branches 7231 7234 +3
=======================================
+ Hits 42549 42580 +31
Misses 3662 3662
Partials 1113 1113 ☔ View full report in Codecov by Sentry. |
I'm adding the memref.extract_strided_metadataop because I need an op to fetch the dynamic offset created by a subview, and otherwise I can not do that.
https://mlir.llvm.org/docs/Dialects/MemRef/#memrefextract_strided_metadata-memrefextractstridedmetadataop