Allow for dynamic indices in the dim operation.
Rather than an attribute, the index is now an operand of type index.
This allows to apply the operation to dynamically ranked tensors.
The correct lowering of dynamic indices remains to be implemented.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM from my side with some nits. Thanks for adding this!
mlir/include/mlir/Dialect/StandardOps/IR/Ops.td | ||
---|---|---|
1379 | I had to read this multiple times to understand what the dim operation's type is. Its result type is always index. I think you wanted to say something like The specified type is the type of the ... to note what the type after the : means. | |
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp | ||
2121 | I would prefer return failure() here, as that will provide context using the usual mlir machinery rather than assert and abort. | |
mlir/lib/Dialect/StandardOps/IR/Ops.cpp | ||
1271 | You can use ConstantIndexOp here and avoid creating the attribute explicitly. |
@frgossen : the pre-merge build is failing on this revision, please check that it is passing or that you understand the failure before pushing.
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp | ||
---|---|---|
2121 | nit: Remove the username from the TODO | |
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | ||
209 | nit: Use llvm::TypeSwitch here instead, should be something like: TypeSwitch<Operation *, bool>(dimOp.memrefOrTensor().getDefiningOp()) .Case<ViewOp, SubViewOp, AllocOp>([&](auto op) { return isMemRefSizeValidSymbol(op, i, region); }) .Default([](Operation *) { return false; }); | |
mlir/lib/Dialect/StandardOps/IR/Ops.cpp | ||
1277 | nit: Remove trivial braces. |
Can you add documentation for what happens in the case where the dimension index is out of bounds? Are we allowed to assume that doesn't happen?
mlir/test/Conversion/VectorToSCF/vector-to-loops.mlir | ||
---|---|---|
319 | You missed a few captures for the constant values here. |
I had to read this multiple times to understand what the dim operation's type is. Its result type is always index. I think you wanted to say something like The specified type is the type of the ... to note what the type after the : means.