Skip to content

Commit 552e679

Browse files
authored
Support negative indices and omitted unit dimensions in index_put_ (#23529)
### Summary Transformers Mistral sliding-window decode fails after reinplace rewrites its cache updates to `aten::index_put_`. The portable kernel rejects the valid index `-1` and requires values `[1, 2, 1, 8]` where the exported graph supplies `[2, 1, 8]`. Accept indices in `[-size, size)` and normalize negative indices in both overwrite and accumulation paths. Allow omitted leading unit dimensions in the values tensor, using the index length to drive the existing copy loops. This requires no additional allocation or data expansion. Depends on #23506 (`JakeStevens:fix/accumulating-index-put-inplace`). Review that first; the follow-up changes are in commit `95badb36fe`. ### Test plan Compiled and ran the complete `op_index_put_test.cpp` against the portable kernel with a local CMake harness (`./build/fixed_index_put_test`): **57 passed, 2 existing dynamic-shape skips**. Added 33 regression cases covering int32/int64 indices, overwrite and accumulation, repeated index aliases, bounds, omitted unit dimensions, scalar updates, and incompatible shapes. The corresponding repository tests can be run from a CMake build configured with `EXECUTORCH_BUILD_TESTS=ON`: ```bash cmake --build cmake-out --target portable_kernels_test -j cmake-out/kernels/test/portable_kernels_test --gtest_filter='*OpIndexPut*' ``` Relinked the local runtime with the changed kernel and ran tiny FP32 Mistral static sliding-window single-token decode exports generated with the planner fix from #23526. All **11 runtime outputs match default bit-for-bit**, including caches and repeated calls after restoring inputs. Both paths match eager PyTorch with maximum absolute error **5.96e-8**. `lintrunner kernels/portable/cpu/op_index_put.cpp kernels/test/op_index_put_test.cpp` and the pre-commit checks pass. Authored with assistance from OpenAI Codex.
1 parent 6f665e1 commit 552e679

4 files changed

Lines changed: 214 additions & 18 deletions

File tree

‎kernels/portable/cpu/op_index_put.cpp‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ bool check_special_case_in_place_args(
200200
Long, Int, index.scalar_type(), ctx, "index_put_", CTYPE, [&]() {
201201
const CTYPE* const index_arr = index.const_data_ptr<CTYPE>();
202202
for (const auto i : c10::irange(index.numel())) {
203-
if (index_arr[i] < 0 ||
203+
if (index_arr[i] < -static_cast<CTYPE>(in.size(*dim)) ||
204204
index_arr[i] >= static_cast<CTYPE>(in.size(*dim))) {
205205
ET_LOG(
206206
Error,
@@ -220,14 +220,6 @@ bool check_special_case_in_place_args(
220220
is_valid_index,
221221
"Some index values are not within bounds of input tensor at indexed dim");
222222

223-
ET_CHECK_OR_RETURN_FALSE(
224-
values.size(*dim) == index.size(0),
225-
"Special case in-place index_put requires values to match index length at the indexed dim; values.size(%zu) = %" ET_PRI_TENSOR_SIZE
226-
", index_length = %zd",
227-
*dim,
228-
values.size(*dim),
229-
index.size(0));
230-
231223
Tensor::SizesType expected_values_size[kTensorDimensionLimit] = {};
232224
size_t in_ndim = static_cast<size_t>(in.dim());
233225
for (const auto i : c10::irange(in_ndim)) {
@@ -237,6 +229,15 @@ bool check_special_case_in_place_args(
237229
}
238230
expected_values_size[*dim] = static_cast<Tensor::SizesType>(index.size(0));
239231

232+
size_t leading_unit_dims = 0;
233+
while (in_ndim - leading_unit_dims > static_cast<size_t>(values.dim()) &&
234+
expected_values_size[leading_unit_dims] == 1) {
235+
++leading_unit_dims;
236+
}
237+
const bool values_shape_matches = tensor_has_expected_size(
238+
values,
239+
{expected_values_size + leading_unit_dims, in_ndim - leading_unit_dims});
240+
240241
#if ET_LOG_ENABLED
241242
auto in_shape_str = executorch::runtime::tensor_shape_to_c_string(
242243
executorch::runtime::Span<const Tensor::SizesType>(
@@ -246,14 +247,14 @@ bool check_special_case_in_place_args(
246247
values.sizes().data(), values.sizes().size()));
247248

248249
ET_CHECK_OR_RETURN_FALSE(
249-
tensor_has_expected_size(values, {expected_values_size, in_ndim}),
250-
"Special case in-place index_put requires values to match input shape except for indexed dim; got input shape %s and values shape %s",
250+
values_shape_matches,
251+
"Special case in-place index_put requires values to match input shape except for indexed dim, allowing omitted leading unit dimensions; got input shape %s and values shape %s",
251252
in_shape_str.data(),
252253
values_shape_str.data());
253254
#else
254255
ET_CHECK_OR_RETURN_FALSE(
255-
tensor_has_expected_size(values, {expected_values_size, in_ndim}),
256-
"Special case in-place index_put requires values to match input shape except for indexed dim");
256+
values_shape_matches,
257+
"Special case in-place index_put requires values to match input shape except for indexed dim, allowing omitted leading unit dimensions");
257258
#endif // ET_LOG_ENABLED
258259

259260
return true;
@@ -299,7 +300,7 @@ Tensor& index_put_(
299300
return in;
300301
}
301302

302-
size_t values_dim_length = values.size(dim);
303+
size_t values_dim_length = index.size(0);
303304
size_t in_dim_length = in.size(dim);
304305

305306
size_t length_per_step = trailing_dims * in.element_size();
@@ -316,10 +317,13 @@ Tensor& index_put_(
316317
VALUE_T* dest = in.mutable_data_ptr<VALUE_T>();
317318
for (const auto i : c10::irange(leading_dims)) {
318319
for (const auto j : c10::irange(values_dim_length)) {
320+
const auto ix = index_arr[j] < 0
321+
? index_arr[j] + static_cast<CTYPE>(in_dim_length)
322+
: index_arr[j];
319323
const size_t src_offset =
320324
(i * values_dim_length + j) * trailing_dims;
321325
const size_t dest_offset =
322-
(i * in_dim_length + index_arr[j]) * trailing_dims;
326+
(i * in_dim_length + ix) * trailing_dims;
323327
for (const auto k : c10::irange(trailing_dims)) {
324328
dest[dest_offset + k] += src[src_offset + k];
325329
}
@@ -331,8 +335,11 @@ Tensor& index_put_(
331335
const char* src = values_data + i * values_dim_length * length_per_step;
332336
char* dest = in_data + i * in_dim_length * length_per_step;
333337
for (const auto j : c10::irange(values_dim_length)) {
338+
const auto ix = index_arr[j] < 0
339+
? index_arr[j] + static_cast<CTYPE>(in_dim_length)
340+
: index_arr[j];
334341
const char* copy_src = src + j * length_per_step;
335-
char* copy_dest = dest + index_arr[j] * length_per_step;
342+
char* copy_dest = dest + ix * length_per_step;
336343
memcpy(copy_dest, copy_src, length_per_step);
337344
}
338345
}

‎kernels/test/op_index_put_test.cpp‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
#include <gtest/gtest.h>
1818
#include <sys/types.h>
19+
#include <tuple>
20+
#include <vector>
1921

2022
using namespace ::testing;
2123
using executorch::aten::ArrayRef;
@@ -1138,3 +1140,179 @@ TEST_F(OpIndexPutInplaceTest, AccumulateHalfRepeatedIndices) {
11381140
op_index_put_(x, indices, values, /*accumulate=*/true);
11391141
EXPECT_TENSOR_EQ(x, tf.make({3}, {1, 6, 1}));
11401142
}
1143+
1144+
class OpIndexPutInplaceNegativeIndexTest
1145+
: public OpIndexPutInplaceTest,
1146+
public WithParamInterface<std::tuple<ScalarType, bool>> {};
1147+
1148+
TEST_P(OpIndexPutInplaceNegativeIndexTest, MixedIndicesInMiddleDimension) {
1149+
const auto [index_type, accumulate] = GetParam();
1150+
TensorFactory<ScalarType::Float> tf;
1151+
TensorFactory<ScalarType::Long> tfl;
1152+
TensorFactory<ScalarType::Int> tfi;
1153+
Tensor index = index_type == ScalarType::Long ? tfl.make({3}, {-1, -3, 1})
1154+
: tfi.make({3}, {-1, -3, 1});
1155+
optional<Tensor> indices[] = {std::nullopt, index};
1156+
Tensor x = tf.ones({2, 3, 2});
1157+
Tensor values = tf.make({2, 3, 2}, {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12});
1158+
Tensor expected = accumulate
1159+
? tf.make({2, 3, 2}, {4, 5, 6, 7, 2, 3, 10, 11, 12, 13, 8, 9})
1160+
: tf.make({2, 3, 2}, {3, 4, 5, 6, 1, 2, 9, 10, 11, 12, 7, 8});
1161+
1162+
Tensor ret = op_index_put_(x, indices, values, accumulate);
1163+
EXPECT_TENSOR_EQ(ret, x);
1164+
EXPECT_TENSOR_EQ(x, expected);
1165+
EXPECT_TENSOR_EQ(
1166+
index,
1167+
index_type == ScalarType::Long ? tfl.make({3}, {-1, -3, 1})
1168+
: tfi.make({3}, {-1, -3, 1}));
1169+
}
1170+
1171+
TEST_P(OpIndexPutInplaceNegativeIndexTest, NegativeIndexOutOfBounds) {
1172+
const auto [index_type, accumulate] = GetParam();
1173+
TensorFactory<ScalarType::Float> tf;
1174+
TensorFactory<ScalarType::Long> tfl;
1175+
TensorFactory<ScalarType::Int> tfi;
1176+
Tensor index = index_type == ScalarType::Long ? tfl.make({2}, {-1, -4})
1177+
: tfi.make({2}, {-1, -4});
1178+
optional<Tensor> indices[] = {std::nullopt, index};
1179+
Tensor x = tf.ones({2, 3, 2});
1180+
Tensor values = tf.zeros({2, 2, 2});
1181+
1182+
ET_EXPECT_KERNEL_FAILURE(
1183+
context_, op_index_put_(x, indices, values, accumulate));
1184+
#ifndef USE_ATEN_LIB
1185+
EXPECT_TENSOR_EQ(x, tf.ones({2, 3, 2}));
1186+
#endif
1187+
}
1188+
1189+
TEST_P(OpIndexPutInplaceNegativeIndexTest, PositiveIndexOutOfBounds) {
1190+
const auto [index_type, accumulate] = GetParam();
1191+
TensorFactory<ScalarType::Float> tf;
1192+
TensorFactory<ScalarType::Long> tfl;
1193+
TensorFactory<ScalarType::Int> tfi;
1194+
Tensor index = index_type == ScalarType::Long ? tfl.make({2}, {-1, 3})
1195+
: tfi.make({2}, {-1, 3});
1196+
optional<Tensor> indices[] = {std::nullopt, index};
1197+
Tensor x = tf.ones({2, 3, 2});
1198+
Tensor values = tf.zeros({2, 2, 2});
1199+
1200+
ET_EXPECT_KERNEL_FAILURE(
1201+
context_, op_index_put_(x, indices, values, accumulate));
1202+
#ifndef USE_ATEN_LIB
1203+
EXPECT_TENSOR_EQ(x, tf.ones({2, 3, 2}));
1204+
#endif
1205+
}
1206+
1207+
TEST_P(OpIndexPutInplaceNegativeIndexTest, NegativeIndexIntoEmptyDimension) {
1208+
const auto [index_type, accumulate] = GetParam();
1209+
TensorFactory<ScalarType::Float> tf;
1210+
TensorFactory<ScalarType::Long> tfl;
1211+
TensorFactory<ScalarType::Int> tfi;
1212+
Tensor index = index_type == ScalarType::Long ? tfl.make({1}, {-1})
1213+
: tfi.make({1}, {-1});
1214+
Tensor x = tf.zeros({0});
1215+
Tensor values = tf.ones({1});
1216+
1217+
ET_EXPECT_KERNEL_FAILURE(
1218+
context_, op_index_put_(x, {index}, values, accumulate));
1219+
}
1220+
1221+
TEST_P(OpIndexPutInplaceNegativeIndexTest, OmittedLeadingUnitDimensions) {
1222+
const auto [index_type, accumulate] = GetParam();
1223+
TensorFactory<ScalarType::Float> tf;
1224+
TensorFactory<ScalarType::Long> tfl;
1225+
TensorFactory<ScalarType::Int> tfi;
1226+
Tensor index = index_type == ScalarType::Long ? tfl.make({1}, {-1})
1227+
: tfi.make({1}, {-1});
1228+
Tensor values = tf.make({2, 1, 2}, {2, 3, 4, 5});
1229+
1230+
for (const std::vector<int32_t>& shape :
1231+
{std::vector<int32_t>{1, 2, 4, 2}, {1, 1, 2, 4, 2}}) {
1232+
Tensor x = tf.ones(shape);
1233+
std::vector<optional<Tensor>> indices(shape.size() - 1, std::nullopt);
1234+
indices.back() = index;
1235+
Tensor expected = accumulate
1236+
? tf.make(shape, {1, 1, 1, 1, 1, 1, 3, 4, 1, 1, 1, 1, 1, 1, 5, 6})
1237+
: tf.make(shape, {1, 1, 1, 1, 1, 1, 2, 3, 1, 1, 1, 1, 1, 1, 4, 5});
1238+
1239+
Tensor ret =
1240+
op_index_put_(x, {indices.data(), indices.size()}, values, accumulate);
1241+
EXPECT_TENSOR_EQ(ret, x);
1242+
EXPECT_TENSOR_EQ(x, expected);
1243+
}
1244+
EXPECT_TENSOR_EQ(values, tf.make({2, 1, 2}, {2, 3, 4, 5}));
1245+
}
1246+
1247+
TEST_P(OpIndexPutInplaceNegativeIndexTest, OmittedIndexedLeadingDimension) {
1248+
const auto [index_type, accumulate] = GetParam();
1249+
TensorFactory<ScalarType::Float> tf;
1250+
TensorFactory<ScalarType::Long> tfl;
1251+
TensorFactory<ScalarType::Int> tfi;
1252+
Tensor index = index_type == ScalarType::Long ? tfl.make({1}, {-1})
1253+
: tfi.make({1}, {-1});
1254+
Tensor x = tf.ones({3, 1, 2});
1255+
Tensor values = tf.make({2}, {2, 3});
1256+
Tensor expected = accumulate ? tf.make({3, 1, 2}, {1, 1, 1, 1, 3, 4})
1257+
: tf.make({3, 1, 2}, {1, 1, 1, 1, 2, 3});
1258+
1259+
op_index_put_(x, {index}, values, accumulate);
1260+
EXPECT_TENSOR_EQ(x, expected);
1261+
}
1262+
1263+
TEST_P(OpIndexPutInplaceNegativeIndexTest, ScalarValuesForSingleElement) {
1264+
const auto [index_type, accumulate] = GetParam();
1265+
TensorFactory<ScalarType::Float> tf;
1266+
TensorFactory<ScalarType::Long> tfl;
1267+
TensorFactory<ScalarType::Int> tfi;
1268+
Tensor index = index_type == ScalarType::Long ? tfl.make({1}, {-1})
1269+
: tfi.make({1}, {-1});
1270+
optional<Tensor> indices[] = {std::nullopt, index};
1271+
Tensor x = tf.ones({1, 3, 1});
1272+
Tensor values = tf.make({}, {5});
1273+
Tensor expected = accumulate ? tf.make({1, 3, 1}, {1, 1, 6})
1274+
: tf.make({1, 3, 1}, {1, 1, 5});
1275+
1276+
op_index_put_(x, indices, values, accumulate);
1277+
EXPECT_TENSOR_EQ(x, expected);
1278+
}
1279+
1280+
TEST_P(OpIndexPutInplaceNegativeIndexTest, IncompatibleValuesShape) {
1281+
const auto [index_type, accumulate] = GetParam();
1282+
TensorFactory<ScalarType::Float> tf;
1283+
TensorFactory<ScalarType::Long> tfl;
1284+
TensorFactory<ScalarType::Int> tfi;
1285+
Tensor index = index_type == ScalarType::Long ? tfl.make({1}, {-1})
1286+
: tfi.make({1}, {-1});
1287+
optional<Tensor> indices[] = {std::nullopt, std::nullopt, index};
1288+
Tensor x = tf.ones({1, 2, 4, 2});
1289+
Tensor values = tf.zeros({2, 2, 1});
1290+
1291+
ET_EXPECT_KERNEL_FAILURE(
1292+
context_, op_index_put_(x, indices, values, accumulate));
1293+
#ifndef USE_ATEN_LIB
1294+
EXPECT_TENSOR_EQ(x, tf.ones({1, 2, 4, 2}));
1295+
#endif
1296+
}
1297+
1298+
INSTANTIATE_TEST_SUITE_P(
1299+
IndexTypesAndAccumulation,
1300+
OpIndexPutInplaceNegativeIndexTest,
1301+
Combine(Values(ScalarType::Long, ScalarType::Int), Bool()));
1302+
1303+
TEST_F(OpIndexPutInplaceTest, AccumulateNegativeAndPositiveAliases) {
1304+
TensorFactory<ScalarType::Float> tf;
1305+
TensorFactory<ScalarType::Long> tfl;
1306+
TensorFactory<ScalarType::Int> tfi;
1307+
Tensor values = tf.make({2, 3, 2}, {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12});
1308+
Tensor expected =
1309+
tf.make({2, 3, 2}, {6, 7, 1, 1, 5, 7, 12, 13, 1, 1, 17, 19});
1310+
1311+
for (const Tensor& index :
1312+
{tfl.make({3}, {-1, 2, -3}), tfi.make({3}, {-1, 2, -3})}) {
1313+
optional<Tensor> indices[] = {std::nullopt, index};
1314+
Tensor x = tf.ones({2, 3, 2});
1315+
op_index_put_(x, indices, values, /*accumulate=*/true);
1316+
EXPECT_TENSOR_EQ(x, expected);
1317+
}
1318+
}

‎runtime/core/exec_aten/util/tensor_shape_to_c_string.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,11 @@ std::array<char, kTensorShapeStringSizeLimit> tensor_shape_to_c_string_impl(
4848
static_cast<uint32_t>(elem));
4949
}
5050
}
51-
*(p - 2) = ')';
52-
*(p - 1) = '\0';
51+
if (!shape.empty()) {
52+
p -= 2;
53+
}
54+
*p++ = ')';
55+
*p = '\0';
5356
return out;
5457
}
5558
} // namespace

‎runtime/core/exec_aten/util/test/tensor_shape_to_c_string_test.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,14 @@ using executorch::runtime::Span;
1717
using executorch::runtime::tensor_shape_to_c_string;
1818
using executorch::runtime::internal::kMaximumPrintableTensorShapeElement;
1919

20+
TEST(TensorShapeToCStringTest, EmptyShape) {
21+
auto str32 = tensor_shape_to_c_string(Span<const std::int32_t>());
22+
EXPECT_STREQ(str32.data(), "()");
23+
24+
auto str64 = tensor_shape_to_c_string(Span<const std::int64_t>());
25+
EXPECT_STREQ(str64.data(), "()");
26+
}
27+
2028
TEST(TensorShapeToCStringTest, Basic) {
2129
std::array<executorch::aten::SizesType, 3> sizes = {123, 456, 789};
2230
auto str = tensor_shape_to_c_string(

0 commit comments

Comments
 (0)