Arm backend: Generalize RemovePermutesAroundElementwiseTosaOps#20238
Open
AdrianLundell wants to merge 5 commits into
Open
Arm backend: Generalize RemovePermutesAroundElementwiseTosaOps#20238AdrianLundell wants to merge 5 commits into
AdrianLundell wants to merge 5 commits into
Conversation
- Use is_param_node for finding constant placeholders - Ensure ops are not modified by multiple subgraphs Signed-off-by: Adrian Lundell <adrian.lundell@arm.com> Change-Id: Ibb972d89c9c4125dc09bc918eae5f3fe81186cc0
AdrianLundell
added
partner: arm
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/20238
Note: Links to docs will display an error until the docs builds have been completed. ❌ 8 New Failures, 6 Unrelated FailuresAs of commit 601922d with merge base 0da9ca3 ( NEW FAILURES - The following jobs have failed:
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. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR generalizes the Arm TOSA pass that removes redundant permutes around elementwise ops by improving constant-placeholder detection and preventing invalid rewrites when multiple candidate subgraphs overlap.
Changes:
- Add a “stale subgraph” guard to skip applying rewrites when a previously-applied rewrite has already changed a candidate’s boundary edges.
- Update
RemovePermutesAroundElementwiseTosaOpsto useis_param_node(exported_program, node)for identifying constant placeholders, and threadexported_programthrough the Arm pass pipeline. - Expand and update Arm backend tests to cover folded scalar constants and shared-boundary/stale-subgraph behavior; adjust transpose count expectations accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backends/transforms/remove_permutes_around_elementwise_ops.py | Adds _subgraph_edges_are_current() and checks it before rewriting a candidate subgraph. |
| backends/arm/_passes/remove_permutes_around_elementwise_tosa_ops.py | Requires ExportedProgram, improves constant detection via is_param_node, and keeps TABLE constant inputs from being permuted. |
| backends/arm/_passes/arm_pass_manager.py | Wires exported_program into the RemovePermutesAroundElementwiseTosaOps pass construction. |
| backends/arm/test/passes/test_remove_permutes_around_elementwise_tosa_ops.py | Adds new unit tests for folded scalar constants and stale shared-boundary subgraphs; updates pass construction to provide an exported program. |
| backends/arm/test/misc/test_transpose_counts.py | Updates expected transpose count for a model affected by improved permute-removal behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
22
to
+27
| PERMUTE_TARGET = exir_ops.edge.aten.permute_copy.default | ||
| RESCALE_TARGET = exir_ops.backend.tosa.RESCALE.default | ||
| TABLE_TARGET = exir_ops.backend.tosa.TABLE.default | ||
| MUL_TARGET = exir_ops.edge.aten.mul.Tensor | ||
| ADD_TARGET = exir_ops.edge.aten.add.Tensor | ||
| ERF_TARGET = exir_ops.edge.aten.erf.default |
meta-cla
Bot
added
the
CLA Signed
Comment on lines
+76
to
+78
| result = RemovePermutesAroundElementwiseTosaOps(_fake_exported_program()).call( | ||
| graph_module | ||
| ) |
Contributor
There was a problem hiding this comment.
I'm not sure I get that part, can you expand on it?
Collaborator
Author
There was a problem hiding this comment.
Since it builds the graph directly instead of exporting it it needs to fake an exported program as the pass now requires it.
…tream/change-1265674 Change-Id: I1e041d0ed886b0122ef2cb1917416ebc979499fe
Signed-off-by: Adrian Lundell <adrian.lundell@arm.com> Change-Id: Ic1d87ceb42e425628c50d95b27eeb0fe7ac0e6ff
Comment on lines
+29
to
+31
| def _is_constant(self, node: torch.fx.Node) -> bool: | ||
| # Override fragile string match check with exported program check | ||
| return super()._is_constant(node) or is_param_node(self.exported_program, node) |
Comment on lines
+29
to
+31
| def _is_constant(self, node: torch.fx.Node) -> bool: | ||
| # Override fragile string match check with exported program check | ||
| return super()._is_constant(node) or is_param_node(self.exported_program, node) |
Comment on lines
+502
to
+504
| def _subgraph_edges_are_current(self, subgraph: Subgraph) -> bool: | ||
| """Return false if an earlier rewrite invalidated this candidate.""" | ||
| for inp, out in subgraph.edges_in: |
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.
cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell @rascani