Repository navigation
Conversation
…ow-rank broadcast operands Profiling a conv encoder-decoder (MI-GAN inpainting, fp16) showed the graph split into 7-8 XNNPACK partitions around portable upsample_nearest2d ops and ~25% of the time spent in NCHW<->NHWC transposes. upsample_nearest2d.vec with an exact x2 scale on a static 4D input is now partitioned and lowered as concatenate4 of the input along channels followed by depth_to_space_2d(block_size=2). XNNPACK has no nearest resize and its static_broadcast is not implemented at run time. This adds an XNNDepthToSpace2d node to the flatbuffer schemas and XNNCompiler. Static quantization is supported: the quantizer annotates the op with shared input/output qparams and the intermediate value carries them. Other scales, including an explicit scale_factors that is not exactly 2.0, stay portable. In ChannelsLastTaggedReshapePass, single-element operands of add/sub/mul/div/maximum/minimum (a lifted Python scalar such as the gain in x * 1.4142) no longer take part in the layout decision, and rank < 4 operands that broadcast over the trailing dims (a [H, W] map added to [N, C, H, W]) are reshaped to 4D with leading ones so they follow the NHWC operand instead of dragging the activation back to NCHW. Dequantized operands keep their rank, since a view between the dequantize and the op would carry no quant params.
Backend passes keep a reference to the ExportedProgram they were built from (XNNPACKPass.exported_program, for one), while ExportedProgramPassManager runs them on a shallow copy of it and rebuilds the signature of that copy afterwards. delete_constant_placeholder replaced the original program's signature object, so the copy kept the old one and its verifier failed with Parameter <name> is not in the state dict. create_constant_placeholder already inserts into the shared input_specs list in place; do the same for deletion. Tests cover a deletion made inside a pass run through the manager for every constant placeholder kind. Authored with Claude Code.
…vs + depth_to_space XNNPACK runs transposed convolutions through its subconvolution IGEMM path one group at a time. With one channel per group (a depthwise ConvTranspose2d, the natural way to write a StyleGAN upfirdn x2 upsampling filter) that is far slower than the depthwise conv microkernels: on MI-GAN the twelve 4x4 / stride-2 filters more than double the forward. DecomposeDepthwiseConvTransposePass rewrites such a node, when it is a clean x s upscale ((H-1)s - 2p + k + output_padding == sH, dilation 1, every phase's taps inside the input), into s*s stride-1 depthwise convs on the low-resolution input (phase (ay, ax) correlates with every s-th tap of the kernel, through a possibly asymmetric zero pad), a channel concatenation in (ay, ax) order and a block-major depth_to_space_2d. That last op is XNNPACK's native channel order (aten.pixel_shuffle is channel-major), so it is a new et_xnnpack custom op created only after partitioning, with a reference implementation for tracing and tests, a node visitor emitting XNNDepthToSpace2d, and an NHWC tag in the channels-last pass. The pass runs before FuseActivationPass, so an activation after the transposed conv is never fused into the node it removes. Every convolution is planned before any is rewritten, so deconvs feeding each other work, and the phase weights keep the original weight's input kind (parameter, buffer or constant). Deleting the folded weight relies on delete_constant_placeholder editing the graph signature in place. Authored with Claude Code.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24114
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 2 PendingAs of commit 12f2694 with merge base 4628b2b ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@pytorchbot label "release notes: xnnpack" |
JakeStevens
left a comment
There was a problem hiding this comment.
there is one bug: two deconvs that share one weight can silently produce wrong output
_decompose names each phase weight f"{weight_node.name}_phase_{ay}_{ax}". When a placeholder with that name already exists, create_constant_placeholder returns it instead of making a new one. But, the phase kernels depend on the stride and padding as well as the weight. So the second deconv gets the first one's kernels whenever the weight is shared but the stride or padding differs.
Repro: one Parameter used by two depthwise deconvs, both clean ×s upscales, with different padding.
- k=5, s=3, (p=1, op=0) and (p=2, op=2): the pass crashes. The aten.cat shape check fails during the retrace because the reused kernels have
the wrong number of taps. - k=9, s=3, (p=3, op=0) and (p=4, op=2): no error, but the output is wrong. Only 9 p_w_phase_* placeholders exist for 18 phase convs, and the
max error against eager is 15.3. - Control, same model with two separate copies of the weight: max error 1.9e-6.
Suggested fix: include s and padding in the name, e.g. f"{weight_node.name}_s{s}p{padding[0]}x{padding[1]}phase{ay}{ax}".
A regression test along the lines of the k=9 case is worth adding.
create_constant_placeholder returns an existing placeholder of the same name, so two depthwise deconvs sharing one weight but using different stride or padding ended up with the first one's phase kernels. Include both in the name and add tests for a shared weight with different and with identical parameters.
|
Good catch, thanks. Fixed in c22288b: the phase placeholders are now named |
|
@borys3kk rebasing with main should resolve the failing CI tests |
|
Merged main in 12f2694, thanks. |
JakeStevens
left a comment
There was a problem hiding this comment.
The shared-weight fix in c22288b7 holds — I checked it across several configs. Two blocking items are inline: test coverage of the lowered path, and the external-constant tag.
| .run_method_and_compare_outputs(atol=tol, rtol=tol) | ||
| ) | ||
|
|
||
| def _check_lowered(self, module, inputs, tol=1e-3, dynamic_shapes=None): |
There was a problem hiding this comment.
the lowered tests pass whether or not the decomposition happens
_check_lowered checks for one delegate, no top-level convolution, and matching outputs. All three already held before this PR: ConvolutionConfig delegates an fp32 depthwise transposed conv and it serializes as XNNConvTranspose2d. The structural checks run the pass on its own through RunPasses, so nothing exercises it in the default XNNPACKPassManager list that preprocess uses. If it falls out of that list, gets reordered, or _plan stops matching inside preprocess, all 17 tests stay green and the speedup quietly disappears. test_deconv_relu only calls _check_lowered, so the behaviour in its comment is never checked.
Repro: stub the pass out during lowering with mock.patch.object(DecomposeDepthwiseConvTransposePass, "call", lambda self, gm: PassResult(gm, False)). DwDeconv(4, 2, 1) still lowers to 1 delegate with 0 top-level convs, and _decompose is never called.
Suggested fix: assert in _check_lowered that the pass actually ran during lowering, e.g.
with mock.patch.object(
DecomposeDepthwiseConvTransposePass,
"_decompose",
autospec=True,
side_effect=DecomposeDepthwiseConvTransposePass._decompose,
) as decompose:
tester.to_edge_transform_and_lower()
self.assertEqual(decompose.call_count, num_deconvs)For test_deconv_relu, add a pass-level check: run RunPasses([DecomposeDepthwiseConvTransposePass, FuseActivationPass]) and assert the single relu reads the depth_to_space_2d output. That pins down the ordering the description relies on. If the pass ever moves after FuseActivationPass, the ReLU gets fused into the conv the pass deletes. A pass list in the default order, as in test_fuse_pad_into_conv_pass.py, would let the same test check that the phase pads fold.
| f"_phase_{ay}_{ax}" | ||
| ) | ||
| with graph.inserting_before(weight_node): | ||
| phase_weight_node = create_constant_placeholder( |
There was a problem hiding this comment.
phase weights fall out of the external .ptd
create_constant_placeholder sets only meta["val"] on the new placeholder. So the phase weights lose the original weight's meta["custom"]["delegate_constant_tag"], and line 225 then deletes the tagged original. NodeVisitor uses that tag to route constants to the external file (operators/node_visitor.py:650). Without it, the phase weights silently land in the .pte. PropagateCustomMetaPass only handles q/dq nodes and runs before this pass, so nothing puts the tag back.
Repro: tag DwDeconv(4, 2, 1) with delegate_external_constants_pass_unlifted(gen_tag_fn=lambda _: "model") (same setup as test_propagate_custom_meta_pass.py), then lower and serialize.
- as is: 4 named-data entries in the
.pte(the phase weights) and 1 inmodel.ptd(the bias) - with the tag copied: 0 in the
.pteand 5 inmodel.ptd
Suggested fix: copy it right after creating each phase placeholder
if "custom" in weight_node.meta:
phase_weight_node.meta["custom"] = weight_node.meta["custom"]and add a test modeled on test_propagate_custom_meta_pass.py that asserts the .pte has no named data. FuseBatchNormPass has the same gap, so having create_constant_placeholder take a node to copy meta from would be the general fix, but that can wait for a follow-up.
Summary
Stacked on #24113 and #24111: their commits are included in this branch, and only the commit titled like this PR is new here. #24113 adds the
XNNDepthToSpace2dnode that this PR emits, and #24111 lets the pass delete the folded weight throughdelete_constant_placeholder.XNNPACK runs transposed convolutions through its subconvolution IGEMM path one group at a time. With one channel per group (a depthwise
ConvTranspose2d, the natural way to write a StyleGAN upfirdn x2 upsampling filter) that is far slower than the depthwise conv microkernels. On the MI-GAN inpainting model, the twelve 4x4 / stride-2 filters more than double the forward.DecomposeDepthwiseConvTransposePassrewrites a depthwise transposed conv (groups == in_channels == out_channels, weight lifted as a parameter, buffer or constant) intos*sstride-1 depthwise convs on the low-resolution input, a channel concatenation in(ay, ax)order, and a block-majordepth_to_space_2d. Phase(ay, ax)correlates the input with everys-th tap of the kernel, through a zero pad that can be asymmetric. The pass only fires when the node is a clean xsupscale on both axes,(H - 1)s - 2p + k + output_padding == sHwith the sames, dilation is 1, and every phase's taps fall inside the input. Anything else, including dequantized weights and grouped but not depthwise convs, stays on XNNPACK's own deconvolution node. Single-elementstride/padding/output_paddingare broadcast the same wayop_conv2d.pydoes, and dynamic spatial dims work.depth_to_space_2dis block-major: input channel(by * s + bx) * C + cgoes to output pixel(h * s + by, w * s + bx), channelc. That is the order XNNPACK's depth-to-space uses, and it matches the phase concatenation directly.aten.pixel_shuffleis channel-major and does not map onto the node. So this is a newet_xnnpack::depth_to_space_2dcustom op inbackends/xnnpack/custom_ops_lib.py, registered withtorch.librarythe same waybackends/vulkan/custom_ops_lib.pydoes it. The pass creates it only after partitioning. Its reference implementation is there for tracing and tests,op_depth_to_space.pyserializes it asXNNDepthToSpace2d, andChannelsLastTaggedReshapePasstags it NHWC.The pass runs right before
FuseActivationPass, so an activation after the transposed conv is never fused into the node the pass removes. It stays a separate node afterdepth_to_space_2d. Every convolution is planned before any is rewritten, so deconvs that feed each other work. The phase weights keep the original weight's input kind.The phase pads are
constant_pad_ndnodes on the conv input. With #24112 they fold into the new convs; without it they run as XNNPACK static pads.Device numbers for MI-GAN fp16 on a Galaxy S23 (Snapdragon 8 Gen 2), measured on an ExecuTorch 1.4.1-based branch with the equivalent of #24113 and #24112 applied. Setup:
executor_runner, 4 threads, mean of 10 forwards after 2 warm-ups, 3 interleaved rounds with 15 s cooldowns, round spread under 1%. "Stock deconv" is the depthwiseConvTranspose2dleft on XNNPACK's deconvolution node. "Hand-written" is the same filter written by hand as nearest upsample + checkerboard mask + pad + 4x4 depthwise conv.Outputs are bit-identical to the hand-written variant. The stock deconv differs by 1e-2 (fp16 accumulation order). The polyphase convs do a quarter of the filter MACs, and the upsample and mask passes go away.
Review order:
custom_ops_lib.pyandoperators/op_depth_to_space.py, then_passes/decompose_depthwise_conv_transpose.pywith its registration in_passes/__init__.pyand the NHWC tag inchannels_last_tagged_reshape_pass.py, then the BUCK targets and the tests.Test plan
Tested on macOS arm64, with pybindings built from #24113 (
cmake --preset pybind, targetportable_lib) so that the run-on-delegate stages use a runtime withXNNDepthToSpace2d. This PR has no C++ changes.New tests in
backends/xnnpack/test/passes/test_decompose_depthwise_conv_transpose.pycheck the node counts after the pass and compare against eager, both after the pass (referencedepth_to_space_2d) and on the XNNPACK delegate. They cover:conv_transpose2dbackends/xnnpack/test/quantizer/test_xnnpack_quantizer.pyrun on its own gives 31 passed and 1 failed.test_grufails the same way on the base branch ("quantized engine NoQEngine is not supported").lintrunnerreports no issues on the changed files.cc @nil-is-all @GregoryComer @digantdesai @cbilgin @JakeStevens