Skip to content

[CI][ROCm] Record native kernel coverage for AMD jobs - #685

Merged
khluu merged 12 commits into
mainfrom
akaratza_rocm_ci_selector
Oct 24, 2026
Merged

khluu merged 12 commits into
mainfrom
akaratza_rocm_ci_selector

Conversation

@AndreasKaratzas

@AndreasKaratzas AndreasKaratzas commented Sep 28, 2026 •

Copy link
Copy Markdown
Member
  • Add a ROCProfiler SDK 1.3.2 recorder for dispatched AMD kernels, including graph replay.
  • Require clean native traces with no drops, recorder errors, or unresolved names before a row can authorize skipping a test.
  • Add matching error accounting to the CUPTI recorder and rebuild its shipped CUDA library.
  • Set up recording for eligible AMD jobs with SDK build, version, and loader checks that preserve workload execution when collection is unavailable.
  • Publish and fetch CUDA and ROCm tables and source maps independently, with separate latest pointers.
  • Read both backends in selection, PR reporting, and validation, preserving observations and unusable matching rows when another pair suggests dropping a test.
  • Arm AMD jobs through the pipeline generator, collect their artifacts, and request matching source maps from the image builds.
  • Record AITER-only imports and editable AITER packages in the Python recorder, together with relevant ROCm execution settings.

The selector already uses CUPTI launch records and image-built source maps to determine which tests exercise a CUDA kernel change. AMD CI needs equivalent coverage for HIP, AITER, Triton, RCCL, and graph workloads. Finalized rocprofv3 JSON does not expose enough loss accounting to make missing kernel names safe evidence for skipping tests. The recorder therefore uses the pinned native SDK with lossless buffers and explicit completion checks, and the CUDA recorder gains matching error accounting. Records stay separate by backend while the selector combines their observations conservatively.

Companion: vLLM source maps and AMD Docker artifact forwarding. Deploy the consumers before enabling producers, then refresh the recorded coverage.

AI assistance was used to implement these changes and run the following checks.

Collect native ROCProfiler SDK dispatches, consume CUDA and ROCm evidence independently, and apply the same completion gates to both backends.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Andreas Karatzas <andreas.karatzas@protonmail.com>
Preserve AMD shell command boundaries and exclude transitive manual-block dependencies from collectors. Default supported ROCm vLLM workers to spawn with enough time to flush traces.

Keep the existing fnrec import-trigger name and trim redundant documentation. Add regression coverage for generated scripts, optional jobs, and setup defaults.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Andreas Karatzas <andreas.karatzas@protonmail.com>
@AndreasKaratzas
AndreasKaratzas marked this pull request as ready for review September 30, 2026 01:45
@AndreasKaratzas

Copy link
Copy Markdown
Member Author

AndreasKaratzas and others added 2 commits October 1, 2026 00:14
Preserve the ROCm recorder and native trace completeness gates while
incorporating main's library-call recording, tests/ coverage, selector
fixes, and removal of declared-dependency gating.

Keep all recorder tests from both parents and verify watched-library calls
when AITER starts recording before vLLM. Adapt the incoming library test
to the selector's multiple-kernel-record interface.

Protect positive CUDA and ROCm observations from the incoming library
narrowing pass, including steps already selected by the code map. Cover
both backend orders and retain narrowing for a quiet step.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: Andreas Karatzas <andreas.karatzas@protonmail.com>
Keep the ROCm kernel record fetch alongside main's rule that a record which
cannot be fetched is removed rather than reused: each fetch now names the
files it owns, so a failed CUDA fetch drops only the CUDA pair and a failed
ROCm fetch only the ROCm pair.

Keep the recorder and PR comment tests from both parents. The ROCm refresh
test now runs against a temporary coverage directory, since a failed fetch
deletes files there, and checks that the ROCm pair survives a CUDA failure.

Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Andreas Karatzas <andreas.karatzas@protonmail.com>
@AndreasKaratzas

Copy link
Copy Markdown
Member Author

Another run complete: https://buildkite.com/vllm/amd-ci/builds/14676/list

AndreasKaratzas and others added 2 commits October 15, 2026 21:45
Conflicts in buildkite/pipeline_generator/buildkite_step.py — AMD mirror
model_copy and _create_amd_step call. Resolutions:

1. model_copy update dict: keep both `dind` (from main/#790) and `num_nodes`
   (from this PR), which are orthogonal fields.

2. _create_amd_step display_label: take `amd_label` from main (correct —
   carries the sharding "shard %N/%t" suffix when applicable).

3. _create_amd_step extra_artifact_paths: take the kernrec extension from
   this PR (`+ [KERNREC_ARTIFACT_PATH] if _kernrec_applies(...) else []`).

Test fix: `test_generator_shards_a_flagged_native_mirror` expected the
raw command string in VLLM_TEST_COMMANDS, but this PR's _prepare_commands
wraps each AMD command in `{ cmd }` (to preserve exports/cd across &&).
Update the test to use the wrapped form when constructing the expected
guard() call and replace pattern. 329/329 tests pass.

Co-Authored-By: Claude <noreply@anthropic.com>

Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
Conflicts in buildkite/pipeline_generator/buildkite_step.py and
buildkite/tests/pipeline_generator/test_kernrec.py.

buildkite_step.py: runnable-step computation for collect groups.
  PR 685 had a manual block-traversal (follow dependency edges to find
  all steps gated behind manual block steps). Main simplified this to
  `[s for g in groups for s in g.steps if not s.deferred]`, using the
  new `deferred` field set by convert_group_step_to_buildkite_step.
  Resolution: take main's deferred approach.

test_kernrec.py: keep PR 685's two new AMD tests
  (test_amd_uses_rocm_setup_artifacts_and_finish,
   test_amd_multinode_mirrors_stay_unarmed_without_affecting_cuda);
  take main's renamed test_collect_group_skips_deferred_steps (was
  test_collect_group_skips_steps_behind_a_block).

test_collect_dependencies.py (PR 685, no conflict but needed updating):
  The two tests used BuildkiteBlockStep (removed in main) and asserted
  that block step keys appear in depends_on. Updated to use the deferred
  field: optional steps that cannot run are now deferred=True rather
  than having a block step inserted. 342/342 tests pass.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
@AndreasKaratzas
AndreasKaratzas force-pushed the akaratza_rocm_ci_selector branch from 21ef3c6 to e995d11 Compare October 16, 2026 02:18
@AndreasKaratzas

Copy link
Copy Markdown
Member Author

AndreasKaratzas and others added 2 commits October 19, 2026 01:09
A ROCm 7.2 run of :amd: (MI250) V1 Engine produced 69 recordings of
which 36 made the job incomplete, so its coverage could never be used:

- 32 were fork_after_sdk_init markers from py-cpuinfo and Ray startup.
  A fork child now writes gpu_use_after_fork only when it loads GPU
  code, the only way it can launch kernels the SDK cannot record;
  exec'd and CPU-only children leave no file, and a fork+exec child no
  longer collides with its own recording.
- 4 were Ray daemons killed without a footer. The recorder marks the
  first code-object load with # gpu_active and advertises it with
  activity=1; a ROCm process that never went active cannot have lost
  dispatches, so the collector accepts it without a footer.

Also:
- Accept ROCProfiler SDK 1.3.5 (ROCm 10, now the default image) next to
  1.3.2, and record the version found instead of assuming 1.3.2.
- List the job's recordings in the sidecar at finish, so a partially
  downloaded job no longer folds as complete.
- A headerless file no longer decides the job's backend.
- KERNREC_DISABLE=1 leaves a job unarmed when the host cannot upload.

Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
Conflict in test_kernrec.py: main's #832 makes coverage collect steps
wait for the recording steps in their command (CI_SELECTOR_COLLECT_WAIT_FOR,
up to a deadline) and depend only on what those steps wait for, such as
the image build. Kept that, and kept this branch's check that the collector
waits for both the CUDA and the AMD recording step, now through the wait
list. The AMD multi-node and deferred-step tests read the wait list the
same way; the code merged cleanly and needed no change.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>

@tahsintunan tahsintunan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good overall. I made a few inline comments.

Comment on lines +474 to 482
def _kernrec_applies(step: Step, setup_profile: SetupProfile = "nvidia") -> bool:
"""Whether the step can load a backend-specific kernel recorder."""
return (
kernrec_enabled()
and not step.no_plugin
and not step.label.startswith(":docker:")
and step.key not in KERNREC_SKIP_STEPS
and not (setup_profile == "amd" and step.num_nodes and step.num_nodes > 1)
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

VLLM_CI_KERNREC=1 now turns on ROCm recording too, so we can't run CUDA recording alone. It also makes the collector wait on AMD jobs and image-build-amd, so a slow or cancelled AMD build holds up the CUDA table. We could maybe give AMD its own switch.

@AndreasKaratzas AndreasKaratzas Oct 23, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added VLLM_CI_KERNREC_AMD=1, which arms the ROCm recorder on AMD steps and the symbol map on image-build-amd. VLLM_CI_KERNREC=1 is CUDA-only again, as on main, and the collector only waits on and depends on what is actually armed. With just the CUDA switch on, it no longer touches image-build-amd or AMD jobs.

Slow AMD jobs weren't really holding it up. The wait stops at the 180-minute deadline, and in the dailies AMD finishes long before NVIDIA: at most 87 and 83 min on #96247 and #96139, against NVIDIA tails of 1445 and 269 min.

With both switches on, a cancelled image-build-amd can still fail the shared collector, the same way fnrec-collect can today.

Comment on lines +136 to +147
# Loading GPU code precedes every dispatch, so a process that never went
# active lost nothing by exiting without a footer (e.g. a killed daemon).
idle = (
valid_header
and reports_activity
and headers == 1
and footers == 0
and not active
and not names
and not dropped
)
return names, dropped, (clean or idle) and not errors

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Question: if a write fails right after the header (e.g. disk full), could # gpu_active be lost too, making an active process look idle?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, that could happen. After the first failed write the recorder closed the fd, so a file with just the header and the sdk_version line passed the idle check. The parser can't tell that apart from a real idle process, so I fixed it in the recorder: on a write failure, kernrec_rocm.cpp now truncates the recording to 0 bytes before closing it. An empty file is never clean or idle. I added a check to test_rocm_sdk.cpp that caps the file size with RLIMIT_FSIZE right after the header. It fails on the old code and passes now. CUDA has no idle rule and counts stdio errors in the footer, so it isn't affected.

Comment on lines +122 to +131
clean = (
valid_header
and footers == 1
and not errors
and total >= 0
and int(fields["errors"]) == 0
and int(fields["unresolved"]) == 0
and int(fields["unique"]) == len(names)
and int(fields["records"]) >= len(names)
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The footer rule makes sense in principle, but it's costly here: in 96247 (current CUDA table), 186 of 234 recorded steps have a job with a footerless process, so they'd become unusable. I compared step kernel sets across 96139 and 96247 and the footerless steps are as stable as the rest (29/30 identical on vLLM kernels), so they don't look lossy in practice. Could CUDA get a softer rule, or make worker exits write the footer?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for measuring this. I get the same cost: 4,400 of 18,118 recordings in 96247 have no footer, across 186 of 234 steps. I'd still keep the rule for CUDA, because the same two builds don't show footerless steps as lossless. On vLLM symbol-map kernels, skipping steps with a failed job, all-footer steps were 38/38 identical and footerless steps 156/172, and most of the differing vLLM names show up only in footerless processes.

The main cause is that EngineCore gets SIGKILLed right after SIGTERM: with shutdown_timeout=0 the process manager waits 0 s on CUDA, and the log shows "force killing remaining process EngineCore". No exit hook can run there, and an idle-style exception would only save 12 of those 186 steps. So I'd go with your second option, but in vLLM, as a follow-up: give EngineCore a shutdown grace period on recording runs, like the existing ROCm 15 s one, so these processes write real footers.

Also note 96247 used main's recorder with old-style footers, so it has no usable rows under this PR either way.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Follow-up is up: vllm-project/vllm#63260 lets an explicitly set VLLM_WORKER_SHUTDOWN_TIMEOUT_SECONDS give EngineCore a cleanup grace after the immediate abort, on every platform (defaults unchanged). This PR now exports =30 on CUDA recording runs too (f369133), as it already did on ROCm. On MI300 with CUDA's 0 s behaviour simulated, EngineCore footers go from 0/10 to 10/10. It covers the ~931 EngineCore/worker recordings killed on that path in 96247, not the other footerless causes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, nice catch. Fixing root cause makes more sense

n_jobs=$(find .kernrec -mindepth 1 -maxdepth 1 -type d 2>/dev/null | wc -l | tr -d ' ')
echo "${n_files} recording files from ${n_jobs} jobs"
if [[ "${n_files}" == "0" ]]; then
if [[ "${n_jobs}" == "0" ]]; then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor: this can publish a table the selector can't use (e.g. all rows unusable after a setup failure, or one that fails the loader's checks) over the last good one. I think we can skip publishing in those cases.

@AndreasKaratzas AndreasKaratzas Oct 23, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Right. The gate only checked that the table had rows, so a build where every job failed would still publish and move latest.json. collect.sh now needs at least one row that passes kernel_table.usable, plus a map version the loader accepts. Otherwise that backend isn't published.

New test case unusable-rerun: a good pair is already published, then a rerun where every job failed. It fails on the old collect.sh and passes now. I didn't pull the full selector loader into collect.sh, because it imports the rules/pipeline_generator package. ci-fetch-kernel-record still runs those loaders before it replaces anything.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: could we pin the ziglang and CUPTI versions in cross-build.sh so this .so is reproducible?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done. cross-build.sh now pins ziglang 0.14.1, CUPTI 13.0.85, cuda-runtime 13.0.96 and cuda-crt 13.0.88-1, and each can be overridden from the environment. With the defaults it rebuilds the committed libkernrec.so byte for byte (sha256 82b3ca62...). Zig was the version that mattered: the .so was built with clang 19.1.7 from ziglang 0.14.1, and an unpinned run today pulls 0.17.0 and gives different bytes. The 13.0.x CUPTI/runtime/crt patch versions don't change the output.

…ixes

- Add VLLM_CI_KERNREC_AMD for the ROCm recorder; VLLM_CI_KERNREC is
  CUDA-only again, so either backend can be recorded alone and the
  collector only waits on what is armed.
- kernrec_rocm: truncate the recording on a failed write, so a file cut
  off after the header can't pass as an idle process.
- collect.sh: publish a backend only if its table has a usable row and
  its map version is one the loader accepts.
- cross-build.sh: pin ziglang, CUPTI, cuda-runtime and cuda-crt; the
  defaults rebuild the committed libkernrec.so byte for byte.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Export VLLM_WORKER_SHUTDOWN_TIMEOUT_SECONDS=30 on CUDA recording runs, as
ROCm already does, so EngineCore exits normally and writes its recording
footer instead of being SIGKILLed right after SIGTERM. Takes effect with
the companion vLLM change that lets this variable set the EngineCore
cleanup grace.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andreas Karatzas <akaratza@amd.com>
AndreasKaratzas and others added 2 commits October 23, 2026 06:06
vllm/ci #96785 armed AMD kernel recording and timed out
e2e-scheduling-accuracy (37 min on main, cut at its 55 min limit) and
minimax-m3-monokernel (all tests passed, cut at 20 min). The recording
margin only reached NVIDIA steps; native AMD steps and AMD mirrors kept
their plain limits.

The ROCProfiler SDK intercepts every kernel dispatch: launch-bound jobs ran
up to 2.4x their main time in that build (v1-spec-decode 16.5 -> 39.8 min),
while the Python recorder alone adds about 3% on AMD. Measured on MI250,
callback tracing at enqueue costs the same as buffer tracing at completion,
so the cost is the interception itself, not a choice the recorder can avoid.

A recorded step now gets the larger of its platform factor (NVIDIA 1.25,
AMD 2.0) and 20 extra minutes on top of its limit, AMD after its three-hour
cap; native AMD steps and both mirror paths apply it. Unrecorded builds keep
their limits.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
Conflicts with main's #823 (a header or bindings change narrows to the
kernels it declares), resolved keeping both sides:
- coverage/kernels.py: SymbolMap keeps this branch's `backend` and gains
  main's `sources`; _why_keep takes both `backend` and `others_cleared`.
- decide.py: kernel evidence is fetched where main now fetches it, before
  the Python record, but as this branch's list of CUDA/ROCm pairs. Main's
  _deferred_to_kernels defers a changed csrc file when any usable pair's map
  compiled it. Each pair's read_pr gets main's python_cleared and its own
  declaration attribution.
- tests/coverage/test_kernels.py: both sides' new tests. The combined-backend
  test's CUDA rows now launch other kernels: under main's new
  row-launched-no-kernel-at-all gate an empty kernel set is no evidence.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>

@tahsintunan tahsintunan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@khluu
khluu merged commit cd854d8 into main Oct 24, 2026
3 checks passed
@AndreasKaratzas
AndreasKaratzas deleted the akaratza_rocm_ci_selector branch October 24, 2026 01:51
AndreasKaratzas added a commit that referenced this pull request Oct 25, 2026
Keeps #737's MI250 queue consolidation in the template next to the
build.sh layout's native agent_tags rule, and updates #685's ROCm kernel
map doc for the build.sh layout (map written in the csrc stage, exported
by the kernel-symbol-map target, uploaded by build.sh ci).

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
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