Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/17740
Note: Links to docs will display an error until the docs builds have been completed. ❌ 4 New Failures, 1 Cancelled Job, 2 Unrelated FailuresAs of commit 7d11bdb with merge base e95555a ( NEW FAILURES - The following jobs have failed:
CANCELLED JOB - The following job was cancelled. Please retry:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
bot
added
the
CLA Signed
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
Removes the legacy Cortex-M post-processing step from aot_arm_compiler.py and makes the delegation pipeline more explicit by applying remaining Q/DQ cleanup directly in the delegated export flow.
Changes:
- Removed
transform_for_cortex_m_backend()and the--enable_qdq_fusion_passCLI flag. - Dropped unused Cortex-M fusion/convert pass imports tied to the removed flag.
- Applied
ReplaceQuantNodesPassinsideto_edge_TOSA_delegate()to handle boundaryquantized_decomposed::*ops after lowering.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…pass flag Summary: Remove the transform_for_cortex_m_backend() function and the --enable_qdq_fusion_pass CLI flag from aot_arm_compiler.py. The function applied Cortex-M passes as a post-hoc step to all non-VGF targets, which made the compilation flow hard to follow and coupled the delegation path to Cortex-M-specific logic. Instead, ReplaceQuantNodesPass is now applied directly inside to_edge_TOSA_delegate() to handle any boundary quantized_decomposed::* nodes that remain outside the delegated subgraph. This makes the delegation path self-contained and explicit about its runtime requirements. This change is in preparation for an upcoming PR (#17075) that introduces Cortex-M as a first-class compilation target with its own dedicated pipeline, including CortexMQuantizer and CortexMPassManager.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| parser.add_argument( | ||
| "--non_strict_export", | ||
| dest="strict_export", | ||
| required=False, | ||
| action="store_false", | ||
| help="Disable strict checking while exporting models.", | ||
| ) |
There was a problem hiding this comment.
This PR removes the --enable_qdq_fusion_pass CLI flag, but examples/arm/run.sh still constructs and passes --enable_qdq_fusion_pass (see run.sh around the qdq_fusion_op_flag assignment). That script will now fail with an “unrecognized arguments” error; update the script (and any other callers) to stop using the removed flag.
There was a problem hiding this comment.
Will clean it up in follow up PR
There was a problem hiding this comment.
If "One way is to keep the flag but it it does nothing more the print that is deprecated?" is done that would be OK (Se my other comment)
But if NOT it will break run.sh args for a while (as the flag can the used from it) so in that case its better to try to fix both in same PR to keep stuff "working" all the time.
There was a problem hiding this comment.
Added deprecated text to the flag in this PR,
Updated the _apply_replace_quant_nodes function to accept a generic edge argument instead of EdgeProgramManager.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zingo
added
the
partner: arm
zingo
changed the title
zingo
changed the title
Remove deprecated --enable_qdq_fusion_pass argument and related logging.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary:
Remove the transform_for_cortex_m_backend() function and deprecate --enable_qdq_fusion_pass CLI flag from aot_arm_compiler.py. Instead, ReplaceQuantNodesPass is now applied directly inside to_edge_TOSA_delegate() and to_edge_no_delegate(), making each compilation path self-contained rather than relying on a post-hoc fixup applied to all targets.
This is a prerequisite for PR #17075, which introduces Cortex-M as a first-class compilation target with its own dedicated pipeline.
cc @digantdesai @SS-JIA @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell