Skip to content

Upstream sync 17/N: merge 3d204dfdaa Revert "[Perf][ROCm] Dual-stream decode with hipgraphs" (#52024) (conflict) - #1264

Merged
roberteg16 merged 4 commits into
rogarcia.merge-upstream-112from
rogarcia.merge-upstream-113
Sep 10, 2026
Merged

roberteg16 merged 4 commits into
rogarcia.merge-upstream-112from
rogarcia.merge-upstream-113

Conversation

@roberteg16

@roberteg16 roberteg16 commented Sep 7, 2026

Copy link
Copy Markdown

Context

Seventeenth step of the batched upstream catch-up. Stacked on #<batch 16 PR>.

Conflict-only step.

Merged upstream changes:

  1. 3d204dfdaa Revert "[Perf][ROCm] Dual-stream decode with hipgraphs" vllm-project/vllm#52024 — Revert "[Perf][ROCm] Dual-stream decode with hipgraphs"

Upstream reverts, one day later, the very commit merged in batch 15. This PR
accepts the revert while keeping the fork's independent gfx11 optimization.

Audit

The question that mattered: is the revert a judgement on the fork's work? It is not, and the dates settle it.

date event
2026-06-26 fork enables shared-expert overlap by default on gfx11 (2e1ce506e5)
2026-08-12 upstream lands dual-stream decode (47ececb58e, batch 15)
2026-08-13 upstream reverts it (3d204dfdaa, this PR)

git merge-base --is-ancestor confirms the fork's commit is not a descendant of vllm-project#48223: the gfx11 work predates it by six weeks and is independent. Upstream is undoing its own 24-hour-old change.

Three hunks. The three-way picture, condensed:

ours:     [gfx11 _stream_token_threshold block]
          [DBO Event pairs]                          (from batch 15)
          @property _should_enable_stream_overlap_heuristic:  (from batch 15)
              ... if on_gfx11(): return True ...
          should_run = (is_cuda_alike() and ... <= self._stream_token_threshold
                        and self._should_enable_stream_overlap_heuristic)
theirs:   [gfx11 block absent — upstream never had it]
          [no Event pairs]                           (reverted)
          [no heuristic property]                    (reverted)
          should_run = (is_cuda() and ... <= envs.VLLM_SHARED_EXPERTS_...)
resolved: [gfx11 _stream_token_threshold block]      <- ours, the only part kept
          [no Event pairs]                           <- theirs
          [no heuristic property]                    <- theirs
          should_run = (is_cuda_alike()              <- OURS, see hunk 3
                        and ... <= self._stream_token_threshold)  <- ours

Hunk by hunk:

  1. __init__ — dropped upstream's DBO Event pairs (they arrived with [Perf][ROCm] Dual-stream decode with hipgraphs vllm-project/vllm#48223 and leave with the revert); kept the fork's _stream_token_threshold block.
  2. _should_enable_stream_overlap_heuristic — deleted entirely. The property was introduced by [Perf][ROCm] Dual-stream decode with hipgraphs vllm-project/vllm#48223, and the gfx11 early-return added to it in batch 15 existed only to stop upstream's dp_size > 1 gate from suppressing the fork's overlap. With the gate reverted the whole property is dead code; keeping the carve-out would leave a method nothing calls.
  3. _determine_shared_experts_order — the dangerous one, and it was not inside the conflict markers. The revert also narrows the platform predicate, and git applied it as a clean auto-merge:
  •        current_platform.is_cuda_alike() +            current_platform.is_cuda()
    

    is_cuda() is CUDA-only (platforms/interface.py:190), whereas
    is_cuda_alike() admits ROCM (line 222). Left as merged, the aux-stream
    overlap would never be selected on any ROCm device and the fork's gfx11
    optimization would be silently dead — no error, no conflict, and nothing
    to notice at the next merge. Restored is_cuda_alike(), which is what the
    fork has carried all along (verified against batch 14's copy), and kept the
    fork's self._stream_token_threshold.

Net effect verified exactly: the merged shared_experts.py is byte-identical to batch 14's — the state before vllm-project#48223 entered the stack — so batches 15 and 17 cancel out precisely, leaving the fork's independent work untouched. The diffstats corroborate it: batch 15 is +62/−53, batch 17 is +53/−62. moe_runner.py, the other file in the revert, merged cleanly.

py_compile, ruff check and ruff format pass.

Merge commit only — do not squash or rebase.

AI assistance was used to prepare this merge.

Test plan

  • Revert scope confirmed to be upstream's own work, via commit ancestry
  • is_cuda_alikeis_cuda narrowing caught outside the conflict markers and reverted; fork's ROCm path preserved
  • Merged file proven byte-identical to the pre-[Perf][ROCm] Dual-stream decode with hipgraphs vllm-project/vllm#48223 fork state (diff)
  • py_compile + ruff check + ruff format
  • Build + 6-model benchmark sweep vs the Strix Halo dashboard — no regression on transformers 5.15.0 (results)

simondanielsson and others added 4 commits August 13, 2026 04:38
…#52024)

Signed-off-by: simondanielsson <simon.danielsson99@hotmail.com>
Co-authored-by: Andreas Karatzas <akaratza@amd.com>
…code with hipgraphs" (vllm-project#52024) (conflict)

Conflict-only step. Upstream reverts, one day later, the very commit merged in
batch 15 (vllm-project#48223). Accepting the revert while KEEPING the fork's independent
gfx11 optimization.

The key question was whether upstream's revert is a judgement on the fork's
work. It is not, and the dates settle it:

  2026-06-26  fork enables shared-expert overlap by default on gfx11 (2e1ce50)
  2026-08-12  upstream lands dual-stream decode          (47ececb, batch 15)
  2026-08-13  upstream reverts it                        (3d204df, this batch)

merge-base confirms the fork's commit is NOT a descendant of upstream's
dual-stream commit: the gfx11 work predates it by six weeks and is independent.
Upstream is undoing its own 24-hour-old change, not the fork's.

Three hunks:

1. __init__ - dropped upstream's DBO Event pairs (they came in with vllm-project#48223 and
   go out with the revert), kept the fork's _stream_token_threshold block.

2. _should_enable_stream_overlap_heuristic - deleted entirely. This property was
   introduced by vllm-project#48223, and the gfx11 early-return added to it in batch 15
   existed ONLY to stop upstream's new dp_size>1 gate from suppressing the
   fork's overlap. With the gate reverted the whole property is dead code;
   keeping the fork's carve-out would have left a method nothing calls.

3. _determine_shared_experts_order - the dangerous one, and it was NOT inside
   the conflict markers. The revert also narrows the platform predicate:

     -            current_platform.is_cuda_alike()
     +            current_platform.is_cuda()

   git applied that silently as a clean auto-merge. is_cuda() is CUDA-only
   (platforms/interface.py:190: self._enum == PlatformEnum.CUDA), whereas
   is_cuda_alike() admits ROCM (line 222). Left as merged, the aux-stream
   overlap would never be selected on any ROCm device and the fork's gfx11
   optimization would be silently dead - no error, no conflict, and nothing to
   notice at the next merge. Restored is_cuda_alike(), which is what the fork
   has carried all along (verified against batch 14's copy of the file), and
   kept the fork's self._stream_token_threshold in place of upstream's direct
   envs read.

Verified the net effect is exact: the merged shared_experts.py is byte-identical
to batch 14's - the state before upstream's dual-stream commit entered the stack
- so batch 15 and batch 17 cancel out precisely, leaving the fork's independent
work untouched. moe_runner.py, the other file in the revert, merged cleanly.

py_compile, ruff check and ruff format pass.
@roberteg16
roberteg16 marked this pull request as ready for review September 8, 2026 08:10
@roberteg16
roberteg16 requested a review from eble-amd September 9, 2026 13:40
@eble-amd

eble-amd commented Sep 9, 2026

Copy link
Copy Markdown

Upstream reverts, one day later, the very commit merged in batch 15. This PR accepts the revert while keeping the fork's independent gfx11 optimization.

The agent probably made a poor decision here. Three consecutive PRs where one negates another and the third is conflict-free would probably have been handled more efficiently as one lump. If you're keeping a list of potential improvements to your agent instructions, consider this.

@roberteg16
roberteg16 added this pull request to stack #1288 September 10, 2026 05:59
@roberteg16
roberteg16 merged commit 17fcf0f into gfx11 Sep 10, 2026
7 of 8 checks passed
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