Skip to content

Commit 91c0aa5

Browse files
committed
[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.
1 parent 31824b2 commit 91c0aa5

File tree

2 files changed

+8
-5
lines changed

2 files changed

+8
-5
lines changed

mlir/include/mlir/Dialect/Tensor/Utils/Utils.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,17 @@ namespace tensor {
2020
// width is calculated as: resDim - sourceDim.
2121
//
2222
// Handling static sizes is trivial. Dynamic dimensions are trickier (*):
23-
// 1. dynamic input sizes are extracted from `source`
24-
// 2. for dynamic output dims, there are two options:
25-
// 2.1 all output dynamic dim sizes are specified in `dynOutDim`,
26-
// 2.2 `dynOutDim` is empty and the corresponding padding width is set to 0.
23+
// 1. Dynamic input sizes are extracted from `source` (e.g. via `tensor.dim`).
24+
// 2. For dynamic output dims, there are two options:
25+
// 2.1 All output dynamic dim sizes are specified in `dynOutDims`, or
26+
// 2.2 `dynOutDims is empty - the padding width for all the output dynamic
27+
// dims is set to 0.
2728
//
2829
// (*) Note that `resType` is just a shape and it only encodes the actual sizes
2930
// for _static_ dimensions.
3031
PadOp createPadHighOp(RankedTensorType resType, Value source, Value pad,
3132
bool nofold, Location loc, OpBuilder &builder,
32-
SmallVector<Value> dynOutDim = {});
33+
SmallVector<Value> dynOutDims = {});
3334

3435
// Creates dim ops for each dynamic dimension of the ranked tensor argument and
3536
// returns these as values.

mlir/lib/Dialect/Tensor/Utils/Utils.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ PadOp mlir::tensor::createPadHighOp(RankedTensorType resType, Value source,
2727
OpBuilder &b,
2828
SmallVector<Value> dynOutDims) {
2929

30+
// This assumption simplifies the following logic without limiting what's
31+
// required _today_. If needed, we can relax it in the future.
3032
assert(((resType.getNumDynamicDims() == dynOutDims.size()) ||
3133
dynOutDims.empty()) &&
3234
"Either none or all output dynamic dims must be specified!");

0 commit comments

Comments
 (0)