Bound the dynamic-qdq traceback in XNNPACK ChannelsLastTaggedReshapePass - #21637
Open
Hyungkeun-Park-Nota wants to merge 1 commit into
Open
Bound the dynamic-qdq traceback in XNNPACK ChannelsLastTaggedReshapePass#21637Hyungkeun-Park-Nota wants to merge 1 commit into
Hyungkeun-Park-Nota wants to merge 1 commit into
Conversation
input_to_nhwc steps back over the dynamic q/dq wrapper so the NHWC copy is inserted ahead of the quantize. The loop stopped only once args[0] was not a Node, so it did not stop at the quantized tensor and ran on into ordinary compute. The rewrite that follows is a blanket replace_all_uses_with from wherever the walk landed, so overshooting either feeds an intermediate op NHWC while leaving that op's own output NCHW, which XNNPACK reports as xnn_status_invalid_parameter when propagating input shapes at execute(), or lands on a non-4D constant and raises "required rank 4 tensor to use channels_last format" in _to_copy. Restrict the walk to q/dq nodes. dq -> q -> source is two hops and the source is not a q/dq node, so it stops there. On a w8a8-dynamic detection model 69 of 83 tracebacks had been overshooting, by up to 26 hops; bounding them leaves the delegate count unchanged and drops 16 now-redundant transposes.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21637
Note: Links to docs will display an error until the docs builds have been completed.
|
Contributor
Author
|
@pytorchbot label 'module: xnnpack' 'release notes: xnnpack' |
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
The dynamic-quant branch of
ChannelsLastTaggedReshapePass.input_to_nhwctraces back over theq/dq wrapper so the NHWC copy is inserted ahead of the quantize, keeping the
x -> q -> dq -> convchain XNNPACK matches intact. The loop stops on "
args[0]is not a Node" rather than on "this isnot a q/dq node", so it does not stop at the quantized tensor and continues into ordinary compute:
The
input_node.replace_all_uses_with(input_node_nhwc)that follows is then applied from whereverthe walk landed, rewriting consumers the pass never reasoned about. That shows up two ways:
If the walk stops on an intermediate op, lowering succeeds but the serialized graph is
inconsistent, and only the first
execute()reports it:The failing partition contains an elementwise op whose input and output dims disagree, next to a
correct one in the sibling branch:
If the walk reaches a non-4D constant it fails earlier, during the pass:
b_mul_10_const_inputis a(256, 1, 1)per-channel constant. A placeholder has noargs, so thewalk stops there and tries to convert it.
can_be_converted_to_nhwcdoes check rank 4, but thispath never calls it.
Both were found on w8a8 dynamic-quantized vision models, a YOLOX detector for the first and a SAM
image encoder for the second.
Fix
Restrict the walk to q/dq nodes.
dq -> q -> sourceis two hops and the source is not a q/dq node,so the walk stops at the source, which is the node it was aiming for, and the
x -> _to_copy -> q -> dq -> convordering is unchanged.is_quantis already exported frombackends/xnnpack/utils/quant_utils.pyalongside theis_dynamic_qdqthis file imports.Instrumenting the loop on the YOLOX graph: 83 invocations, every one with exactly 2 q/dq hops, and
69 of them (83%) walking past that, up to 26 hops. Bounding the walk leaves the delegate count at
16 either way and removes 16
XNNStaticTransposenodes (414 to 398 total), so the overshoot wasnot buying larger fused partitions.
Test
test_dq_conv2d_eltwise_source_channels_last_tagged_reshape_passbuilds the smallest graph thattriggers it: a dynamically quantized conv whose input is a
sigmoidreading the placeholder, sothe conv sees
sigmoid -> q -> dq. It asserts the sigmoid keeps its placeholder input and that thechannels-last copy sits on the sigmoid's output.
Without the fix the pass produces
x -> _to_copy(channels_last) -> sigmoid -> q -> dq -> convandthe test fails with
AssertionError: 'call_function' != 'placeholder'; with it the order isx -> sigmoid -> _to_copy(channels_last) -> q -> dq -> conv. The assertion is structural becausechannels_lastdoes not change eager results, sorun_method_and_compare_outputsalone cannotcatch this.
The full file passes (22 tests). Separately, 10 w8a8 dynamic models that already lowered and
executed correctly before this change (googlenet, inception_v3, efficientnet_b4, wideresnet50,
sesr_m5, mobile_vit_s, swin_t, vit_b_16, quicksrnet_small, squeezenet1_0) were re-lowered with the
grouped partitioner and executed: 10/10 pass. The two models above now lower and execute, with
output shapes matching a per-op-partitioned reference build.
cc @GregoryComer @digantdesai @cbilgin @JakeStevens