[mlir][tensor][nfc] Clarify comments for `createPadHighOp` While reviewing this code, I realised that the rationale behind the assert was not very clear, so I updated the comments to clarify it. Relaxing the assert (i.e., allowing `resType.getNumDynamicDims() != dynOutDims.size()`) would require generating a mapping between `dynOutDims` and the dynamic dimensions of the output tensor. At the moment, this additional complexity is unnecessary. To minimize PR noise, I am submitting this without a review. However, please ping me if you believe this or similar changes should be reviewed before merging.
diff --git a/mlir/include/mlir/Dialect/Tensor/Utils/Utils.h b/mlir/include/mlir/Dialect/Tensor/Utils/Utils.h index 83cc665..22ca8a9 100644 --- a/mlir/include/mlir/Dialect/Tensor/Utils/Utils.h +++ b/mlir/include/mlir/Dialect/Tensor/Utils/Utils.h
@@ -20,16 +20,17 @@ // width is calculated as: resDim - sourceDim. // // Handling static sizes is trivial. Dynamic dimensions are trickier (*): -// 1. dynamic input sizes are extracted from `source` -// 2. for dynamic output dims, there are two options: -// 2.1 all output dynamic dim sizes are specified in `dynOutDim`, -// 2.2 `dynOutDim` is empty and the corresponding padding width is set to 0. +// 1. Dynamic input sizes are extracted from `source` (e.g. via `tensor.dim`). +// 2. For dynamic output dims, there are two options: +// 2.1 All output dynamic dim sizes are specified in `dynOutDims`, or +// 2.2 `dynOutDims is empty - the padding width for all the output dynamic +// dims is set to 0. // // (*) Note that `resType` is just a shape and it only encodes the actual sizes // for _static_ dimensions. PadOp createPadHighOp(RankedTensorType resType, Value source, Value pad, bool nofold, Location loc, OpBuilder &builder, - SmallVector<Value> dynOutDim = {}); + SmallVector<Value> dynOutDims = {}); // Creates dim ops for each dynamic dimension of the ranked tensor argument and // returns these as values.
diff --git a/mlir/lib/Dialect/Tensor/Utils/Utils.cpp b/mlir/lib/Dialect/Tensor/Utils/Utils.cpp index 52462aa..c3d5675 100644 --- a/mlir/lib/Dialect/Tensor/Utils/Utils.cpp +++ b/mlir/lib/Dialect/Tensor/Utils/Utils.cpp
@@ -27,6 +27,8 @@ OpBuilder &b, SmallVector<Value> dynOutDims) { + // This assumption simplifies the following logic without limiting what's + // required _today_. If needed, we can relax it in the future. assert(((resType.getNumDynamicDims() == dynOutDims.size()) || dynOutDims.empty()) && "Either none or all output dynamic dims must be specified!");