Fix transpose convolution scratch width for asymmetric strides - #243
Merged
AdrianLundell merged 1 commit intoSep 11, 2026
Merged
Conversation
Use the width stride in both rolling-buffer size calculations, matching arm_transpose_conv_s8. Using the height stride underallocates scratch when the width stride exceeds both the height stride and filter width, and can overallocate when the height stride is larger. Cover asymmetric strides and a larger-filter control through the scalar, DSP, and MVE sizing bindings. Authored with Codex.
Collaborator
|
LGTM, thanks for the fix! |
rascani
added a commit
to pytorch/executorch
that referenced
this pull request
Sep 11, 2026
### Summary Reserve the larger of the CMSIS-NN reported size and the rolling-buffer kernel requirement during export, leaving reverse-convolution allocations unchanged. Allow the larger allocation in the optional runtime check while awaiting ARM-software/CMSIS-NN#243. ### Test plan Enable runtime checks in the test runner and cover strided pointwise transpose convolution in explicit layout, with and without Hardtanh. The focused M55 suite passed with 51 passes and 9 expected failures; 30 separate allocation checks also passed. Authored with Codex.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The rolling-buffer width calculation in both transpose-convolution buffer-size functions uses
stride.hinMAX(filter_dims->w, ...). The execution kernel,arm_transpose_conv_s8, usesstride.w. This can underallocate scratch when the width stride exceeds both the height stride and filter width, and can overallocate when the height stride is larger.For input NHWC
[2, 1, 9, 4], filter OHWI[4, 1, 1, 4], output NHWC[2, 1, 17, 4], and stride HW[1, 2], the sizing API returns 272 bytes while the kernel initializes and resets 288 bytes. A guarded host execution against the original implementation overwrote 16 bytes beyond the reported allocation. The corrected size leaves the guard intact. This surfaced as output corruption in ExecuTorch when its memory planner placed a live output immediately after the scratch allocation.Use the width stride in both sizing functions, matching the existing kernel. The execution kernel and reverse-convolution selection are unchanged. Add 15 binding regression cases covering asymmetric strides and a larger-filter control across scalar, DSP, and MVE backends.
Validation: all 23 tests in
Tests/Bindingspass on the rebased branch, and the changed C file passes clang-format 18. The asymmetric-stride cases fail against the original sizing implementation. The same fix was also validated against ExecuTorch's previously pinned CMSIS-NN revision with an end-to-end Cortex-M55 reproducer, with and without Hardtanh; the focused ExecuTorch run had 51 passes and 9 existing expected failures.Authored and validated with Codex assistance.