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