Repository navigation
feat(hrr): refuse the calls replay cannot reproduce, by name - #10642
Merged
Merged
Conversation
danielbolsson
force-pushed
the
users/danielbolsson/hrr-scope-exclusions
branch
from
August 24, 2026 18:31
9c78e35 to
1f1b32c
Compare
danielbolsson
requested review from
a team,
AryanSalmanpour,
amd-arosa,
atgutier,
cfreeamd,
dayatsin-amd,
jrmadsen,
kentrussell,
kiritigowda,
rrawther,
shwetagkhatri and
xiaogang-chen-amd
as code owners
August 27, 2026 09:52
…oads' into users/danielbolsson/hrr-scope-exclusions
…ix' into users/danielbolsson/hrr-api-payloads
…oads' into users/danielbolsson/hrr-scope-exclusions
Raw fprintf(stderr, ...) is not permitted in clr: it bypasses the AMD_LOG_LEVEL machinery and writes unconditionally from inside a library that other processes embed. mark_incomplete() is the last such call site in this file, so it is routed through LogPrintfError from utils/debug.hpp, which the file already includes. Error level rather than warning: once this fires the archive is not faithful and replay must not treat it as one. The trailing newline is dropped because amd::log_printf appends its own. The conversion was reverted on #10634 (cd4d649) so that PR's existing approval stays valid. That revert is an ancestor of this branch, so cascading it up the stack silently restored the raw fprintf here with no conflict. This commit re-applies the conversion explicitly, so the fix still reaches develop with this PR while #10634 keeps its approved shape. The converted text is byte-identical to the form already carried by the rest of the stack, which is what keeps the remaining merges conflict-free. Signed-off-by: Daniel Olsson <Daniel.Olsson@amd.com>
…bolsson/hrr-scope-exclusions
…bolsson/hrr-scope-exclusions
…n/hrr-api-payloads
…bolsson/hrr-scope-exclusions
The generated capture shim called note_unreplayable() before the real call, but writes the event only when the call succeeds. A failed call therefore listed the API under manifest.json's unreplayable_apis and logged "The call is recorded" although nothing was recorded. Emit the note next to write_event_raw() instead. Also correct the PlaybackContext comment: unreplayable_apis and reproduced_errors de-duplicate the per-API stderr lines; they are not printed in the replay summary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gandryey
approved these changes
Sep 23, 2026
mangupta
reviewed
Sep 24, 2026
…bolsson/hrr-scope-exclusions
…list
note_unreplayable() de-duplicated on the API name alone, so once an API was
listed for one reason every later, different reason (another truncated
argument, say) was dropped from both the warning and manifest.json. Key on
(api, reason) instead: each distinct pair warns once, and the manifest now
records the reasons as "unreplayable_apis": {"<api>": ["<reason>", ...]}.
Nothing reads the field yet, so the shape change breaks no consumer.
Both strings are written through metadata::json_escape(), now exported from
hip_capture_metadata.h, so a name or reason with a quote or backslash can no
longer produce a manifest.json that does not parse.
Host graph nodes: the unreplayable handler returns hipErrorNotSupported, so
by default replay stops at the call, not at instantiate. mark_graph_incomplete
stays because it is what makes instantiate refuse the graph under
--continue-on-error; the generator comment and DESIGN.md now say so.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n/hrr-api-payloads # Conflicts: # projects/clr/hipamd/src/hrr/hip_capture.cpp Co-authored-by: Daniel Olsson <danielbolsson@users.noreply.github.com>
…oads' into users/danielbolsson/hrr-scope-exclusions Co-authored-by: Daniel Olsson <danielbolsson@users.noreply.github.com>
…n/hrr-api-payloads Co-authored-by: Daniel Olsson <danielbolsson@users.noreply.github.com>
…oads' into users/danielbolsson/hrr-scope-exclusions Co-authored-by: Daniel Olsson <danielbolsson@users.noreply.github.com>
# Conflicts: # projects/hrr/tools/gen_hrr_api_args.py
Contributor
TheRock Submodule Bump ActivityNewest first
|
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
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.
AIRUNTIME-2705 · seventh of nine in the stack rooted at #10221 · base is #10641
What this changes
Some calls cannot be reproduced in a different process no matter how much of the argument is recorded. A host callback is a function pointer into the capturing process; a shareable handle names an export only its owner can make; a multi-device launch descriptor carries a host function address per device. Recording more bytes does not help — the missing thing is not in the arguments.
Until now these were handed to the real API anyway and failed somewhere inside the runtime, where the diagnosis is a bare HIP error code with no attribution, or they were quietly stubbed out. Twelve APIs become
UNREPLAYABLE_PLAYBACK_APISinstead. The handler names itself and its reason on stderr, returnshipErrorNotSupported(fatal unless--continue-on-error), and is listed in the replay summary. Capture warns once at record time too, so the gap is visible when the recording is made rather than only when someone later tries to replay it, and the archive manifest carries the list.A graph node that cannot be replayed marks its graph incomplete, so instantiation fails loudly rather than running a graph short a node.
The twelve
hipLaunchHostFunc,hipLaunchHostFunc_spt,hipStreamAddCallback,hipStreamAddCallback_spthipGraphAddHostNode,hipGraphHostNodeSetParams,hipGraphExecHostNodeSetParamshipMemImportFromShareableHandle,hipMemPoolImportFromShareableHandlehipLaunchCooperativeKernelMultiDevice,hipExtLaunchMultiKernelMultiDevicehipUserObjectCreatehipMemPoolImportFromShareableHandleis the no-op #10641 deliberately left behind for this PR: its export side now genuinely mints an fd, so the recorded handle names something else here and faults inside the pool import.Guardrails in the generator
An unreplayable API with a hand-written capture shim is refused, because that shim would skip the capture-time warning. So is one that appears in a second playback class, because only the first would take effect.
Reviewing this
Small and self-contained — 344 insertions across 11 files, and the whole PR is one idea. The expectations delta is the twelve rows moving to
kHrrExpectUnreplayable.Validation
Full HRR unit suite on gfx950, GPU 7: 86/86, both passes.
gen_hrr_api_args.pyregenerates to an empty diff.Made with Cursor