Skip to content

[CK] Repin CK submodule to fetchable develop commit (relu2/half_t fix) - #6178

Merged
valarLip merged 2 commits into
ROCm:mainfrom
shantipriya-amd:users/shantipriya-amd/ck/repin-relu2-math-fix
Oct 9, 2026
Merged

valarLip merged 2 commits into
ROCm:mainfrom
shantipriya-amd:users/shantipriya-amd/ck/repin-relu2-math-fix

Conversation

@shantipriya-amd

@shantipriya-amd shantipriya-amd commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Repins the 3rdparty/composable_kernel submodule to a valid, fetchable upstream commit, fixing the CI submodule-sync breakage that led to #6142 reverting #5708.

Technical Details

#5708 originally pinned 3rdparty/composable_kernel to 1f66d862231703a0f354e5771e58573139bb6efb — a commit that existed only in a local CK checkout, never pushed to any branch on ROCm/composable_kernel. This made the pin unfetchable and broke CI's submodule sync, so #6142 reverted it back to the last-known-good pin, af9e1d1f1ae347c22feeb08fd2d42645075e0c5d.

The two commits that were stacked on the bad pin (Add MoeRelu2 activation functor for gate-only MoE flatmm kernel and Fix half_t intrinsics (neg/sin/exp/log) via float round-trip for gfx950) have since been properly submitted and merged upstream via ROCm/rocm-libraries#13071, landing on CK's develop branch at commit 96afae1d54fc79c26857de7e1683b0dde1ff3c6f.

This PR repins 3rdparty/composable_kernel from af9e1d1f1ae347c22feeb08fd2d42645075e0c5d to 96afae1d54fc79c26857de7e1683b0dde1ff3c6f.

Note on scope (updated after review feedback): this repin advances the submodule to the current tip of CK's develop branch, not a hand-picked commit isolating only the two fixes above. Since develop is a shared branch, the pin advance necessarily carries 132 other commits that landed upstream between the old pin (af9e1d1f) and now — build/CI changes, gfx1250 enablement, FMHA performance tuning, dependency bumps, docs, etc. — none of which touch MoE/relu2/half_t code. This is standard/expected behavior for a submodule bump against a tracked branch (there is no way to cherry-pick just two commits onto the old base and still call it "develop"). No functional changes to aiter's own source are introduced by this PR itself — only the submodule pointer.

Test Plan

  • Confirmed the target commit (96afae1d54fc79c26857de7e1683b0dde1ff3c6f) is reachable via git fetch origin against ROCm/composable_kernel and resolves cleanly with git checkout.
  • Confirmed git submodule status reports a clean, non-dirty pin after the update (no +/- prefix indicating divergence).
  • Confirmed via git log --oneline af9e1d1f1ae347c22feeb08fd2d42645075e0c5d..96afae1d54fc79c26857de7e1683b0dde1ff3c6f that the 132 intervening commits are all unrelated upstream CK work (gfx1250 enablement, FMHA perf, build/CI, dependency bumps) with no overlap with MoE/relu2/half_t code paths.
  • No source-level testing needed for aiter itself in this PR, since it only changes the submodule pointer; the two upstreamed fixes' own functional correctness (relu2 CK-Tile kernel, half_t intrinsics fix) was already validated on real gfx950 hardware as part of [HIP] [CK] Add relu2 (Squared ReLU) activation support to CK-Tile fused MoE GEMM #5708's test plan and will be re-exercised end-to-end when the relu2 Tier 2 PR is resubmitted on top of this repin.
  • Regression coverage for the other 131 commits relies on this PR's own CI (aiter op tests on gfx942/gfx950) passing at the new pin, which is standard practice for submodule bumps of this kind rather than a commit-by-commit audit.

Test Result

git submodule status 3rdparty/composable_kernel after this change reports:

96afae1d54fc79c26857de7e1683b0dde1ff3c6f 3rdparty/composable_kernel (...)

with no dirty/divergence marker — the pin is clean and fetchable.

git log --oneline af9e1d1f1ae..96afae1d54f | wc -l → 132 commits, all confirmed unrelated to MoE/relu2/half_t.

Submission Checklist

@shantipriya-amd
shantipriya-amd requested review from a team and a balanced review from Copilot October 6, 2026 15:23
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests on MI35X (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:gfx1250-ffm-triton Run the five-shard gfx1250 FFM Triton test suite
ci:triton-300x Run an additional Triton test job on MI300X in PRs (added automatically when gfx942 configs change); main branch always runs both MI35X and MI300X
ci:triton-355 Run the full Triton test suite on MI35X, not only the tests the change affects
multigpu Aiter multi-GPU tests on the 8-GPU runner
ci:sglang SGLang integration tests: DeepSeek-R1-MXFP4 accuracy, Qwen 3.5 accuracy
ci:atom ATOM benchmark: DeepSeek-R1-0528, GPT-OSS-120B
ci:atom_full ATOM accuracy suite for PR and main models from ATOM models_accuracy.json
ci:vllm vLLM benchmark: GPT-OSS-120B, DeepSeek-R1-0528, Kimi-K2.5
ci:all All standard extended tests (excludes ci:atom_full)

Only add ci:atom_full for FlyDSL or Triton upgrades.
Add labels via the sidebar or gh pr edit 6178 --add-label <label>

One backend per PR:
A PR changes one kernel backend: [Triton/Gluon] (Triton and Gluon count as one), [HIP], [ASM], [CK], [OPUS] or [FlyDSL]. If the title ends up with two backend tags, split the PR -- as stacked pull requests when one part cannot merge without the other.

PR title tags & labels:
Component tags ([Triton/Gluon], [HIP], [CK], [ASM], ...) are added to the PR title and as PR labels automatically from the changed files and re-synced on every push — change-type tags like [fix]/[Perf], op tags like [MLA], and human labels (ci:*) are left untouched. Add the no-auto-title label to stop the title rewrites; labels stay in sync either way.

@github-actions github-actions Bot changed the title Repin CK submodule to fetchable develop commit (relu2/half_t fix) [CK] Repin CK submodule to fetchable develop commit (relu2/half_t fix) Oct 6, 2026
@github-actions github-actions Bot added the CK label Oct 6, 2026

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.

Copilot review overview

🔵 Needs a closer look

The broad CK upgrade requires expanded documentation and regression validation beyond the two stated fixes.

Review effort: Balanced
Findings: None

What changed in this PR

Repins CK to a fetchable upstream commit, but also incorporates 132 CK commits rather than only the two described fixes.

Changes:

  • Updates the CK submodule revision.
  • Restores upstream Relu2 and half_t fixes.
File Description
3rdparty/​composable_kernel Advances the CK submodule pin.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@shantipriya-amd

Copy link
Copy Markdown
Contributor Author

@yzhou103: Could one of you review this ?. It repins 3rdparty/composable_kernel from the last-known-good commit to 96afae1d54fc79c26857de7e1683b0dde1ff3c6f on CK's develop branch, now that the two orphaned commits (MoeRelu2 functor + gfx950 half_t intrinsics fix) have been properly upstreamed and merged via rocm-libraries#13071.

@zufayu
zufayu requested a review from valarLip October 7, 2026 01:48
@shantipriya-amd

Copy link
Copy Markdown
Contributor Author

@valarLip : Can you please review at your convenient time ?

Copilot AI balanced review requested due to automatic review settings October 8, 2026 19:29

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.

🔵 Needs a closer look

The pin incorporates 132 upstream commits, and the broad regression CI remains incomplete.

0 open findings

🧠 Review effort: Balanced

@valarLip
valarLip merged commit 29c3c15 into ROCm:main Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants