Skip to content

Commit 930b764

Browse files
titaiwangmsCopilot
andauthored
Hoist CropAndResize index offset to a named int64_t (microsoft#29623)
### Description Follow-up to the merged microsoft#29605, addressing a review nit from @apsonawane on the base-offset index arithmetic in `CropAndResizeForward`. The suggestion was that the `static_cast<int64_t>(...)` around the per-channel base offset looked redundant. It turns out the cast could **not** simply be removed: the offset expression is a `SafeInt<int64_t>`, and `pointer + SafeInt<int64_t>` is ambiguous — `SafeInt` exposes many implicit conversion operators (`char`, `short`, `int`, …), so the operand for pointer arithmetic cannot be resolved unambiguously (and `SafeInt`'s own `operator+(U, SafeInt)` tries to treat the pointer as an integer). The cast was therefore load-bearing. To honor the readability intent without the awkward inline cast, this change hoists the offset into a named `int64_t` local in both the bilinear and nearest branches: ```cpp const int64_t bottom_data_offset = (SafeInt<int64_t>(roi_batch_ind) * channels + c) * height * width; const T* offset_bottom_data = bottom_data + bottom_data_offset; ``` Initializing an `int64_t` directly from the `SafeInt<int64_t>` expression is unambiguous (it selects `operator int64_t()`), so this compiles cleanly, keeps the `SafeInt` overflow checking on the offset computation, and reads more clearly than the inline cast. **No behavior change.** ### Motivation and Context Improves readability of the index arithmetic introduced in microsoft#29605 while keeping the overflow-checked offset computation intact. ### Tests Covered by the existing `CropAndResizeTest` suite — all 10 tests pass: ``` onnxruntime_provider_test --gtest_filter='CropAndResizeTest.*' ``` Signed-off-by: Tita Wang <titaiwang@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 73260b2 commit 930b764

1 file changed

Lines changed: 4 additions & 6 deletions

File tree

onnxruntime/contrib_ops/cpu/crop_and_resize.cc

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,8 @@ void CropAndResizeForward(const TensorShape& output_shape,
149149
for (auto c = 0; c < channels; c++) {
150150
int64_t index_n_c = index_n + c * pooled_width * pooled_height;
151151
int64_t index = index_n_c + ph * pooled_width + pw;
152-
const T* offset_bottom_data =
153-
bottom_data +
154-
static_cast<int64_t>((SafeInt<int64_t>(roi_batch_ind) * channels + c) * height * width);
152+
const int64_t bottom_data_offset = (SafeInt<int64_t>(roi_batch_ind) * channels + c) * height * width;
153+
const T* offset_bottom_data = bottom_data + bottom_data_offset;
155154
const float top_left(static_cast<float>(offset_bottom_data[top_left_index]));
156155
const float top_right(static_cast<float>(offset_bottom_data[top_right_index]));
157156
const float bottom_left(static_cast<float>(offset_bottom_data[bottom_left_index]));
@@ -169,9 +168,8 @@ void CropAndResizeForward(const TensorShape& output_shape,
169168
for (auto c = 0; c < channels; c++) {
170169
int64_t index_n_c = index_n + c * pooled_width * pooled_height;
171170
int64_t index = index_n_c + ph * pooled_width + pw;
172-
const T* offset_bottom_data =
173-
bottom_data +
174-
static_cast<int64_t>((SafeInt<int64_t>(roi_batch_ind) * channels + c) * height * width);
171+
const int64_t bottom_data_offset = (SafeInt<int64_t>(roi_batch_ind) * channels + c) * height * width;
172+
const T* offset_bottom_data = bottom_data + bottom_data_offset;
175173
top_data[index] = static_cast<float>(offset_bottom_data[closest_index]);
176174
}
177175
}

0 commit comments

Comments
 (0)