This commit adds a tiling op to the transform dialect as an external op.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h | ||
---|---|---|
2 | Nit: wrong filename | |
14 | Nit: wouldn't OpDefinition.h suffice in the header? | |
mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td | ||
10 | These aren't really linear algebra operations, but transforms thereof. | |
mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp | ||
64 | function_ref (std::function is only useful if the callee needs to outlive the caller) | |
67–68 | Is it possible to reserve space for these? | |
71 | I'd rather dyn_cast and report a user-visible error here. | |
82–84 | Nit: drop trivial braces. | |
88–90 | Nit: drop trivial braces. | |
118 | Ultra-nit: we tend to use single-quotes rather than backticks in user-visible text. | |
121 | Don't we have something like getSizesAttrName() to avoid magic strings? | |
124 | Nit: there is no need to use formatv to insert one statically known string into another statically known string, just write the full string. I suppose this could have been taken from some other place that used getSizesAttrName() instead of hardcoding the attribute name. Even in that case, something like parser.emitError(opLoc) << "expected " << getSizesAttrName() << " attribute"; is probably cheaper / easier for the compiler to optimize than formatv. | |
160 | We also need Read/Write effect on PayloadIRResource, not attached to a particular value. |
mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h | ||
---|---|---|
14 | Actually requires OpImplementation.h because OpAsmOpInterface is needed. | |
mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.td | ||
10 | Ah, this was copy pasta... | |
mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp | ||
121 | We do, but getSizesAttrName is a member function and TileOp::parse() is static. |
mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp | ||
---|---|---|
121 | Isn't there a static version that takes OperationName as argument? You can get OperationName from OperationState. |
address comments
mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp | ||
---|---|---|
121 | Nice I didn't know that |
Nit: wrong filename