[ROCm] Remove stale SDPA and skinny GEMM workarounds - #50907
Merged
Merged
Conversation
Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
Contributor
|
Documentation preview: https://vllm--50907.org.readthedocs.build/en/50907/ |
mergify
Bot
added
documentation
Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
Member
Author
|
/ci run |
|
✅ Triggered Buildkite CI #82434 for commit |
Member
Author
|
Testing changes: |
Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
…issue # Conflicts: # vllm/v1/core/kv_cache_utils.py Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
AndreasKaratzas
commented
Aug 6, 2026
Comment on lines
6330
to
+6559
| @@ -6522,16 +6537,26 @@ def _init_minimal_kv_cache_for_profiling(self) -> None: | |||
| @staticmethod | |||
| @contextmanager | |||
| def _freeze_gc(): | |||
| gc_was_enabled = gc.isenabled() | |||
| gc.collect() | |||
| should_freeze = not envs.VLLM_ENABLE_CUDAGRAPH_GC | |||
| if should_freeze: | |||
| gc.freeze() | |||
| # A Triton kernel finalized during stream capture unloads its | |||
| # module and invalidates the captured graph. | |||
| gc.disable() | |||
| try: | |||
| yield | |||
| finally: | |||
| if should_freeze: | |||
| gc.unfreeze() | |||
| gc.collect() | |||
| try: | |||
| gc.unfreeze() | |||
| gc.collect() | |||
| finally: | |||
| if gc_was_enabled: | |||
| gc.enable() | |||
| else: | |||
| gc.disable() | |||
Member
Author
There was a problem hiding this comment.
This patch addresses a latent, timing-sensitive initialization bug. The key difference is how Triton specializes the rejection-sampling kernel:
| Phase | is_greedy value |
Triton specialization |
|---|---|---|
| Previous dummy warmup | Tensor/pointer | Mixed sampling |
| Real suffix inference | None |
All-greedy sampling |
Previously, initialization only compiled the mixed-sampling specialization. The first real all-greedy request therefore triggered JIT compilation during inference. Shortly afterward, ROCm reported a null-address GPU memory fault. The Python stack at .cpu().numpy() was only where the asynchronous GPU error became visible.
To resolve this, we:
- Warm both mixed and all-greedy kernel specializations during initialization.
- Synchronize afterward, so any GPU failure surfaces immediately during startup.
- Disable cyclic GC during graph capture, then restore its previous state.
…issue Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com> # Conflicts: # vllm/v1/core/kv_cache_utils.py
AndreasKaratzas
added
the
ready
Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
Signed-off-by: Andreas Karatzas <Andreas.Karatzas@amd.com>
|
✅ Queued 5 failed job(s) for retry in Buildkite CI #83067. |
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Preserve default ROCm SDPA selection while retaining the incoming Transformers multimodal hardening. Assisted-by: OpenAI Codex Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Member
Author
|
/ci run |
|
✅ Triggered Buildkite CI #83188 for commit |
Member
Author
|
/ci retry |
|
✅ Queued 4 failed job(s) for retry in Buildkite CI #83188. |
Assisted-by: OpenAI Codex Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Leave explicit checkpoint layout handling to #50727. Assisted-by: OpenAI Codex Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Member
Author
|
/ci run |
|
✅ Triggered Buildkite CI #83310 for commit |
Member
Author
|
Full build again successful here (none of the modified test groups are failing): |
Member
|
/ci retry |
|
✅ Queued 2 failed job(s) for retry in Buildkite CI #83310. |
mgoin
approved these changes
Aug 11, 2026
github-project-automation
Bot
moved this from To Triage
to Ready
in gpt-oss Issues & Enhancements
github-project-automation
Bot
moved this from Ready
to Done
in gpt-oss Issues & Enhancements
JohnQinAMD
added a commit
to JohnQinAMD/vllm-amd
that referenced
this pull request
Aug 18, 2026
wvSplitKQ stages the activation by walking stride(0) for N rows while its compute loop stops at the valid width K, so a strided activation is read (stride(0) - K) elements past its extent per row. The over-read never reaches the math, so results stay correct and it only surfaces when it crosses an unmapped page -- an allocator-dependent fault far from its cause. PR vllm-project#50907 fixed this for the call sites in layers/utils.py. scaled_mm/rocm.py was not covered, and there is a reachable path to it that runs entirely on is_contiguous() reporting True for a single-row view: QuantFP8.forward_hip gates the aiter path on that flag, rocm_aiter_ops.per_tensor_quant allocates with torch.empty_like which preserves the row stride, and ScaledMMLinearKernel's view(-1, shape[-1]) is a no-op on an already-2D tensor. At 2 and 4 rows the flag is False and the output comes back dense, so the exposure is single-row decode -- which is what this dispatch is for. Densify there with an explicit stride test rather than .contiguous(), which is a no-op on a single-row view for the same reason. State the contract in the kernel wrapper as well, so a future call site fails at the boundary instead of over-reading. Reproduced on MI355X: an 8 MB over-read faults under PYTORCH_NO_CUDA_MEMORY_CACHING=1; over-reads of 8 KB, 24 KB and 504 KB survive and return correct results, which is why no allclose test caught it. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Yanyuan Qin <yanyuan.qin@amd.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This addresses #37736, where stale ROCm Math SDPA forcing made the 32-image Gemma3 profiling forward OOM on MI250. With default SDPA selection, the same profile completes while preserving model behavior. Removing blanket skinny GEMM disables exposed a non-contiguous activation bug in the ROCm GEMM dispatcher, which is fixed by normalizing supported inputs and falling back for unsupported operand layouts. Broad multimodal and targeted ROCm validation passed, with unrelated model-access and optional-dependency failures classified separately.