Skip to content

Revert "Revert "[AMD][ROCm] Keep cos_sin_cache fp32 on HIP for fused QSA indexer kernel (#41282)"" - #42558

Merged
BBuf merged 2 commits into
sgl-project:mainfrom
michaelzhang-ai:cursor/revert-42541-hip-cos-sin-fp32-5ace
Oct 5, 2026
Merged

BBuf merged 2 commits into
sgl-project:mainfrom
michaelzhang-ai:cursor/revert-42541-hip-cos-sin-fp32-5ace

Conversation

@michaelzhang-ai

@michaelzhang-ai michaelzhang-ai commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

#42541 reverted #41282 because the HIP sgl_kernel.rotary_embedding fallback could not read an fp32 cos_sin_cache. #42494 had already fixed that kernel to read an fp32 cache, about 12 hours before #42541 merged, and it moved test_rope_rocm.py into AMD stage A with a check that the HIP cache stays fp32.

With #41282 reverted, HIP casts the cache to the query dtype again, so that check fails stage-a-test-1-gpu-small-amd and every AMD stage B/C job is skipped:

File "/sglang-checkout/test/registered/rotary/test_rope_rocm.py", line 62, in _run_case
    self.assertEqual(rope_hip.cos_sin_cache.dtype, torch.float32)
AssertionError: torch.bfloat16 != torch.float32

#42541's own stage A run already failed this way (job). Since it merged, 18 of the 19 completed stage A jobs failed the same way, e.g. run 37272668001 and run 37275678823. The one pass is a stacked PR whose base branch does not contain #42541.

With #41282 and #42494 both on main, stage A passed 9/9, including Llama, EAGLE3 and test_rope_rocm.py: #42494's own run (job) and later PRs such as run 37254377497 and run 37265045192, the last one 12 minutes before #42541 merged.

Modifications

git revert 456d533e (#42541). base.py and mrope.py go back byte-for-byte to their state after #41282; the patch is identical to f6fcda8, and nothing else has touched either file since.

This brings back the fp32 cos_sin_cache on HIP that test_rope_rocm.py checks for, and with it the fused QSA indexer prep on HIP (_use_fused_prep requires an fp32 cache; benchmarks in #41282).

Checklist

  • Clean revert onto current main, no conflicts
  • CI: stage-a-test-1-gpu-small-amd passes

CI States

Latest PR Test (Base): ⏳ Run #37280502385
Latest PR Test (Extra): ❌ Run #37280518422
Latest PR Test (AMD ROCm 10): ❌ Run #37280502098

@michaelzhang-ai
michaelzhang-ai marked this pull request as ready for review October 5, 2026 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants