Repository navigation
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24121
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit eb39e44 with merge base 703a411 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
Contributor
|
@pssrawat has exported this pull request. If you are a Meta employee, you can view the originating Diff in D127046193. |
This PR needs a
|
Summary:
`ValidateCortexMExplicitLayoutPass` requires every NHWC layout copy to move
an int8 tensor, and reads `node.meta["val"].dtype` to decide. In
`explicit_layout_pass_list` that validator runs before `AtenToCortexMPass`,
so at validation time only the activations that `ActivationFusionPass` has
already folded into the convolution clamp -- relu, clamp, hardtanh -- carry
an int8 fake tensor. The LUT activations (tanh, sigmoid, which become
`cortex_m::quantized_activation`) and softmax (which becomes
`cortex_m::softmax`) are still `aten.*` with an fp32 fake tensor, even
though `FoldAndAnnotateQParamsPass` has already attached their int8 qparams.
That fp32 propagates into any convolution fed by one, and the layout copy on
its output trips the assertion.
The explicit-layout path validates rather than falling back, so this is a
hard abort, not a coverage loss -- and explicit layout is the only mode that
supports Conv1d. A two-node graph reproduces it: `Conv1d(relu(x))` lowers,
`Conv1d(tanh(x))` raises
RuntimeError: Cortex-M layout copy channels_last_permute_copy_default
must move an int8 tensor.
out of `_CortexMLoweringPass`. `LutActivationThenConv1d` in
`test_explicit_layout_pipeline.py` is that graph.
Judge the condition the kernels will actually see. `produces_int8` accepts
an int8 fake tensor as before, and otherwise reads the folded
`output_qparams[0]`, which is in place well before this point. Graphs that
were rejected lower unchanged: on a 31-convolution ECAPA-TDNN speaker model,
0 of 31 convolutions reached a CMSIS-NN kernel before and 31 of 31 do after,
with 0.000e+00 max absolute difference against the quantized eager module.
Folded qparams on their own are the wrong bar, though, and this is what the
first version of the diff got wrong. They say what dtype a node was
annotated for, not that anything acts on the annotation, and a node nothing
rewrites keeps an fp32 kernel in the middle of an int8 chain. So
`produces_int8` also asks whether a rewrite is coming: either
`AtenToCortexMPass` has a substitution registered for the target, or
`QuantizedClampActivationPass` has already put the node on int8 terms. It
lives in `aten_to_cortex_m_pass.py`, which owns the substitution registry
and is therefore the module that can answer the question.
`QuantizedClampActivationPass` is the pass that makes the second arm
necessary, and it had the same premature read. It quantizes the bounds of a
clamp that will see int8, and decided that from the producer's fake tensor,
so a clamp behind a LUT activation was skipped -- harmless while the
validator rejected those graphs outright, silently wrong once it stopped.
`sigmoid -> relu -> Conv1d` left `aten.relu` clamping at 0 instead of the
zero point, erasing everything below it, where the relu should have been a
no-op; `tanh -> clamp(-0.5, 0.5) -> Conv2d` kept fp32 limits and an fp32
value on the tensor `cortex_m::quantized_conv2d_nhwc` reads as int8. Both
fail an output comparison against the quantized eager module, so this was
wrong numerics, not a wrong annotation. It now asks `produces_int8`, seven
passes ahead of the validator, and records the clamps it quantized.
That record is a node predicate in `cortex_m/utils.py`, alongside
`skips_quantized_max_pool2d`, which uses the same `meta["custom"]`
convention. A clamp never becomes a `cortex_m::` kernel -- it stays on the
portable one and reads int8 only because its bounds were rewritten into
codes -- so no other pass can re-derive the decision, and a clamp the pass
declined (bounds that are not compile-time scalars, output qparams that are
not per-tensor) is indistinguishable from one it never saw. With the
decision recorded, a declined clamp is rejected by the validator instead of
being lowered.
`has_qparams` moves to `cortex_m/utils.py` as part of this. It was private
to `aten_to_cortex_m_pass`; utils is already where this backend keeps its
node predicates. Its ten existing call sites change in name only.
The error message now also names the producing node. The layout copy is
synthesized by this pass and appears in no graph the caller holds, so its
own name gives nothing to go on.
Differential Revision: D126708891
Summary: `_get_permute_replacement`, `_get_minimum_replacement` and `_get_maximum_replacement` choose their kernel from the input fake tensor. That tensor is stale inside `AtenToCortexMPass`: `AtenToDialectPass` gives each new `cortex_m` node the replaced node's meta dict, so an input already rewritten from `aten.tanh` still reads fp32 until the retrace at the end of the pass. Read `lowered_output_dtype` instead, as the layout validator and clamp pass now do. Rewritten nodes keep their folded `output_qparams`, so it reports int8 for them; a `cortex_m` dequantize has none and still reports float. The cost was coverage, not numerics. `Conv1d(tanh(x))` lowered its input-side layout move to a portable `aten::permute_copy` and its output-side move to `cortex_m::transpose`, and `minimum(tanh(x), y)` / `maximum(tanh(x), y)` stayed on portable kernels. That breaks the explicit-layout guarantee that every layout move is a CMSIS transpose, and a selective build that registers only the expected kernels fails to load. Differential Revision: D127046193
meta-codesync
Bot
changed the title
pssrawat
force-pushed
the
export-D127046193
branch
from
October 23, 2026 19:35
72d21fd to
eb39e44
Compare
This branch has not been deployed
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
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.
Summary:
_get_permute_replacement,_get_minimum_replacementand_get_maximum_replacementchoose their kernel from the input fake tensor. That tensor is stale insideAtenToCortexMPass:AtenToDialectPassgives each newcortex_mnode the replaced node's meta dict, so an input already rewritten fromaten.tanhstill reads fp32 until the retrace at the end of the pass.Read
lowered_output_dtypeinstead, as the layout validator and clamp pass now do. Rewritten nodes keep their foldedoutput_qparams, so it reports int8 for them; acortex_mdequantize has none and still reports float.The cost was coverage, not numerics.
Conv1d(tanh(x))lowered its input-side layout move to a portableaten::permute_copyand its output-side move tocortex_m::transpose, andminimum(tanh(x), y)/maximum(tanh(x), y)stayed on portable kernels. That breaks the explicit-layout guarantee that every layout move is a CMSIS transpose, and a selective build that registers only the expected kernels fails to load.Differential Revision: D127046193