Uh oh!
There was an error while loading. Please reload this page.
Cortex-M backend: plan avg_pool2d scratch buffer AoT - #19825
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19825
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 6d749a7 with merge base 189ffaa ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a |
CMSIS-NN's DSP-variant `arm_avgpool_s8` returns `ARM_CMSIS_NN_ARG_ERROR` when its `ctx->buf` is NULL and `arm_avgpool_s8_get_buffer_size(...)` is non-zero. The kernel hardcoded `cmsis_ctx.buf = nullptr`, which worked on Cortex-M55 because the MVE variant ignores `ctx` entirely, but failed on any DSP-class core (e.g. Cortex-M7). The AoT scratch-buffer planning system introduced for conv/depthwise-conv/transpose-conv/bmm missed `quantized_avg_pool2d`; this extends it to cover that op. The `quantized_avg_pool2d` and `.out` schemas gain a `Tensor scratch` parameter. A new `cmsis_nn_avgpool_buffer_size` size function is registered, and avg_pool2d's lowering moves out of `QuantizedOpFusionPass` (which cannot create `exir.memory.alloc` nodes because it routes through `ExportPass.call_operator`) into `ConvertToCortexMPass`, alongside conv2d/bmm. The `count_include_pad` decomposition into an explicit `cortex_m::pad` node carries over to the new location. The kernel reads `scratch.nbytes()` and `scratch.mutable_data_ptr<int8_t>()` to wire `cmsis_nn_context`, with a `CORTEX_M_ENABLE_RUNTIME_CHECKS`-guarded assertion that the AoT and runtime buffer sizes agree — matching the conv2d pattern. The Python `_NHWC_DIM_ORDER` / `to_physical_order` helper that both passes need moves into `passes_utils.py` to avoid duplication. A new dialect test exercises the `ceil_mode=True` fallback so that future refactors do not silently change which path it takes. Authored with Claude.
4fd3c18 to
47e7fb8CompareUh oh!
There was an error while loading. Please reload this page.
digantdesai
commented
Jun 9, 2026
@claude review this PR. |
Claude finished @digantdesai's task in 3m 6s —— View job PR Review: Cortex-M backend: plan avg_pool2d scratch buffer AoT
Overall: Clean, well-structured PR that follows the established scratch-buffer AoT planning pattern (conv2d, depthwise-conv, transpose-conv, bmm). The move from Findings1. List aliasing in pre_pad=post_pad=to_physical_order([0, 0, pad_h, pad_w], input_tensor)When Suggestion: 2. No negative-return guard on The conv2d kernel's runtime check guards against negative return values from 3. Runtime check placement ( The runtime check validates the buffer size after the buffer pointer has already been assigned to 4. Reviewer question re: buffer size assertion in test (responding to @digantdesai's comment on line 108) The 5. Schema & YAML consistency — Verified: the 6. Test coverage — Good. The PR adds:
SummaryThis is a solid, well-scoped extension of the existing AoT scratch buffer system. The code correctly follows the established patterns, the kernel change is minimal and guarded, and the test coverage is reasonable. The only material suggestion is the minor list-aliasing nit in the pad decomposition path. |
…t-scratch # Conflicts: # backends/cortex_m/passes/convert_to_cortex_m_pass.py
Adapt _get_avg_pool2d_replacement from the old ConvertToCortexMPass class to a @register_dialect_substitution function on AtenToCortexMPass, matching the pattern established by pytorch#20070. Add casts to satisfy mypy's node.args union type. Add a structural assertion to test_dialect_avg_pool2d that verifies the scratch buffer size matches cmsis_nn.avgpool_buffer_size() for the given input/output dimensions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
) Use `dialect_pass: AtenToDialectPass` parameter matching the SubstitutionFn type updated in #19676. The `exported_program` parameter caused a mypy arg-type error when both #19676 and #19825 landed on main. Fixes the lintrunner-mypy failure on main. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The library is generated from this repository, so an ExecuTorch change can break it without touching examples/arduino at all. Nothing checked that. The job builds the library, verifies the shipped models against it, compiles every example for arduino:zephyr:unoq, and runs arduino-lint in Library Manager submission mode. It runs on changes under examples/arduino, backends/cortex_m, kernels/portable, runtime, and schema, which is the set that can break any of the four. verify_models.py is the part worth having. A .pte records how many values each operator call puts on the stack, and the generated kernel wrappers reject anything else; the two only agree when the model and the library came from the same commit. Cortex-M schemas change - scratch was added to the conv operators in pytorch#19636 and pytorch#19825 - and a mismatch is invisible until far too late, because the program loads, every operator resolves, and then execute returns InvalidProgram naming nothing useful. Tracking that down by hand took the better part of a day. The check compares the two directly in about a second, needs no board, and reports: FAIL KeywordSpotting cortex_m::quantized_avg_pool2d.out: model supplies 10, library expects 11 Compiling is deliberately not the whole job. Every failure worth finding in this library compiled cleanly first, so the model check runs before the sketches and arduino-lint guards the thing users actually install. Authored with Claude Code (Opus 5).
The library is generated from this repository, so an ExecuTorch change can break it without touching examples/arduino at all. Nothing checked that. The job builds the library, verifies the shipped models against it, compiles every example for arduino:zephyr:unoq, and runs arduino-lint in Library Manager submission mode. It runs on changes under examples/arduino, backends/cortex_m, kernels/portable, runtime, and schema, which is the set that can break any of the four. verify_models.py is the part worth having. A .pte records how many values each operator call puts on the stack, and the generated kernel wrappers reject anything else; the two only agree when the model and the library came from the same commit. Cortex-M schemas change - scratch was added to the conv operators in pytorch#19636 and pytorch#19825 - and a mismatch is invisible until far too late, because the program loads, every operator resolves, and then execute returns InvalidProgram naming nothing useful. Tracking that down by hand took the better part of a day. The check compares the two directly in about a second, needs no board, and reports: FAIL KeywordSpotting cortex_m::quantized_avg_pool2d.out: model supplies 10, library expects 11 Compiling is deliberately not the whole job. Every failure worth finding in this library compiled cleanly first, so the model check runs before the sketches and arduino-lint guards the thing users actually install. Authored with Claude Code (Opus 5).
The library is generated from this repository, so an ExecuTorch change can break it without touching examples/arduino at all. Nothing checked that. The job builds the library, verifies the shipped models against it, compiles every example for arduino:zephyr:unoq, and runs arduino-lint in Library Manager submission mode. It runs on changes under examples/arduino, backends/cortex_m, kernels/portable, runtime, and schema, which is the set that can break any of the four. verify_models.py is the part worth having. A .pte records how many values each operator call puts on the stack, and the generated kernel wrappers reject anything else; the two only agree when the model and the library came from the same commit. Cortex-M schemas change - scratch was added to the conv operators in pytorch#19636 and pytorch#19825 - and a mismatch is invisible until far too late, because the program loads, every operator resolves, and then execute returns InvalidProgram naming nothing useful. Tracking that down by hand took the better part of a day. The check compares the two directly in about a second, needs no board, and reports: FAIL KeywordSpotting cortex_m::quantized_avg_pool2d.out: model supplies 10, library expects 11 Compiling is deliberately not the whole job. Every failure worth finding in this library compiled cleanly first, so the model check runs before the sketches and arduino-lint guards the thing users actually install. Authored with Claude Code (Opus 5).
…els (#21546) ## Summary The Arduino library this directory generates could not compile, and had it compiled it could not have run a model. Kernels never reached the operator registry — ExecuTorch registers them through codegen and the build script never ran it, so every `Method::load` would have failed with `OperatorMissing`. CMSIS-NN was never vendored, so the Cortex-M ops shipped without the library they call. `schema/*.cpp` was never copied, the `*_aten.cpp` exclusion also deleted the portable-mode `tensor_parser_exec_aten.cpp`, and `__errno` was missing because the Zephyr core mixes picolibc with newlib's `libm_nano`. Both `minimal.cpp` and `zephyr.cpp` were vendored, so which `et_pal_*` backend you got depended on link order, and every `ET_LOG` was discarded either way. One backend now ships and its logs route to a hook the examples implement against `Serial`. The examples ship their models — previously none did, so every sketch `#error`ed when opened from the IDE menu. The README pointed at the Ethos-U `pte_to_header.py`, whose `network_model_sec` section no Arduino core defines, so following the docs produced a model that fails `Program::load`. Static link mode is mandatory and undocumented; the Dynamic default yields a silently dead board. Renamed to `ExecuTorch` because `arduino-lint` rejects "Arduino" in an Arduino library's name. Registering every portable kernel costs 1.58 MB against 786 KB of flash, so the op set is a curated default overridable via `ROOT_OPS`/`ALL_OPS`. Models and libraries must come from the same ExecuTorch commit — Cortex-M schemas change (`scratch` in #19636, #19825), and a mismatch loads fine, resolves every operator, then fails at `Method::execute`. The library now records and pins that commit. CI to enforce it follows separately. ## Test plan `arduino:zephyr:unoq`, board core 0.55.2, `link_mode=static`, flashed on hardware: ``` HelloExecuTorch Model loaded OK!, 1 method 60% flash, 20% RAM AddModel [1,2,3] + 1 = [2.00, 3.00, 4.00] 64% flash, 26% RAM KeywordSpotting 10/10 keywords correct 70% flash, 53% RAM ``` All ten MFCC inputs in one sketch, exercising the CMSIS-NN conv / depthwise / avgpool / linear kernels: ``` yes 8.95 no 4.78 up 4.63 down 9.72 left 7.87 right 7.41 on 12.03 off 8.02 stop 8.64 go 9.57 ``` `arduino-lint --library-manager submit`: no errors, no warnings, under both `specification` and `strict`. Clean regeneration is byte-identical across all 626 generated files. Only the Uno Q was tested; the other three boards remain marked Planned. Authored with Claude Code (Opus 5).
The library is generated from this repository, so an ExecuTorch change can break it without touching examples/arduino at all. Nothing checked that. The job builds the library, verifies the shipped models against it, compiles every example for arduino:zephyr:unoq, and runs arduino-lint in Library Manager submission mode. It runs on changes under examples/arduino, backends/cortex_m, kernels/portable, runtime, and schema, which is the set that can break any of the four. verify_models.py is the part worth having. A .pte records how many values each operator call puts on the stack, and the generated kernel wrappers reject anything else; the two only agree when the model and the library came from the same commit. Cortex-M schemas change - scratch was added to the conv operators in #19636 and #19825 - and a mismatch is invisible until far too late, because the program loads, every operator resolves, and then execute returns InvalidProgram naming nothing useful. Tracking that down by hand took the better part of a day. The check compares the two directly in about a second, needs no board, and reports: FAIL KeywordSpotting cortex_m::quantized_avg_pool2d.out: model supplies 10, library expects 11 Compiling is deliberately not the whole job. Every failure worth finding in this library compiled cleanly first, so the model check runs before the sketches and arduino-lint guards the thing users actually install.
Summary
CMSIS-NN's DSP-variant
arm_avgpool_s8returnsARM_CMSIS_NN_ARG_ERRORwhen itsctx->bufis NULL andarm_avgpool_s8_get_buffer_size(...)is non-zero. The kernel hardcodedcmsis_ctx.buf = nullptr, which worked on Cortex-M55 because the MVE variant ignoresctxentirely, but failed on any DSP-class core (e.g. Cortex-M7). The AoT scratch-buffer planning system introduced for conv/depthwise-conv/transpose-conv/bmm missedquantized_avg_pool2d; this extends it to cover that op.The
quantized_avg_pool2dand.outschemas gain aTensor scratchparameter. A newcmsis_nn_avgpool_buffer_sizesize function is registered, and avg_pool2d's lowering moves out ofQuantizedOpFusionPass(which cannot createexir.memory.allocnodes because it routes throughExportPass.call_operator) intoConvertToCortexMPass, alongside conv2d/bmm. Thecount_include_paddecomposition into an explicitcortex_m::padnode carries over to the new location. The kernel readsscratch.nbytes()andscratch.mutable_data_ptr<int8_t>()to wirecmsis_nn_context, with aCORTEX_M_ENABLE_RUNTIME_CHECKS-guarded assertion that the AoT and runtime buffer sizes agree — matching the conv2d pattern. The Python_NHWC_DIM_ORDER/to_physical_orderhelper that both passes need moves intopasses_utils.pyto avoid duplication.Test plan
A new dialect test exercises the
ceil_mode=Truefallback so that future refactors do not silently change which path it takes.Authored with Claude.