You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(rocmlir): decompose ConvTranspose into one DepthToSpace conv - #1295
DecomposeConvTranspose now builds the residue filters once and, when every residue shares a kernel shape and padding, stacks them on the output-channel axis into a single stride-1 hip.conv. Its result is viewed as [N, S0, S1, M, Ht0, Ht1] and transposed to [N, M, Ht0, S0, Ht1, S1] (DepthToSpace, MIGraphX reassemble_interleave), and hip-fuse-rocmlir folds that transpose into the conv's store. Residues with different tap counts keep one strided insert_slice each.
New hip-simplify-algebra pass (SimplifyAlgebra.cpp) with the MIGraphX find_conv_dot_horiz_fusion matcher: sibling hip.convs of one activation with the same operator, weight shape and output shape merge into one conv plus channel extract_slices. It runs between hip-decompose-conv-transpose and hip-fuse-rocmlir in CompilerDriver, buildRocMlirPipeline and hip-rocmlir-compiler.
On dnt-v5-fp16-512x512 (gfx1151, quick autotune) each upsample level is one rocMLIR kernel (MIGraphX: conv + concat_kernel + layout_kernel). Against the previous form (merged residue conv, concat copies, separate transpose) the five levels go from 1.501 to 1.007 ms and the model from 7.085 to 6.622 ms with profiling off. Output matches the CPU EP.
Depends on the FuseROCMlir reshaper traversal from the pointwise-fusion change.
This project follows LLVM's incremental-development and AI-tool-use
guidance. See CONTRIBUTING.md
for the project workflow.
Before requesting review, please check that:
The change is focused. Substantial work links the relevant issue
or design discussion.
The PR documents relevant test results and updates affected
documentation.
If AI tools provided substantial assistance, the description
explains what was assisted and how it was validated, and commit
trailers identify the tool. The contributor has reviewed and
understands the result.
Reviewers are assigned through CODEOWNERS where ownership
is configured.
This PR optimizes ConvTranspose lowering for rocMLIR by combining equal-shaped residues into one convolution followed by a DepthToSpace-style interleave.
Changes:
Replace equal-residue scatter operations with a wide convolution and reshape/transpose sequence.
Add horizontal convolution merging before rocMLIR fusion.
Extend LIT coverage for decomposition, interleaving, and sibling merging.
Place the merged conv and its slices before the first sibling, so a use
of an earlier sibling that sits before a later one still sees its slice.
Group siblings by block as well as input, so convolutions in different
regions, such as the two branches of an scf.if, are not merged. Erase a
replaced conv's weight or init only when it is an unused inline constant
or tensor.empty, so block-argument inits are left alone. A MapVector
keeps the merge order deterministic.
Co-Authored-By: Cursor <cursoragent@cursor.com>
Made-with: Cursor
The equal-residue path lacks a value-sensitive regression combining multiple input/output channels with distinct spatial taps. A residue-major versus channel-major packing error could scramble outputs without failing the current checks. Add a regression that checks the packed wide-filter values and interleave layout together, including channel swapping and spatial reversal.
Reject integer weights before APFloat access
lib/Dialect/Transforms/SimplifyAlgebra.cpp:28
hip.conv permits integer tensor types, so two matching siblings with inline i32 filters pass this matcher. mergeHorizontal then calls getValues<APFloat>() on their integer attributes, which asserts instead of declining the unsupported rewrite. Reject non-floating-point weights here, or concatenate the elements without assuming a floating-point type.
Avoid accessing deleted shared constant during cleanup
lib/Dialect/Transforms/SimplifyAlgebra.cpp:126
If a sibling uses the same inline constant for both its weights and its outs operand (for example, a 1x1 convolution with both tensors shaped 1x1x1x1), erasing the last sibling makes that constant unused. This branch erases it, but the following init.getDefiningOp<tensor::EmptyOp>() then dereferences the same deleted value, causing a use-after-free. Clean up the empty init before deleting the weight constant so neither cached value is inspected after its producer is erased.
The merge concatenates filters as APFloat, so integer filters now decline
the rewrite instead of asserting. A conv whose filter constant is also its
init no longer reads the init through the erased constant: the init is
checked first. Also pin the exact wide filter of a multi-channel
equal-residue ConvTranspose, whose packing no test covered by value.
Co-Authored-By: Cursor <cursoragent@cursor.com>
Made-with: Cursor
Addressed the three previously missed issues from the latest Copilot review in 94f01ce:
Integer weights before APFloat access (SimplifyAlgebra.cpp): inlineWeight now rejects filters without a floating-point element type, so i32 siblings are left unmerged. Added @integer_weights to test_simplify_algebra_horiz.mlir: the previous code aborts on it.
Deleted shared constant during cleanup (SimplifyAlgebra.cpp): the tensor.empty init is now checked before the weight constant is erased, so a constant that is both a conv's weight and its init is never inspected after it is deleted. Added @weight_as_init: the previous code segfaults on it.
Value-sensitive regression for equal-residue packing (DecomposeConvTranspose.cpp): added case 15, @equal_residue_values, to test_decompose_conv_transpose.mlir. It uses 2 input and 3 output channels, a 4x2 kernel at stride 2 (four residues of 2x1 taps), and distinct weights. It pins the full wide filter, the conv pads, the expand, the [0, 3, 4, 1, 5, 2] transpose and the collapse. The packing itself was already correct: a numpy emulation of the emitted IR matches a direct ConvTranspose, and the CHECKs fail on a channel-major packing and on missing tap reversal.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The interaction between horizontal merging and fused interleave stores needs human review with end-to-end rocMLIR correctness and performance evidence.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Avoid merging ConvTranspose ops with unfused interleave chains
lib/Dialect/Transforms/SimplifyAlgebra.cpp:174
When two matching ConvTranspose ops share an activation, this merge replaces their wide-convolution results with channel slices and prevents their interleave transposes from fusing into the convolution stores. FuseROCMlir.cpp:126–136 requires a single-use chain and does not traverse tensor.extract_slice, so those transposes require separate materialization. Exclude candidates with these interleave chains from horizontal merging, or extend fusion to preserve the chains.
Fuse-rocmlir folds the DepthToSpace interleave after a decomposed
ConvTranspose into the conv's stores only while the conv has that single
use. Merging two such convs on one activation replaced each with a
channel slice, leaving each interleave as a standalone transpose. A conv
whose only use reaches a hip.transpose through reshapes is no longer a
merge candidate.
Co-Authored-By: Cursor <cursoragent@cursor.com>
Made-with: Cursor
Addressed the previously missed issue from the latest Copilot review in b77505b:
Avoid merging ConvTranspose ops with unfused interleave chains (SimplifyAlgebra.cpp): confirmed. With two ConvTransposes on one activation, the merge left one conv kernel plus two standalone hip.transpose ops. A conv whose only use reaches a hip.transpose through reshapes is now not a merge candidate, so each conv keeps its interleave fused into its stores. Added a second case to test_conv_transpose_algebra.mlir, run through decompose, simplify-algebra and fuse-rocmlir: it checks two kernels, each with its transpose, and no transpose left in main_graph. The previous code fails it.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The new fused convolution/interleave path still needs end-to-end rocMLIR compilation and GPU correctness validation before human approval.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Update pipeline guide for hip-simplify-algebra and ConvTranspose IR
lib/Dialect/Transforms/Pipelines.cpp:528
Please update docs/pipeline_pass_menu.md:169–183 alongside this pipeline change. The guide still lists the registered pipeline without hip-simplify-algebra and describes ConvTranspose reassembly exclusively as per-residue convolutions with strided inserts, so its documented pipeline and expected IR no longer match the implementation.
The pipeline guide now lists hip-simplify-algebra in the rocMLIR pipeline
and describes the stacked convolution and DepthToSpace interleave that
hip-decompose-conv-transpose emits for equal residues. The pass
description in Passes.td gains the same-block, floating-point filter and
fused-transpose restrictions.
Co-Authored-By: Cursor <cursoragent@cursor.com>
Made-with: Cursor
Addressed the previously missed issue from the latest Copilot review in 43e0bcf:
Update pipeline guide for hip-simplify-algebra and ConvTranspose IR (docs/pipeline_pass_menu.md): the registered pipeline block and the ordering note now include hip-simplify-algebra, which has its own row in the rocMLIR pass table. The hip-decompose-conv-transpose row now describes the stacked convolution with the DepthToSpace interleave for equal residues, next to the per-residue strided tensor.insert_slice form. The hip-fuse-rocmlir row notes that the interleave transpose is folded into the kernel's store. The hip-simplify-algebra description in Passes.td now also states the same-block, floating-point filter and fused-transpose restrictions.
This branch has not been deployed
No deployments
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
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.
No description provided.