Skip to content

Commit a1a43ff

Browse files
authored
Support negative padding in constant_pad_nd (pytorch#23116)
### Summary Fixes pytorch#13554. Match PyTorch's `constant_pad_nd` behavior for negative padding by cropping the input before applying positive padding. The optimized library uses the same portable implementation through its fallback registration, so both paths receive the fix. Handle mixed cropping and padding, unchanged output shapes, empty tensors, fully cropped inputs, dynamic outputs, and channels-last layouts. Reject excessive cropping and output-dimension overflow, and ignore unused fill values for crop-only operations as PyTorch does. Add regression coverage for these cases, including the input from pytorch#13554. This branch starts at upstream `main` (`283297e430`) and contains only this fix, without the earlier local commits. ### Test plan Validation used a fresh runtime build from this branch and a focused local CMake harness compiling the shared operator tests with generated portable and optimized bindings. | Check | Result | | --- | --- | | Portable `OpConstantPadNDOutTest` | 32/32 passed | | Optimized `OpConstantPadNDOutTest` | 31/32 passed; existing scalar-overflow failure described below | | PyTorch parity under UBSan, portable and optimized | 3,253 cases per build passed: 2,803 valid results and 450 expected rejections | | `clang-format --dry-run --Werror` on the three changed files | Passed | | `git diff --check` | Passed | Commands run against the focused build: ```sh cmake --build /private/tmp/executorch-pad-pr.r6f52N/tests --parallel 8 ctest --test-dir /private/tmp/executorch-pad-pr.r6f52N/tests --output-on-failure python .outputs/issue-13554/parity.py \ /private/tmp/executorch-pad-pr.r6f52N/tests/libportable_parity.dylib \ /private/tmp/executorch-pad-pr.r6f52N/tests/liboptimized_parity.dylib ``` The optimized `LongTensorTooLargeScalarDies` test fails with the locally installed PyTorch 2.11 development headers, whose floating-point-to-int64 overflow check accepts `2^63`. The same failure was reproduced using the unmodified operator from upstream `main`; the positive-padding scalar conversion is unchanged by this PR. Portable tests use the repository's current vendored headers and pass this check. Full repository CI and lintrunner were not run locally. Authored with assistance from OpenAI Codex.
1 parent abedcd2 commit a1a43ff

3 files changed

Lines changed: 259 additions & 60 deletions

File tree

‎kernels/portable/cpu/op_constant_pad_nd.cpp‎

Lines changed: 49 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
#include <c10/util/irange.h>
10-
#include <cmath>
10+
#include <algorithm>
1111
#include <cstring>
1212

1313
#include <executorch/runtime/kernel/kernel_includes.h>
@@ -32,12 +32,12 @@ template <typename CTYPE>
3232
void apply_padding_to_dim(
3333
KernelRuntimeContext& ctx,
3434
size_t ndim,
35+
executorch::aten::ArrayRef<executorch::aten::DimOrderType> dim_order,
3536
const CTYPE* self_data,
3637
IntArrayRef self_sizes,
3738
IntArrayRef self_strides,
3839
CTYPE* out_data,
3940
CTYPE* out_data_end,
40-
IntArrayRef out_sizes,
4141
IntArrayRef out_strides,
4242
IntArrayRef pad,
4343
const CTYPE value,
@@ -47,25 +47,18 @@ void apply_padding_to_dim(
4747
return;
4848
}
4949

50-
size_t pad_i = ndim - 1 - dim;
50+
const size_t logical_dim = dim_order[dim];
51+
size_t pad_i = ndim - 1 - logical_dim;
5152

5253
size_t pad_before = 0;
5354
size_t pad_after = 0;
5455
if (pad_i < pad.size() / 2) {
55-
int64_t pb = pad[2 * pad_i];
56-
int64_t pa = pad[2 * pad_i + 1];
57-
ET_KERNEL_CHECK_MSG(
58-
ctx,
59-
pb >= 0 && pa >= 0,
60-
InvalidArgument,
61-
/* void */,
62-
"Padding values must be non-negative.");
63-
pad_before = static_cast<size_t>(pb);
64-
pad_after = static_cast<size_t>(pa);
56+
pad_before = std::max<int64_t>(pad[2 * pad_i], 0);
57+
pad_after = std::max<int64_t>(pad[2 * pad_i + 1], 0);
6558
}
6659

67-
size_t out_step_len = out_strides[dim];
68-
size_t in_step_len = self_strides[dim];
60+
size_t out_step_len = out_strides[logical_dim];
61+
size_t in_step_len = self_strides[logical_dim];
6962

7063
// Do not copy padding beyond the out tensor bounds.
7164
// Use division to avoid potential overflow in multiplication.
@@ -92,19 +85,10 @@ void apply_padding_to_dim(
9285
// If subsequent dims are not padded, then the whole block of memory can be
9386
// copied.
9487
if (dim >= last_padded_dim) {
95-
size_t copy_len = in_step_len * self_sizes[dim];
88+
size_t copy_len = in_step_len * self_sizes[logical_dim];
9689
size_t copy_nbytes = copy_len * sizeof(CTYPE);
9790

9891
if (copy_nbytes > 0) {
99-
// Check that out_data and self_data do not overlap.
100-
ET_KERNEL_CHECK_MSG(
101-
ctx,
102-
out_data != self_data &&
103-
((out_data + copy_len <= self_data) ||
104-
(self_data + copy_len <= out_data)),
105-
InvalidArgument,
106-
/* void */,
107-
"Out tensor overlaps with the input tensor. This is not supported.");
10892
// Bounds check before memcpy
10993
ET_KERNEL_CHECK_MSG(
11094
ctx,
@@ -119,23 +103,31 @@ void apply_padding_to_dim(
119103
InvalidArgument,
120104
/* void */,
121105
"Out tensor is too small for the copy operation.");
106+
// Check that out_data and self_data do not overlap.
107+
ET_KERNEL_CHECK_MSG(
108+
ctx,
109+
out_data != self_data &&
110+
((out_data + copy_len <= self_data) ||
111+
(self_data + copy_len <= out_data)),
112+
InvalidArgument,
113+
/* void */,
114+
"Out tensor overlaps with the input tensor. This is not supported.");
122115
memcpy(out_data, self_data, copy_nbytes);
123116
out_data += copy_len;
124-
self_data += copy_len;
125117
}
126118
}
127119
// Otherwise, call this function recursively
128120
else {
129-
for (ET_UNUSED const auto i : c10::irange(self_sizes[dim])) {
121+
for (const auto i : c10::irange(self_sizes[logical_dim])) {
130122
apply_padding_to_dim(
131123
ctx,
132124
ndim,
125+
dim_order,
133126
self_data,
134127
self_sizes,
135128
self_strides,
136129
out_data,
137130
out_data_end,
138-
out_sizes,
139131
out_strides,
140132
pad,
141133
value,
@@ -147,7 +139,9 @@ void apply_padding_to_dim(
147139
}
148140

149141
out_data += out_step_len;
150-
self_data += in_step_len;
142+
if (i + 1 < self_sizes[logical_dim]) {
143+
self_data += in_step_len;
144+
}
151145
}
152146
}
153147

@@ -181,6 +175,10 @@ void constant_pad_nd_out_impl(
181175
IntArrayRef pad,
182176
CTYPE value_v,
183177
Tensor& out) {
178+
if (out.numel() == 0) {
179+
return;
180+
}
181+
184182
const CTYPE* self_data = self.const_data_ptr<CTYPE>();
185183
CTYPE* out_data = out.mutable_data_ptr<CTYPE>();
186184

@@ -193,42 +191,49 @@ void constant_pad_nd_out_impl(
193191

194192
int64_t self_sizes[kTensorDimensionLimit];
195193
int64_t self_strides[kTensorDimensionLimit];
196-
int64_t out_sizes[kTensorDimensionLimit];
197194
int64_t out_strides[kTensorDimensionLimit];
198195

199196
// Collect sizes and strides of input and output tensors and determine the
200197
// last padded dimension
201198
size_t last_padded_dim = 0;
199+
size_t input_offset = 0;
202200
for (const auto i : c10::irange(ndim)) {
203-
self_sizes[i] = self.size(i);
204-
self_strides[i] = getTrailingDims(self, static_cast<int64_t>(i));
205-
out_sizes[i] = out.size(i);
206-
out_strides[i] = getTrailingDims(out, static_cast<int64_t>(i));
201+
const size_t dim = self.dim_order()[i];
202+
self_sizes[dim] = self.size(dim);
203+
self_strides[dim] = self.strides()[dim];
204+
out_strides[dim] = out.strides()[dim];
207205

208-
size_t pad_i = ndim - 1 - i;
206+
size_t pad_i = ndim - 1 - dim;
209207
if (pad_i < pad.size() / 2) {
210-
if (pad[2 * pad_i] + pad[2 * pad_i + 1] > 0) {
208+
const int64_t crop_before = -std::min<int64_t>(pad[2 * pad_i], 0);
209+
const int64_t crop_after = -std::min<int64_t>(pad[2 * pad_i + 1], 0);
210+
self_sizes[dim] -= crop_before + crop_after;
211+
input_offset += crop_before * self_strides[dim];
212+
if (pad[2 * pad_i] != 0 || pad[2 * pad_i + 1] != 0) {
211213
last_padded_dim = i;
212214
}
213215
}
216+
if (self_sizes[dim] == 0) {
217+
set_all_to_value(out_data, out.numel(), value_v);
218+
return;
219+
}
214220
}
215221

216222
IntArrayRef self_sizes_ref(self_sizes, ndim);
217223
IntArrayRef self_strides_ref(self_strides, ndim);
218-
IntArrayRef out_sizes_ref(out_sizes, ndim);
219224
IntArrayRef out_strides_ref(out_strides, ndim);
220225

221226
CTYPE* out_data_end = out_data + out.numel();
222227

223228
apply_padding_to_dim(
224229
ctx,
225230
ndim,
226-
self_data,
231+
self.dim_order(),
232+
self_data + input_offset,
227233
self_sizes_ref,
228234
self_strides_ref,
229235
out_data,
230236
out_data_end,
231-
out_sizes_ref,
232237
out_strides_ref,
233238
pad,
234239
value_v,
@@ -244,16 +249,12 @@ Tensor& constant_pad_nd_out(
244249
IntArrayRef pad,
245250
const Scalar& value,
246251
Tensor& out) {
247-
(void)ctx;
248-
249252
ET_KERNEL_CHECK(
250253
ctx, check_constant_pad_args(in, pad, value, out), InvalidArgument, out);
251254

252255
ET_KERNEL_CHECK(
253256
ctx, tensors_have_same_dim_order(in, out), InvalidArgument, out);
254257

255-
ET_KERNEL_CHECK(ctx, tensor_is_default_dim_order(in), InvalidArgument, out);
256-
257258
// resize out tensor for dynamic shapes
258259
ET_KERNEL_CHECK_MSG(
259260
ctx,
@@ -267,9 +268,12 @@ Tensor& constant_pad_nd_out(
267268
// @lint-ignore CLANGTIDY facebook-hte-CArray
268269
static constexpr const char op_name[] = "constant_pad_nd.out";
269270

271+
const bool has_positive_padding =
272+
std::any_of(pad.begin(), pad.end(), [](int64_t p) { return p > 0; });
270273
ET_SWITCH_REALHBBF16_TYPES(in_type, ctx, op_name, CTYPE, [&]() {
271-
auto opt_value_casted =
272-
utils::internal::check_overflow_scalar_cast<CTYPE>(value);
274+
// PyTorch ignores the fill value when the operation only crops or copies.
275+
auto opt_value_casted = utils::internal::check_overflow_scalar_cast<CTYPE>(
276+
has_positive_padding ? value : Scalar(0));
273277
ET_KERNEL_CHECK(ctx, opt_value_casted.has_value(), InvalidArgument, );
274278
auto value_casted = opt_value_casted.value();
275279
constant_pad_nd_out_impl<CTYPE>(ctx, in, pad, value_casted, out);

‎kernels/portable/cpu/util/kernel_ops_util.cpp‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
*/
88

99
#include <c10/util/irange.h>
10+
#include <algorithm>
1011
#include <cstring>
12+
#include <limits>
1113

1214
#include <executorch/kernels/portable/cpu/util/kernel_ops_util.h>
1315
#include <executorch/runtime/core/exec_aten/util/tensor_util.h>
@@ -616,12 +618,21 @@ bool check_constant_pad_args(
616618
pad.size() / 2,
617619
in.dim());
618620

619-
for (size_t i = 0; i < pad.size(); ++i) {
620-
ET_CHECK_OR_RETURN_FALSE(
621-
pad[i] >= 0,
622-
"Padding values must be non-negative, but got pad[%zu] = %" PRId64,
623-
i,
624-
pad[i]);
621+
for (const auto i : c10::irange(pad.size() / 2)) {
622+
int64_t size = in.size(in.dim() - 1 - i);
623+
for (const auto j : c10::irange(2)) {
624+
const int64_t crop = std::min<int64_t>(pad[2 * i + j], 0);
625+
ET_CHECK_OR_RETURN_FALSE(
626+
crop >= -size, "Negative padding exceeds the input dimension.");
627+
size += crop;
628+
}
629+
for (const auto j : c10::irange(2)) {
630+
const int64_t padding = std::max<int64_t>(pad[2 * i + j], 0);
631+
ET_CHECK_OR_RETURN_FALSE(
632+
padding <= std::numeric_limits<Tensor::SizesType>::max() - size,
633+
"Padded dimension exceeds the tensor size limit.");
634+
size += padding;
635+
}
625636
}
626637

627638
return true;

0 commit comments

Comments
 (0)