FillOp allows complex ops, and filling a properly sized buffer with
a default zero complex number is implemented.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp | ||
---|---|---|
63 | Yes, it would be more future-proof to query the size in bytes from the data layout, even if it's for the element type for now (there is no support for complex type in the data layout, yet). I would also suggest to construct the DataLayout object at the pass entry and forward it down here to avoid recomputation. | |
283 | Nit: just cast to catch cases of other element types? | |
286 | Nit: Please make braces symmetric |
mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp | ||
---|---|---|
63 | I switched to using DataLayout, but to cache the computation will require changing AllocBufferCallbackFn (https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h#L213). I don't know if that would be a good change? The reason that change would be needed is because the alternative is to only use this in the default callback, but that can be constructed and used across multiple modules which leads to segfaults from using dangling pointers. |
mlir/lib/Interfaces/DataLayoutInterfaces.cpp | ||
---|---|---|
55–58 | Actually implemented here https://reviews.llvm.org/D100289 |
mlir/lib/Interfaces/DataLayoutInterfaces.cpp | ||
---|---|---|
55–58 | You can add that diff as parent for this to create a "stack" of diffs only showing relevant changes. Click "edit related revisions" on the top right, and then "edit parent revisions" |
mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp | ||
---|---|---|
56–57 | Use something like DataLayout::closest(size.getDefiningOp()), otherwise it's the default layout that ignores the specification. Incidentally, that is also why you may want to pass it in from above. | |
63 | I expect other things will eventually want data layout as well, so it's okay to change the callback signature. |
mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp | ||
---|---|---|
63 | +1 re. ok to change signature. @ftynse is there a future in which the DataLayout could be auto-whatever from a builder insertion point (haven't thought this through, just basic idea that passed through my mind) ? Also @maheshkhanwalkar for visibility if signature changes. |
mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp | ||
---|---|---|
63 | You can already do DataLayout::closest(builder.getInsertionPoint()). Doing it the other way around feels like a layering violation, builder isn't supposed to know about individual interface. |
I forwarded the DataLayout through several layers of functions. I started at the last point that the call sequence has the originating LinalgOp and passed it through as an additional argument rather than inside of any of the LinalgPromotionOption data structures. Please let me know what you think.
Use something like DataLayout::closest(size.getDefiningOp()), otherwise it's the default layout that ignores the specification. Incidentally, that is also why you may want to pass it in from above.