if transpose can't be cancelled , do not push it by xiaohanAMD · Pull Request #32714 · microsoft/onnxruntime

xiaohanAMD · 2026-09-21T15:09:30Z

Summary
Stop the transpose optimizer from pushing a Transpose through an op when that move does not actually cancel a later Transpose.

Stock ORT treats “any downstream Transpose” as a reason to push. Two Transposes cancel only when their perms are inverses (both disappear). They merge when the perms compose into a remaining Transpose. For a shared Transpose, a merge does not remove the original node, so the graph is not simpler and QDQ layout can be broken.

Behavior
Cost model: waive the cost of a Transpose inserted on the output only if a downstream Transpose would cancel the pushed perm, or the incoming Transpose has a single consumer (merge still drops a node). A shared Transpose that would only merge is not pushed.

QDQ guard: if a Transpose already sits in a complete DQ → Transpose → Q unit, do not push it through that Q unless the relocated Transpose would cancel downstream. Dead-end branches (no Transpose) are allowed when cancellation happens on another branch, because the relocated Transpose can stay on those branches.

Test plan
TestSliceSharedTransposeCancel / TestSliceSharedTransposeNoCancel / TestSliceUnsharedTransposeNoCancel
TestQDQSharedTransposeSliceCancel / TestQDQSharedTransposeSliceNoCancel
TestQDQTransposeNotPushedThroughOwnQ

azure-pipelines · 2026-09-21T15:09:39Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Copilot review overview

🟡 Changes recommended

Several traversal and cost paths can still misclassify cancellation or bypass the QDQ safeguard.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 4

Open (5)

Apply QDQ protection to every final push-transpose decision · New
Reject incomplete consumer sets during cancellation analysis · New
Validate propagated permutations before transpose cancellation analysis · New
Avoid quadratic transpose cancellation walks · New
Account for all outputs when waiving transpose cost · New

What changed in this PR

Refines transpose optimization to push shared transposes only when downstream cancellation provides a real benefit while preserving QDQ units.

Changes:

Adds cancellation-aware transpose cost analysis.
Guards complete QDQ units from unhelpful rewrites.
Adds shared, unshared, and QDQ regression tests.

File	Description
onnxruntime/test/optimizer/transpose_optimizer_test.cc	Adds transpose cancellation and QDQ test scenarios.
onnxruntime/core/optimizer/transpose_optimization/onnx_transpose_optimization.cc	Implements cancellation-aware cost and QDQ safeguards.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Apply the QDQ guard to every push decision, preview handler perms, reject incomplete consumers, cache walks, and waive output cost only when every pushed output is beneficial. Co-authored-by: Cursor <cursoragent@cursor.com>

Only waive the inserted output Transpose cost when an output actually leads to a downstream Transpose. The sole-consumer merge exception was being applied even with no downstream Transpose, which pushed Transposes the optimizer should leave alone (TestOnlyOptimizeTowardsTranspose, MobileClip attention fusion). Restrict the QDQ node unit guard to the default cost path again. Layout transformation returns kPushTranspose to move layout Transposes through Q/DQ on purpose and relies on FixQDQNodeUnits afterwards, so blocking that stopped layout transformation entirely (ConstantFoldTransposeAndSqueezeOutputCorrectness). Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Copilot review overview

🟡 Changes recommended

Cancellation prediction can cross infeasible paths, use incorrect output permutations, and strand relocated Transposes.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 3

Open (4)

Replace recursive DFS with an explicit worklist · New
Validate unmodeled handlers before traversing permutations · New
Apply transposability and modifiability checks during cancellation walks · New
Use rank-adjusted permutation when walking through handlers · New

Resolved since last review (5)

Apply QDQ protection to every final push-transpose decision
Account for all outputs when waiving transpose cost
Avoid quadratic transpose cancellation walks
Validate propagated permutations before transpose cancellation analysis
Reject incomplete consumer sets during cancellation analysis

…ow unpushable paths. The walk is iterative, previews only handlers that preserve perm (including HandleConcat), and requires a transposable, modifiable consumer plus a rank-adjusted output perm. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Cancellation analysis can approve pushes that never cancel or simplify the graph.

Review effort: Balanced
Findings: None

Resolved since last review (4)

Replace recursive DFS with an explicit worklist
Use rank-adjusted permutation when walking through handlers
Apply transposability and modifiability checks during cancellation walks
Validate unmodeled handlers before traversing permutations

Previously missed (2)

In code that hasn't changed since last review

Avoid cancellation predictions when downstream push is infeasible

onnxruntime/core/optimizer/transpose_optimization/onnx_transpose_optimization.cc:3292

This walk can report a cancellation through a node that the optimizer will never push through. For example, after pushing a shared Transpose through Slice, let that output feed Add(output, dynamic_same_rank) and then an inverse Transpose. PreviewPushedOutputPerm follows the Add and returns kCancel, but when Add is visited its input cost is -rank + rank == 0, so DefaultCostCheck stops and the newly inserted Transpose remains stranded (and the first rewrite can increase the Transpose count). The same false positive can also let the QDQ guard dismantle a node unit. The walk needs to include downstream push feasibility/cost, or conservatively stop at handlers whose other inputs prevent the push.

Do not waive merge cost for partially shared output Transposes

onnxruntime/core/optimizer/transpose_optimization/onnx_transpose_optimization.cc:3435

kept_output_transposes <= 1 counts node outputs, not whether the one surviving output Transpose is shared. With an unshared incoming Transpose, a single-output op whose output fans out to a non-cancelling Transpose and a dead-end branch passes this condition. After the push, the inserted output Transpose must remain for the dead-end branch, while merging on the other branch leaves another Transpose, so the graph still has two Transposes and gains nothing. Only waive the merge cost when every consumer branch reaches a merge/cancellation (or otherwise prove the inserted output Transpose itself is removable).

Copilot

Copilot review overview

🟡 Changes recommended

Cancellation analysis can traverse an intermediate operator that the cost model later refuses, stranding an additional Transpose.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Avoid false transpose cancellation through blocking multi-input ops · New

A shared Transpose through Slice into Add (or Concat) with a same-rank dynamic input was treated as free, then stranded; merge cost is also not waived when the kept output fans out. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Copilot review overview

🟡 Changes recommended

Cancellation and merge feasibility can be misclassified, causing stranded transposes or missed valid simplifications.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3

Open (3)

Validate input ranks before accepting broadcast cancellation · New
Trace downstream consumers before waiving transpose output cost · New
Allow fully covered multi-output transpose merges · New

Resolved since last review (1)

Avoid false transpose cancellation through blocking multi-input ops

…e cost only when kept outputs are fully covered or a unique incoming Transpose does not fan out downstream. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review effort: Lite
Findings: 3 · 6

Open (9)

ctx.graph.GetValueInfo(...) is dereferenced without checking for nullptr. If value-info is… · New
ctx.graph.GetValueInfo(...) is dereferenced without checking for nullptr. If value-info is… · New
Same potential null dereference as above: ctx.graph.GetValueInfo(inputs[i]) may be nullptr and… · New
The cache key construction allocates and formats integers (std::to_string) on every visited edge… · New
PreviewPushedOutputPerm requires a non-const api::NodeRef&, forcing const_cast at call sites.… · New
PreviewPushedOutputPerm requires a non-const api::NodeRef&, forcing const_cast at call sites.… · New
Allow fully covered multi-output transpose merges
Trace downstream consumers before waiving transpose output cost
Validate input ranks before accepting broadcast cancellation

…st_cast at its call sites. Guard GetValueInfo before reading a shape so a value without value info makes the preview conservative instead of crashing. Replace the string cache key with a structured key and hasher, and let PreviewPushedOutputPerm take a const NodeRef. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Fanout traversal can approve unreachable cancellations and unproductive rewrites, while cache invalidation introduces quadratic behavior on deep graphs.

Review effort: Balanced
Findings: None

Resolved since last review (9)

Same potential null dereference as above: ctx.graph.GetValueInfo(inputs[i]) may be nullptr and…
ctx.graph.GetValueInfo(...) is dereferenced without checking for nullptr. If value-info is…
ctx.graph.GetValueInfo(...) is dereferenced without checking for nullptr. If value-info is…
PreviewPushedOutputPerm requires a non-const api::NodeRef&, forcing const_cast at call sites.…
PreviewPushedOutputPerm requires a non-const api::NodeRef&, forcing const_cast at call sites.…
The cache key construction allocates and formats integers (std::to_string) on every visited edge…
Allow fully covered multi-output transpose merges
Trace downstream consumers before waiving transpose output cost
Validate input ranks before accepting broadcast cancellation

Previously missed (2)

In code that hasn't changed since last review

Misses merge fanout across separate Split outputs

onnxruntime/core/optimizer/transpose_optimization/onnx_transpose_optimization.cc:3498

This only detects a merge/dead-end fanout when the Transpose and the other consumer share the same value. For Transpose -> Relu -> Split, with a same-perm Transpose on one Split output and a graph-output branch on the other, the two Split outputs are visited separately, so one sees only has_transpose and the other only has_other. The function returns false, CalculateCost waives Relu's output cost, and the Transpose moves even though Split subsequently refuses the merge and no node is removed. Aggregate transpose and stranded-terminal state across all descendant outputs, not just consumers of one value.

Memo clearing causes quadratic traversal on unary chains

onnxruntime/core/optimizer/transpose_optimization/onnx_transpose_optimization.cc:3657

Clearing the entire memo after every successful push makes a long unary chain quadratic. With Transpose -> op1 -> ... -> opN -> inverse Transpose, each op performs a walk over the remaining suffix, then clears all results when the Transpose advances one node, for Θ(N²) consumer visits instead of the previous linear reverse reachability pass. Preserve unaffected downstream results or compute cancellation reachability incrementally so model initialization does not regress on deep graphs.

…os after a push. Per-value fanout missed Transpose -> Relu -> Split with a merge on one output and a dead-end on the other, and wiping the whole memo after each rewrite made unary chains quadratic. Co-authored-by: Cursor <cursoragent@cursor.com>

xiaohanAMD · 2026-09-24T13:53:10Z

@microsoft-github-policy-service agree [company="{your company}"]

@microsoft-github-policy-service agree [company="AMD"]

xiaohanAMD · 2026-09-24T13:54:37Z

@microsoft-github-policy-service agree [company="{your company}"]
@microsoft-github-policy-service agree [company="AMD"]
@microsoft-github-policy-service agree company="AMD"

Copilot

Copilot review overview

🟡 Changes recommended

The cancellation walk bypasses custom cost decisions and introduces quadratic traversal for merge chains.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Cache fanout reachability to prevent quadratic DFS re-walks · New

The fanout DFS kept its visited set local to each call, so a unique Transpose ahead of a long chain ending in a same-perm Transpose re-walked the whole suffix after every push. Track 'reaches a Transpose' and 'reaches a stranded terminal' as composable per-value flags in an OptimizerCtx memo that the existing rewrite invalidation clears. Co-authored-by: Cursor <cursoragent@cursor.com>

The two DFS copies used the same stack, in-progress marker, and rewrite invalidation with only the combine rule different, so a third walk could go call-local again. ExpandPushWalk classifies consumers; RunCachedPushWalk owns the memo. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Copilot review overview

🔵 Needs a closer look

The output-cost fallback can still push an unshared Transpose when the downstream Transpose is unreachable.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Cache fanout reachability to prevent quadratic DFS re-walks

Copilot

Copilot review overview

🟡 Changes recommended

The walk can waive costs across unreachable paths, and cache invalidation makes long chains quadratic.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Avoid quadratic full-map scans during cache invalidation · New

+  for (auto it = ctx.pushed_walk_cache.begin(); it != ctx.pushed_walk_cache.end();) {
+    if (rewired_values.find(it->first.value) != rewired_values.end()) {
+      it = ctx.pushed_walk_cache.erase(it);
+    } else {
+      ++it;
+    }


xiaohanAMD · 2026-09-28T07:50:29Z

I will close this PR. #32868 please review this new PR, Scott McKay (@skottmckay) mirounga
Edward Chen (@edgchen1)

if transpose can't be cancelled , do not push it

6964bc1

Xavier Dupré (xadupre) requested a balanced review from Copilot September 21, 2026 15:15

Copilot started reviewing on behalf of Xavier Dupré (xadupre) September 21, 2026 15:16 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

xiaoh and others added 2 commits September 22, 2026 00:07

Tighten transpose cancellation analysis from PR review.

80f2774

Apply the QDQ guard to every push decision, preview handler perms, reject incomplete consumers, cache walks, and waive output cost only when every pushed output is beneficial. Co-authored-by: Cursor <cursoragent@cursor.com>

xiaohanAMD requested a balanced review from Copilot September 23, 2026 02:53

Copilot started reviewing on behalf of xiaohanAMD September 23, 2026 02:56 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

xiaohanAMD requested a balanced review from Copilot September 23, 2026 05:47

Copilot started reviewing on behalf of xiaohanAMD September 23, 2026 05:49 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

xiaohanAMD requested a balanced review from Copilot September 23, 2026 12:36

Copilot started reviewing on behalf of xiaohanAMD September 23, 2026 12:37 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread onnxruntime/core/optimizer/transpose_optimization/onnx_transpose_optimization.cc Outdated

xiaohanAMD requested a balanced review from Copilot September 23, 2026 13:20

Copilot started reviewing on behalf of xiaohanAMD September 23, 2026 13:22 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Refuse broadcast cancel walks the handler cannot push, and waive merg…

0820f25

…e cost only when kept outputs are fully covered or a unique incoming Transpose does not fan out downstream. Co-authored-by: Cursor <cursoragent@cursor.com>

xiaohanAMD requested a balanced review from Copilot September 23, 2026 14:29

Copilot stopped reviewing on behalf of xiaohanAMD due to an error September 23, 2026 14:30
Failed to launch agent

Copilot AI reviewed Sep 23, 2026

View reviewed changes

xiaohanAMD requested a balanced review from Copilot September 24, 2026 12:22

Copilot started reviewing on behalf of xiaohanAMD September 24, 2026 12:23 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

xiaohanAMD requested a balanced review from Copilot September 24, 2026 13:49

Copilot started reviewing on behalf of xiaohanAMD September 24, 2026 13:50 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread onnxruntime/core/optimizer/transpose_optimization/onnx_transpose_optimization.cc Outdated

xiaoh and others added 2 commits September 24, 2026 09:08

xiaohanAMD requested a balanced review from Copilot September 24, 2026 14:16

Copilot started reviewing on behalf of xiaohanAMD September 24, 2026 14:17 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

xiaohanAMD requested a balanced review from Copilot September 24, 2026 17:25

Copilot started reviewing on behalf of xiaohanAMD September 24, 2026 17:26 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

mirounga requested review from Edward Chen (edgchen1), mirounga and Scott McKay (skottmckay) September 25, 2026 20:48

xiaohanAMD closed this Sep 28, 2026

Conversation

xiaohanAMD commented Sep 21, 2026

Uh oh!

azure-pipelines Bot commented Sep 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

xiaohanAMD commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xiaohanAMD commented Sep 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

xiaohanAMD commented Sep 28, 2026

xiaohanAMD commented Sep 24, 2026 •

edited

Loading