Skip to content

Commit 18f88f7

Browse files
committed
[OnnxToTorch] Fix AvgPool ceil dynamic result
The static ceil-split branch (count_include_pad, static input) read the result spatial extent without checking it was static. A static-input / dynamic-result AveragePool with ceil_mode therefore hit the kUnknownSize sentinel, computed a negative "extra", skipped the split, and emitted an ONNX-wrong floor-mode pool. Fold result-dim dynamism into anyDynamicSpatial so that case routes through the runtime constant_pad_nd path instead. Also extract the static split formula into a single staticSplitPad helper shared by both static call sites, which previously duplicated it (the divergence above originated from that duplication).
1 parent b759eea commit 18f88f7

2 files changed

Lines changed: 52 additions & 19 deletions

File tree

lib/Conversion/TorchOnnxToTorch/DefaultDomainAtoF.cpp

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -554,13 +554,18 @@ void mlir::torch::onnx_c::populateDefaultDomainAtoF(
554554
ArrayRef<int64_t> spatialShape =
555555
inputTensorType.getSizes().drop_front(2);
556556
auto isDynamicDim = [](int64_t d) { return d == Torch::kUnknownSize; };
557-
bool anyDynamicSpatial = llvm::any_of(spatialShape, isDynamicDim);
558557
// In ceil mode ONNX drops any trailing window that would begin entirely
559558
// inside the padding. The result type reflects that drop (shape
560559
// inference applied it), so use it as the output extent -- the injected
561560
// padding below is sized to reproduce it.
562561
ArrayRef<int64_t> resultSpatialShape =
563562
resultType.getSizes().drop_front(2);
563+
// The static "bake the split into the pad attr" branch below needs both
564+
// the input and the result extent of every spatial dim; if either is
565+
// unknown, take the runtime `constant_pad_nd` path (buildSplitPadList
566+
// handles the mixed case per dim).
567+
bool anyDynamicSpatial = llvm::any_of(spatialShape, isDynamicDim) ||
568+
llvm::any_of(resultSpatialShape, isDynamicDim);
564569

565570
// Expand a collapsed one-value-per-dim padding (used when the input
566571
// pads are symmetric) into the explicit per-dim begin/end form the ceil
@@ -573,6 +578,23 @@ void mlir::torch::onnx_c::populateDefaultDomainAtoF(
573578
pads[i + spatialRank] = pads[i];
574579
};
575580

581+
// Resolve the ONNX ceil split for one static spatial dim: the extra
582+
// ceil extent (actualPadded - inDim - pBegin - pEnd) is halved onto the
583+
// leading edge and the remainder onto the trailing edge. Updates pBegin
584+
// / pEnd in place. Kept as the single source of the static split so the
585+
// two static call sites below cannot drift (an earlier divergence
586+
// between them was a bug). Requires inDim and outDim to be static.
587+
auto staticSplitPad = [](int64_t inDim, int64_t outDim, int64_t stride,
588+
int64_t dilatedKernel, int64_t &pBegin,
589+
int64_t &pEnd) {
590+
int64_t actualPadded = (outDim - 1) * stride + dilatedKernel;
591+
int64_t extra = actualPadded - inDim - pBegin - pEnd;
592+
if (extra > 0) {
593+
pBegin += extra / 2;
594+
pEnd += extra - extra / 2;
595+
}
596+
};
597+
576598
// Build the aten.constant_pad_nd pad list for the ONNX ceil split
577599
// (begin = p + extra/2, end = p + extra - extra/2), emitted as runtime
578600
// arithmetic for dynamic dims and constants otherwise. Order is
@@ -590,14 +612,8 @@ void mlir::torch::onnx_c::populateDefaultDomainAtoF(
590612
int64_t pEndI = normPad[i + spatialRank];
591613
if (!isDynamicDim(spatialShape[i]) &&
592614
!isDynamicDim(resultSpatialShape[i])) {
593-
int64_t inDim = spatialShape[i];
594-
int64_t outDim = resultSpatialShape[i];
595-
int64_t actualPadded = (outDim - 1) * stride + dilatedKernel;
596-
int64_t extra = actualPadded - inDim - pBeginI - pEndI;
597-
if (extra > 0) {
598-
pBeginI += extra / 2;
599-
pEndI += extra - extra / 2;
600-
}
615+
staticSplitPad(spatialShape[i], resultSpatialShape[i], stride,
616+
dilatedKernel, pBeginI, pEndI);
601617
padPairsFront.push_back(Torch::ConstantIntOp::create(
602618
rewriter, loc, rewriter.getI64IntegerAttr(pBeginI)));
603619
padPairsFront.push_back(Torch::ConstantIntOp::create(
@@ -706,17 +722,9 @@ void mlir::torch::onnx_c::populateDefaultDomainAtoF(
706722
// attr, then emit floor mode.
707723
toBeginEndPadding(padding);
708724
for (int i = 0; i < spatialRank; ++i) {
709-
int64_t inDim = spatialShape[i];
710725
int64_t dilatedKernel = dilations[i] * (kernel[i] - 1) + 1;
711-
int64_t &pBegin = padding[i];
712-
int64_t &pEnd = padding[i + spatialRank];
713-
int64_t outDim = resultSpatialShape[i];
714-
int64_t actualPadded = (outDim - 1) * strides[i] + dilatedKernel;
715-
int64_t extra = actualPadded - inDim - pBegin - pEnd;
716-
if (extra > 0) {
717-
pBegin += extra / 2;
718-
pEnd += extra - extra / 2;
719-
}
726+
staticSplitPad(spatialShape[i], resultSpatialShape[i], strides[i],
727+
dilatedKernel, padding[i], padding[i + spatialRank]);
720728
}
721729
ceilMode = false;
722730
} else if (ceilMode && !countIncludePad) {

test/Conversion/TorchOnnxToTorch/simple_ops_a_to_f.mlir

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,6 +1002,31 @@ func.func @test_averagepool_2d_ceil_count_include_pad(%arg0: !torch.vtensor<[1,2
10021002

10031003
// -----
10041004

1005+
// Static input but a dynamic result extent: the static "bake the split into the
1006+
// pad attr" path cannot size the split (it would read an unknown result dim), so
1007+
// this must fall through to the runtime `constant_pad_nd` + zero-padded,
1008+
// floor-mode pool path -- the same one used when the input is dynamic. The ONNX
1009+
// split pads are still derived at runtime (size.int / ge / Int.bool / sub), and
1010+
// the pool itself carries zero padding, so the original [1,1] pads must NOT
1011+
// appear as the pool's padding arg on %arg0.
1012+
// The input extent is a compile-time constant here (static input), so the split
1013+
// is derived from constant ints -- but still through the runtime drop logic
1014+
// (ge -> Int.bool -> sub), not the static bake-into-attr path.
1015+
// CHECK-LABEL: @test_averagepool_2d_ceil_count_include_pad_static_in_dynamic_out
1016+
// CHECK: %[[DROP:.*]] = torch.aten.ge.int %{{.*}}, %{{.*}} : !torch.int, !torch.int -> !torch.bool
1017+
// CHECK: %[[DROPI:.*]] = torch.aten.Int.bool %[[DROP]]
1018+
// CHECK: %[[PADDED:.*]] = torch.aten.constant_pad_nd %arg0
1019+
// CHECK: %[[ZEROPAD:.*]] = torch.prim.ListConstruct %int0{{.*}}, %int0{{.*}} : (!torch.int, !torch.int) -> !torch.list<int>
1020+
// CHECK: %[[CEIL:.*]] = torch.constant.bool false
1021+
// CHECK: %[[CIP:.*]] = torch.constant.bool true
1022+
// CHECK: torch.aten.avg_pool2d %[[PADDED]], %{{.*}}, %{{.*}}, %[[ZEROPAD]], %[[CEIL]], %[[CIP]], %none
1023+
func.func @test_averagepool_2d_ceil_count_include_pad_static_in_dynamic_out(%arg0: !torch.vtensor<[1,2,10,11],f32>) -> !torch.vtensor<[1,2,?,?],f32> attributes {torch.onnx_meta.ir_version = 9 : si64, torch.onnx_meta.opset_version = 19 : si64} {
1024+
%0 = torch.operator "onnx.AveragePool"(%arg0) {torch.onnx.ceil_mode = 1 : si64, torch.onnx.count_include_pad = 1 : si64, torch.onnx.kernel_shape = [3 : si64, 3 : si64], torch.onnx.pads = [1 : si64, 1 : si64, 1 : si64, 1 : si64], torch.onnx.strides = [2 : si64, 2 : si64]} : (!torch.vtensor<[1,2,10,11],f32>) -> !torch.vtensor<[1,2,?,?],f32>
1025+
return %0 : !torch.vtensor<[1,2,?,?],f32>
1026+
}
1027+
1028+
// -----
1029+
10051030
// Same as above but with a dynamic spatial dim: the ONNX ceil split depends on
10061031
// the runtime extent, so it is materialized as a runtime `constant_pad_nd` in
10071032
// front of a zero-padded, floor-mode pool (count_include_pad keeps the injected

0 commit comments

Comments
 (0)