Skip to content

feat(rocmlir): fuse pointwise chains into rocMLIR elementwise kernels - #1294

Open
nvunnam57128 wants to merge 5 commits into
mainfrom
users/nvunnam/rocmlir-pointwise-fusion
Open

nvunnam57128 wants to merge 5 commits into
mainfrom
users/nvunnam/rocmlir-pointwise-fusion

Conversation

@nvunnam57128

Copy link
Copy Markdown
Contributor

No description provided.

FuseROCMlir gains FusePointwiseChain, which outlines a chain of pointwise ops (stepping through transpose, reshape, expand_shape and collapse_shape the way MIGraphX skips conv_dot_reshaper_names) into one rocMLIR elementwise kernel. A chain that reads a single-use conv/gemm/matmul is left to the anchor pattern (chainConsumesSingleUseAnchor), so conv epilogues stay fused into the conv kernel.

Quick autotune has no tuning space for elementwise kernels and aborts on them, so RocMlirAutotune compiles them with the default elem:v1:256,1,4,1,0 perf config instead of searching.

cmake/rocmlirTriton-elementwise-tuning-interface.patch makes ElementwiseParamsAttr implement the RockTuningParamAttrInterface accessors added after the elementwise-fusions series.

Requires a rocmlirTriton with elementwise kernel support; cmake/deps.txt is not bumped here.

dnt-v5-fp16-512x512 (gfx1151, quick autotune, profiling off): 8.154 -> 7.554 ms, 292 -> 245 launches.
Copilot AI balanced review requested due to automatic review settings October 22, 2026 18:00
@github-actions

Copy link
Copy Markdown

Thanks for opening a PR!

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:

  1. The change is focused. Substantial work links the relevant issue
    or design discussion.
  2. The PR documents relevant test results and updates affected
    documentation.
  3. 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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The dependency patch lacks prerequisites, and newly outlined kernels can fail lowering or default compilation.

6 open issues
What changed in this PR

This PR extends hip-ep’s rocMLIR fusion pass to compile standalone pointwise chains as GPU kernels while preserving convolution and matrix-operation epilogues.

Changes:

  • Adds pointwise-chain outlining and traversal through layout operations.
  • Selects a default elementwise configuration during autotuning and adds a dependency compatibility patch.
  • Adds fusion tests for standalone chains, transposes, and anchor ownership.
File Description
test/​lit/​Dialect/​hip-fuse-rocmlir.mlir Adds pointwise and mixed-chain fusion tests.
lib/​Dialect/​Transforms/​FuseROCMlir.cpp Shares outlining logic and adds standalone pointwise matching.
lib/​Compiler/​RocMlirAutotune.cpp Bypasses tuning-space enumeration for elementwise kernels.
cmake/​rocmlirTriton-elementwise-tuning-interface.patch Adds elementwise tuning-interface and load-builder compatibility changes.
cmake/​deps.cmake Registers the new dependency patch.

Review effort: Balanced

Comment thread cmake/deps.cmake
Comment thread lib/Dialect/Transforms/FuseROCMlir.cpp Outdated
Comment thread lib/Compiler/RocMlirAutotune.cpp Outdated
Comment thread lib/Dialect/Transforms/FuseROCMlir.cpp Outdated
Comment thread lib/Dialect/Transforms/FuseROCMlir.cpp Outdated
Comment thread lib/Dialect/Transforms/FuseROCMlir.cpp Outdated
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

L2 Accuracy Results (EP vs CPU)

Model Combined L2 Total Elems Skipped NaN/Inf
conv_test_hybrid 1.12256E-07 64 0
GroupQueryAttention_seq256 59.2561 2621440 0
MatMulNBits_o_seq128 259.905 368640 0
QMoE_seq128 24.2583 368640 0

Threshold: 0.01 | Run: 7859 - Commit: 04da3d3

@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Binary sizes

Baseline: run 39993879378 - d5979f7 (dll-sizes)

File This (MB) Main (MB) Delta (MB)
hipgpu.dll 59.27 59.26 +0.02
custom_kernels_gfx1170.dll 5.56 5.56 0.00
custom_kernels_gfx11-generic.dll 5.85 5.85 0.00

MorphiZen EP Performance Results

Model QPS Session (s) 1st Infer (ms) CPU% Mem (MB)
full_model_seq128 8.79 7.24 244 3 1159
GroupQueryAttention_seq128 11449.70 2.44658 1 5 231
matmul_down_seq128 525.20 3.22 129 3 274

EPContext Export Performance

Model QPS Session (s) 1st Infer (ms) CPU% Mem (MB)
full_model_seq128 8.83 49.21 243 3 15505

EPContext Import Performance

Model QPS Session (s) 1st Infer (ms) CPU% Mem (MB)
full_model_seq128 8.83 8.94 249 3 15639

OGA Benchmark Results

Model Warmup Reps Prompt Len Gen Tokens TTFT (ms) TPS Peak Mem (GB) GPU Mem (GB)
gpt-oss-20b-webgpu-int4-rtn-block-32 1 5 128 128 93.5 76.1 1.25 12.24
Llama-3.1-8B-awq-g128-int4-asym-fp16-onnx-dml 1 5 128 128 125.5 47.7 1.15 5.23

OGA Wheel Smoke (Python benchmark_e2e.py)

Model TTFT (ms) TPS
Llama-3.1-8B-awq-g128-int4-asym-fp16-onnx-dml 123 48.0

VLM Benchmark Results

Model TTFT (ms) TPS Output
Qwen3.8-27B-fp16-ve-fp16-int4-k_quant-gs128-text-dml-onnx 14395.7 14.0 The user wants a detailed description of the image. Let me carefully observe all elements before writing. Subject: A white do

Run: 7859 - Commit: 04da3d3

asparkhi
asparkhi previously approved these changes Oct 23, 2026
FuseROCMlir only outlines ops hip-to-tosa can convert. A chain step must
have static shapes (the layout ops it steps through too), no rank-0
result, and pass isTosaExpressiblePointwiseOp: element types rocMLIR
lowers (f16/bf16/f32, signless i1..i64) and the i1 rules hip-to-tosa
claims compare, logical and sign ops by. The chain stops before any other
op, so the prefix still compiles, and that op no longer keeps its
consumer from heading a chain. tensor.reshape is no longer stepped
through: hip-to-tosa and rocMLIR have no lowering for it.

compileRocMlirDefaultPerfConfig (the path with no compileOne, used by the
EP without autotune) now compiles elementwise kernels with the same
elem:v1:256,1,4,1,0 perf config the autotuner uses, shared as
kRocMlirElementwisePerfConfig.

Also applies clang-format to FuseROCMlir.cpp and RocMlirAutotune.cpp.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread lib/Conversion/HipToTosa/HipToTosa.cpp Outdated
isTosaExpressiblePointwiseOp returned true for every op past the element
type allow-list, so an i8/i16 hip.div, a floating-point hip.mod, or a
hip.mod with fmod = 1 was outlined into a rock kernel whose hip-to-tosa
conversion then failed. The predicate now mirrors each fusible op's
converter: Div and Mod reuse isTosaExpressibleDivType and
isTosaExpressibleModType plus Mod's fmod = 0 rule, the float-only unary
ops require a float that matches the input type, the binary ops and Where
require broadcastable operands (with an i1 condition for Where), Gelu its
approximate values, and Cast and Transpose a static input. Any op it does
not know returns false.

FuseROCMlir's head check and anchor check now step with the same helper
as the chain itself, so all three agree on where a chain ends.

The fusion lit test gets i16 div and fmod = 1 mod cases, and a second RUN
line that pipes every outlined kernel through --convert-hip-to-tosa.

Co-Authored-By: Cursor <cursoragent@cursor.com>
Made-with: Cursor

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Identity-cast compilation and indirect anchor fusion remain unresolved, alongside the acknowledged upstream dependency blocker.

3 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Trace indirect predecessors to preserve convolution epilogue anchors

lib/​Dialect/​Transforms/​FuseROCMlir.cpp:411

[P2] For conv -> exp -> add or conv -> transpose -> add with an independent cast -> add input, this guard sees only the exp/transpose predecessor and misses the anchor. If the cast is rewritten first, it claims the add and splits the convolution epilogue, potentially passing a full activation between kernels instead of a small cast result. Trace backward through eligible predecessors to the anchor, and add regressions for both indirect paths.

1 resolved since last review

Review effort: Balanced

Comment thread lib/Dialect/Transforms/FuseROCMlir.cpp
A chain made only of identity hip.casts and hip.transposes lowers to no
arith or math op, so rock::isElementwiseKernel does not recognise it and
compilation falls through to the tuning space it does not have.
FusePointwiseChain now leaves such a chain on the HIP path; an identity
cast in front of an op that computes still fuses.

chainJoinsAnchorChain (was chainConsumesSingleUseAnchor) traces back
through the chain ops between an anchor and each op of the head's chain,
so conv -> exp -> add and conv -> transpose -> add stay the conv's
epilogue whichever op the driver visits first.

Co-Authored-By: Cursor <cursoragent@cursor.com>
Made-with: Cursor
@nvunnam57128

Copy link
Copy Markdown
Contributor Author

Re the indirect conv epilogue (conv -> exp -> add and conv -> transpose -> add with a cast -> add input), fixed in 5846ea6. chainJoinsAnchorChain now asks, for each op of the head's chain, whether an anchor's chain reaches it, tracing back through the chain ops in between (isReachedByAnchorChain) instead of looking only for an anchor directly behind it. Both paths have lit cases: the cast is its own kernel, and the conv kernel takes the exp or transpose and the add.

With the current greedy order the conv is rewritten first, so the previous commit gave the same result on these cases; the change removes the dependence on that order.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Folded-away computations can still abort kernel compilation, and anchor-ownership checks introduce quadratic traversal.

3 open issues
2 resolved since last review

Review effort: Balanced

Comment thread lib/Dialect/Transforms/FuseROCMlir.cpp
Comment thread lib/Dialect/Transforms/FuseROCMlir.cpp
Lowering can fold a kernel's whole computation away -- hip.max(%x, %x)
becomes its input -- and createTunableParamSpace then aborts with
"Expected to find GEMM, convolution, ... or elementwise op". After the
high-level pipeline, compileAndEmbedRocMlirKernels now checks
hasRocMlirComputation (a gemm-like op or an elementwise kernel), and for a
kernel with neither puts the hip ops it was outlined from back in place of
its dispatches, from a copy taken before the hip->tosa conversion.
compileRocMlirDefaultPerfConfig returns false on such a kernel instead of
aborting.

chainJoinsAnchorChain walks back once from the last op of the chain, with
a worklist, instead of from every op, which was quadratic in chain length.

Co-Authored-By: Cursor <cursoragent@cursor.com>
Made-with: Cursor

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The author-confirmed rocmlirTriton prerequisite and compatible dependency-pin update remain outstanding, blocking clean configuration with rocMLIR enabled.

2 open issues
1 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The acknowledged rocmlirTriton dependency blocker still prevents clean configuration until a compatible revision is pinned.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants