Conversation
…tial hidden blocks
The AMDGPU dispatch path synthesized the HIP/OpenCL implicit ("hidden")
kernarguments by memsetting a fixed iree_amdgpu_kernel_implicit_args_t
struct at a single base offset (the lowest hidden-argument offset) and
required that base to be 8-byte aligned. That model is wrong for
hand-written assembly kernels -- for example MIOpen Winograd convolution
kernels -- which declare only a partial hidden block that begins with
sub-8-byte hidden_none padding. For those kernels the region base is
unaligned, so layout construction rejected the kernel outright; and even
when a kernel was accepted, splatting a full fixed-size struct wrote
fields the kernel never declared and could run past the kernarg segment.
Replace the single-struct model with per-field emplacement:
- Record each synthesized hidden field's own native offset from the code
object metadata value_kind (hidden_block_count_x/y/z,
hidden_group_size_x/y/z, hidden_grid_dims, hidden_dynamic_lds_size) and
reject any recognized field that declares an unexpected byte size.
Fields the kernel does not declare stay marked absent and are never
written.
- Drop the 8-byte base-alignment rejection. Instead validate that every
declared implicit field lies within the kernarg segment, and require
hidden_block_count_x/y/z to be either all omitted or declared as a
contiguous uint32[3] block anchored at a 4-byte-aligned offset -- the
contract the device indirect-parameter patch relies on when it rewrites
block_count as a fixed uint32[3] store anchored at the x field. The
4-byte anchor alignment replaces the removed 8-byte base reject, scoped
to where the device actually performs aligned dword stores rather than
to the (possibly unaligned) region base.
- Size the native kernarg segment as exactly the code object's
kernarg_segment_size instead of reserving a fixed implicit suffix past
it.
- At dispatch, zero the implicit region (so undeclared fields such as the
global offset and printf/hostcall buffers are zero/NULL, matching the
HIP defaults) and write only the declared fields, each at its own
offset. Both the native and custom-direct argument paths now source the
implicit offsets from the same native metadata layout, so the device
block_count patch is anchored consistently regardless of path.
The device-side iree_hal_amdgpu_device_dispatch_emplace_implicit_args,
whose sole caller was the host dispatch reroute above, is orphaned by that
reroute and removed along with its declaration and unit test.
Adds regression tests covering: per-field offset recording at the ABI
offsets and rejection of a hidden field with an unexpected size; full-block
emplacement leaving every undeclared byte zero; partial-block emplacement
over an unaligned region with no field writes; and per-field rejection of
an out-of-bounds field, a non-contiguous block_count, and a misaligned
block_count anchor.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
1 participant
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.
Problem
Hand-written GCN assembly kernels (for example MIOpen Winograd convolution) start their AMD hidden/implicit kernarg block at an unaligned base offset (observed at byte 201) and may declare only a partial subset of hidden fields. The AMDGPU kernarg layout rejected any implicit base that was not 8-byte aligned and wrote a fixed full implicit struct at the base offset, so these kernels failed to load — and the failed load surfaced downstream as a crash.
Fix
Record each declared implicit field's own native offset during code-object analysis; validate every declared field's bounds; require hidden
block_countto be either omitted or a contiguousuint32[3]with a 4-byte-aligned anchor (the device performsuint32dword stores); and emplace only the declared fields at their own offsets, zeroing the region first. The host dispatch path is rerouted so both the native and custom-direct paths source the native layout and anchor the indirectblock_countwrite at the declared offset, skipping it whenblock_countis undeclared. Removes a device implicit-emplace helper left with no caller by the reroute.Test
Adds unit coverage for per-field offset recording (catching a transposed mapping), unexpected-size rejection, and the out-of-bounds, non-contiguous, and misaligned
block_countrejections, plus full and partial/unaligned emplacement.Notes
Composes on top of the native-argument packing already on main. Opened as a draft: the amdgpu package builds under a pinned-ROCm-header configuration, and CI validates the full driver build and GPU dispatch path — a recommended gate before this leaves draft, as it was rebased across a recent rewrite of the dispatch-packet construction it interacts with.
🤖 Generated with Claude Code