Skip to content

fix(hrr): translate kernel arguments that hide a pointer, keep the ones that do not - #10644

Merged
danielbolsson merged 118 commits into
developfrom
users/danielbolsson/hrr-kernel-args
Sep 30, 2026
Merged

danielbolsson merged 118 commits into
developfrom
users/danielbolsson/hrr-kernel-args

Conversation

@danielbolsson

Copy link
Copy Markdown
Contributor

AIRUNTIME-2706 · last of nine in the stack rooted at #10221 · base is #10642

What this changes

A pointer-typed kernel argument does not always hold a pointer, and a pointer does not always sit in a slot typed as one. Both cases end with the GPU reading a capture-time address, and both were being handled the same way.

A pointer the metadata does not describe as one. Kernel metadata is only as good as whoever wrote it. Hand-written assembly carries hand-written metadata, and aiter's MLA decode kernel declares a 64-bit buffer address as two 4-byte by_value i32 fields — out16 followed by a pad. Neither half is eight bytes wide, so neither the capture-side detector nor the per-argument rescan finds a pointer in it, the capture-time address reaches the GPU unchanged, and the kernel builds a buffer descriptor that faults on an address belonging to the recording process. Adjacent scalar halves are now joined and the pair translated when it resolves to a recorded allocation.

A sentinel in a slot typed as a pointer. The same MLA kernel passes a literal 1 in one. Nulling that changes what the kernel computes, silently and only on replay.

Narrowing, not reversing

These two want opposite defaults for a value that resolves nowhere, and the review history has already been around this once: andraghetti asked on #10225 whether an unresolved argument should reach the kernel as the recorded address rather than null, that was accepted, and dbe7faf7ea reverted it to null. Passing everything through would put that back.

They do not actually conflict, because the cases are distinguishable by shape:

Recorded value Treatment Why
Not VA-shaped (< 0x10000, or a packed-integer false positive) passed through unchanged no device VA could be this; it is a sentinel and it means something
VA-shaped, resolves in no allocation, VMM reservation or region reaches the kernel as null a pointer HRR genuinely lost — the driver tends to reproduce VA layout across runs, so forwarding it would more likely scribble over an unrelated live buffer than fault

The predicate already existed on the embedded-pointer path; it is now hoisted and shared rather than duplicated, and the warning names which of the two happened.

Recorded addresses that never reach an argument

An address can also arrive inside a buffer — written by an earlier kernel, or by the host into memory it shares with the device — where nothing in the translation path sees it. --scan-args and --scan-h2d read those bytes back and report the addresses rather than rewriting them, since a payload is opaque and a value that looks like an address may be data.

Diagnostics alongside: --dump-ptrs now names the allocation each argument lands in (calling out pinned host memory, which the application fills with CPU stores no HIP call reports) and prints every argument as capture classified it; HIP_HRR_REPLAY_FILL_BYTE fills fresh allocations with a recognisable pattern, so memory nothing in the archive ever wrote is identifiable on sight in a fault address.

Reviewing this

434 lines / 4 files, plus the regression test. hip_playback.cpp is the only file with logic in it — hip_playback.h is flags and helpers, hrr_playback.cpp is the usage text.

Semantically independent of Stage C; it stacks last only so the decode_kernel_args context #10641 restructures does not have to be re-resolved.

Validation

Full HRR unit suite on gfx950, GPU 7: 87/87, both passes.

Unit_HRR_KernelArgs_SentinelKeptLostPointerNulled launches a kernel with both a sentinel (0x1) and a far-offset lost pointer, and asserts each is treated its own way. Verified failing first: with the narrowing reverted to unconditional pass-through, the sentinel assertion still passes while both lost-pointer assertions fail (arg[2]: recorded 0x7f8cec201000 ... passing it through unchanged).

Made with Cursor

@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

danielbolsson and others added 20 commits September 21, 2026 08:40
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>
…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>
…clusions' into users/danielbolsson/hrr-kernel-args

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>
…clusions' into users/danielbolsson/hrr-kernel-args

Co-authored-by: Daniel Olsson <danielbolsson@users.noreply.github.com>
danielbolsson and others added 7 commits September 28, 2026 16:26
# Conflicts:
#	projects/hrr/tools/gen_hrr_api_args.py
Resolve conflicts from the #10641 squash merge by keeping this branch's
side: develop's copy of every conflicted file equals #10641's final head
(7039999), which this branch already contains.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nielbolsson/hrr-kernel-args

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…clusions' into users/danielbolsson/hrr-kernel-args
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants