Skip to content

feat(rocr): add signal handle hints and hsa_amd_signal_batch_create - #12988

Merged
arvindcheru merged 1 commit into
developfrom
users/saleelk/amdSignalV2
Oct 17, 2026
Merged

arvindcheru merged 1 commit into
developfrom
users/saleelk/amdSignalV2

Conversation

@saleelk

@saleelk saleelk commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Add per-use handle hints for AQL completion signals, and an API that
allocates many signals from one arena in a host fine-grained memory pool
or in large-BAR GPU memory.

Signal handle hints (amd_hsa_signal.h):

  • amd_signal_t is unchanged. A signal is 64-byte aligned, so the low 6
    bits of its handle are zero, and amd_signal_hint_t names hints a
    producer may set there per use: interrupt kind (bits 1-2), binary
    value update (bit 3) and timestamp mode INHERIT, ON or OFF (bits 4-5).
    Bit 0 is reserved. Zero in every field means today's behaviour.
  • INTR_DEFAULT interrupts only if the signal's event_id is non-zero;
    INTR_ON_ZERO only on a non-zero to zero transition, and only if
    event_id is non-zero.
  • Hints are gated on a CP capability and binding once advertised. CP
    firmware that consumes them sets AMD_QUEUE_CAPS_CP_SIGNAL_HINTS (bit 2
    of amd_queue_t::caps) and must then honour every defined hint; firmware
    without it takes a hinted handle as the signal address. The CP writes
    caps only once it has processed a packet on the queue, so the new GPU
    agent query HSA_AMD_AGENT_INFO_SIGNAL_HINTS_SUPPORTED submits one empty
    barrier-AND (no fence scopes, a busy-wait completion signal) to the
    utility queue the first time it is asked, waits for it, and reports
    the bit.
    Nothing in ROCr emits hints.
  • AMD_SIGNAL_HINT_MASK and amd_signal_from_handle() strip hints.
    hsa_ext_amd.h adds inline hsa_amd_signal_set_hints(), which drops bits
    outside the mask, and hsa_amd_signal_get_hints(). A hinted handle is
    only for an AQL completion_signal.

SharedSignal:

  • The SDMA timestamp sidecar is removed. SDMA writes amd_signal.start_ts
    and end_ts directly (SDMA V4+ only needs 8-byte alignment), and the
    32-byte alignment asserts go away.
  • core_signal and id stay at 0x48 and 0x50 for IPC compatibility.

Batch API (interface version 1.36, AmdExtTable step 0x17):

  • hsa_amd_signal_batch_create(count, initial_values, num_consumers,
    consumers, attributes, pool, batch, signals)
  • hsa_amd_signal_batch_destroy(batch)
  • core::SignalBatch (signal.h/.cpp) allocates one NonPaged arena of
    128-byte slots. Each slot backs an ordinary DefaultSignal or
    InterruptSignal, so waits, copies and profiling are unchanged.
  • LocalSignal can wrap a batch-owned slot. It holds a batch reference,
    so the arena is freed only after the batch handle and every member
    signal are released. Members can be destroyed individually.
  • HSA_AMD_MEMORY_POOL_INFO_SIGNAL_CAPABLE reports whether batch create
    accepts a pool, so runtimes pick a device pool without hard-coding
    GPU targets. AMD::MemoryRegion::SignalCapable holds the rule for both.
  • The interrupt/busy-wait choice is shared with hsa_amd_signal_create.
  • IPC attributes and other pool kinds are rejected.

Device memory:

  • The pool may also be a device pool of a GPU with a large BAR, one
    where every queue's CP sees a completion: the uncached
    (extended-scope fine-grained) pool on gfx94x/gfx95x, and the
    coarse-grained pool on gfx125x. On MI300X the
    coarse-grained and fine-grained pools are mapped MTYPE RW, where the
    CP's binary completion store is not seen by another queue's CP; the
    uncached pool is MTYPE CC. Every other pool is rejected. The arena is
    made accessible to the CPU and to the consumers, or to every GPU if
    there are none.
  • Two new GPU agent queries describe how that memory is reached:
    HSA_AMD_AGENT_INFO_PCIE_STORES_NEED_FLUSH (true over PCIe, for the host
    and for peers) and HSA_AMD_AGENT_INFO_HOST_ATOMICS_SUPPORTED (true for
    A+A, CPU and GPU coherently connected over xGMI). Runtimes
    that write signal values directly use them to batch their own flushes.
  • When host stores go through HDP, every host store from the ROCr
    signal API is followed by a readback (store fence, re-store the value
    just written, full fence, read it back, with the portable fences from
    core/util/utils.h), as CLR does for device kernel
    arguments. Re-storing the known value avoids reading it over PCIe
    first. Host read-modify-write (add, subtract, and, or, xor, exchange,
    cas) is not supported on device-memory members unless the GPU reports
    host atomics: there it becomes load, modify, store, which is not atomic,
    even between host threads. The signal caches this at creation, so
    host operations do not read the signal over PCIe to decide.

rocprofiler-sdk traces the two new AmdExtTable entries (step 0x17): API
ids, argument records, enum labels, ABI slots 96 and 97, and a handle
formatter for hsa_amd_signal_batch_t.

Test: standalone HSA test on MI300X, with SDMA and blit-kernel copies.
Covers batches with interrupt and busy-wait members from host
fine-grained and GPU uncached pools (GPU coarse-grained and fine-grained
rejected on MI300X, matching HSA_AMD_MEMORY_POOL_INFO_SIGNAL_CAPABLE),
stride/alignment, handle hint set/get/stripping, the two agent queries,
blocked waits, async copy completion with profiling timestamps, early
member destroy, error paths and 2000 batch create/destroy cycles. A C99
translation unit checks the header compiles as C. A caps probe on MEC
firmware 1019 and 1062 shows bit 2 set only after the queue's first
packet, the hint query true, and hinted TS_ON timestamps on a queue
without profiling.

rocrtst adds rocrtstFunc.Signal_Batch_{Host_Pool,Device_Pools,
Async_Copy_Completion,Negative,Agent_Queries} and Signal_Handle_Hints
(standard tier). Expectations follow the queries rather than a GPU
target: device pools must be accepted exactly when SIGNAL_CAPABLE is
set, and PCIE_STORES_NEED_FLUSH must be the inverse of
HOST_ATOMICS_SUPPORTED. All six pass on MI300X.

@therock-pr-bot

therock-pr-bot Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

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

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Oct 7, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@cfreeamd cfreeamd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 AI-assisted pre-review (for @cfreeamd)

hsa_table_interface.cpp / signal.h / hsa_ext_amd.h additions look structurally sound. amd-ext-api-version-bump checklist passes (step 0x14→0x15, interface 1.33→1.34, both new fn pointers appended at tail of AmdExtTable, expected_amd_ext_table_size bumped 760→776). completion-signal-ordering and event-wait-hang patterns assessed — no issues found.

[Required] PR title rocr: add amd_signal_v2_t layout and hsa_amd_signal_batch_create does not follow the required type(component): format. ROCm squash-merges, so this becomes the permanent commit title. Suggest: feat(rocr): add amd_signal_v2_t layout and hsa_amd_signal_batch_create

@saleelk
saleelk force-pushed the users/saleelk/amdSignalV2 branch from 4484f78 to f9d109e Compare October 10, 2026 01:08
@saleelk saleelk changed the title rocr: add amd_signal_v2_t layout and hsa_amd_signal_batch_create rocr: add signal handle hints and hsa_amd_signal_batch_create Oct 10, 2026
saleelk added a commit that referenced this pull request Oct 10, 2026
…12991)

Stacked on #12990 (part 1), which is stacked on #12988 (ROCr), so this
PR shows only the part 2 commit. It will be retargeted once those merge.

Part 2 of a series moving CLR completion signals to the ROCr batch API
and device memory. This part prepares HwQueueTracker for a second
signal reuse strategy, which the next part adds for queue completion
signals in device memory. No behaviour change.

HwQueueTracker becomes an abstract base that keeps the flow shared by
any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal,
WaitCurrent, the interrupt/non-interrupt pools and the external and
dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the
parts that decide how a signal is proven free and reset:

- AcquireSlot: move to the next slot and make its signal reusable.
- BeginUse: start the use at init_val.
- SetAsyncHandler: register the completion handler for a callback.
- IsBusy: whether a signal to wait on may still be in use.
- ResetCurrentSignal: roll back a slot after a ROCr failure.

LegacySignalTracker implements them with the existing code, moved
unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext,
replace a signal shared with an event, then signal_silent_store at
hand-out.

VirtualGPU owns the tracker through std::unique_ptr, so the tracker no
longer derives from amd::EmbeddedObject, which forbids heap allocation.
Barriers() still returns a reference, so call sites are unchanged.

Test (MI300X):
- The moved bodies (AcquireSlot, WaitNext, ResetCurrentSignal) are
  textually identical to the code they replace.
- HIP stream/graph test passes; graph_bench --verify passes.
- graph_bench --sync times match the previous commit within noise.
- hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical
  to the previous commit with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1 (all
  pre-existing).
@saleelk
saleelk force-pushed the users/saleelk/amdSignalV2 branch from f9d109e to 89c890e Compare October 12, 2026 01:03
saleelk added a commit that referenced this pull request Oct 12, 2026
…12991)

Stacked on #12990 (part 1), which is stacked on #12988 (ROCr), so this
PR shows only the part 2 commit. It will be retargeted once those merge.

Part 2 of a series moving CLR completion signals to the ROCr batch API
and device memory. This part prepares HwQueueTracker for a second
signal reuse strategy, which the next part adds for queue completion
signals in device memory. No behaviour change.

HwQueueTracker becomes an abstract base that keeps the flow shared by
any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal,
WaitCurrent, the interrupt/non-interrupt pools and the external and
dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the
parts that decide how a signal is proven free and reset:

- AcquireSlot: move to the next slot and make its signal reusable.
- BeginUse: start the use at init_val.
- SetAsyncHandler: register the completion handler for a callback.
- IsBusy: whether a signal to wait on may still be in use.
- ResetCurrentSignal: roll back a slot after a ROCr failure.

LegacySignalTracker implements them with the existing code, moved
unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext,
replace a signal shared with an event, then signal_silent_store at
hand-out.

VirtualGPU owns the tracker through std::unique_ptr, so the tracker no
longer derives from amd::EmbeddedObject, which forbids heap allocation.
Barriers() still returns a reference, so call sites are unchanged.

Test (MI300X):
- The moved bodies (AcquireSlot, WaitNext, ResetCurrentSignal) are
  textually identical to the code they replace.
- HIP stream/graph test passes; graph_bench --verify passes.
- graph_bench --sync times match the previous commit within noise.
- hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical
  to the previous commit with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1 (all
  pre-existing).
@saleelk saleelk changed the title rocr: add signal handle hints and hsa_amd_signal_batch_create feat(rocr): add signal handle hints and hsa_amd_signal_batch_create Oct 12, 2026
@saleelk
saleelk force-pushed the users/saleelk/amdSignalV2 branch from 89c890e to 6c08439 Compare October 12, 2026 19:22
saleelk added a commit that referenced this pull request Oct 13, 2026
…QueueTracker (#12990)

Stacked on #12988 (ROCr batch signal API). It will be retargeted to
`develop` once #12988 merges. This includes the HwQueueTracker split
reviewed in #12991.

Part 1 of a series moving CLR completion signals to the ROCr batch API
and device memory. This part places the hip graph pool signals in device
memory, and prepares HwQueueTracker for a second signal reuse strategy,
which a later part adds for queue completion signals in device memory.

Graph pool signals:

Use the ROCr batch signal API for the signals CLR pre-creates for the
segmented hip graph path, so each pool set comes from one allocation,
and place those signals in device memory where the host can reach it.

- rocrctx: load hsa_amd_signal_batch_create/destroy and add
  Hsa::signal_batch_create/destroy wrappers. GET_ROCR_OPTIONAL_SYMBOL
  now casts through decltype like GET_ROCR_SYMBOL, so
  hsa_amd_queue_create no longer needs a hand-written function typedef,
  and GET_ROCR_OPTIONAL_SYMBOL_DECLTYPE, which did the same, goes away.
- Device::CreateSignals(count, initial_value, attributes, batched,
  device_memory, signals, in_device_memory): when batched, create v1
  signals as one batch and release the batch handle immediately;
  device_memory takes the batch from the device's uncached fine-grained
  pool (gpu_ext_fine_grained_segment_), which is only reachable through
  a batch. ProfilingSignal still destroys each signal individually,
  which ROCr allows in any order. Falls back to per-signal host signals
  if the batch fails, and reports where the signals were placed.
- amd::Device::CreateHwEvents gains a batched argument. Graph pool sets
  are batched: GraphSignalManager::Prepopulate and the AcquireSet
  fallback, whose set joins the pool when released. The non-pooled
  launch path keeps per-signal creation unless the signals go to device
  memory.
- DEBUG_CLR_GRAPH_DEV_SIGNALS (default on) places graph signals in
  device memory when the device has a large BAR, so the barrier packets
  between graph segments poll device-local memory instead of host
  memory. Each ProfilingSignal records where it lives. ResetHwEvents
  re-arms device signals with direct stores and one HDP flush
  (Device::FlushSignalStores), issued only when ROCr's
  HSA_AMD_AGENT_INFO_HOST_STORES_NEED_HDP_FLUSH reports that host
  stores need it; host signals keep signal_silent_store.
- The flush is skipped when the device's AQL rings are in device memory
  (Device::AqlRingsInDeviceMemory, now also used by acquireQueue). Only
  packets on this device's rings wait on graph signals, and they reach
  a device-memory ring through the same BAR after the stores, in order,
  as dispatch already relies on for kernel arguments.

Queue completion signals are unchanged. All batches are v1 (128-byte
slots); no signal hints are emitted.

A host batch costs about 60 us regardless of count on MI300X and a
device batch 86-158 us, versus under 1 us per pooled signal, so only
pooled sets are batched; the AcquireSet fallback pays it once per pool
growth.

HwQueueTracker split (no behaviour change):

HwQueueTracker becomes an abstract base that keeps the flow shared by
any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal,
WaitCurrent, the interrupt/non-interrupt pools and the external and
dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the
parts that decide how a signal is proven free and reset:

- AcquireSlot: move to the next slot and make its signal reusable.
- BeginUse: start the use at init_val.
- SetAsyncHandler: register the completion handler for a callback.
- IsBusy: whether a signal to wait on may still be in use.
- ResetCurrentSignal: roll back a slot after a ROCr failure.

LegacySignalTracker implements them with the existing code, moved
unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext,
replace a signal shared with an event, then signal_silent_store at
hand-out.

VirtualGPU owns the tracker through std::unique_ptr, so the tracker no
longer derives from amd::EmbeddedObject, which forbids heap allocation.
Barriers() still returns a reference, so call sites are unchanged.

Performance (MI300X, end to end, median of 3 alternating rounds, host
vs device graph signals, AQL rings in device memory):
- graph_bench --sync: paths4 98 -> 64 us (64 nodes), 318 -> 214 us
  (256), 1263 -> 815 us (1024); paths2 -19% to -21%; full4 up to -13%;
  full2 and straight unchanged.
- MoE dual-stream graph: 5 layers 344 -> 299 us (original order),
  308 -> 284 us (reversed); 93 layers 6289 -> 5347 us and
  5531 -> 5079 us; single-stream unchanged.

Test (MI300X):
- graph_bench --verify and hipGraphCollapseCorrectness (collapsed and
  multi-stream) pass with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1, with AQL
  rings in device and in system memory.
- HIP stream/graph test with an LD_PRELOAD counter, under
  DEBUG_HIP_GRAPH_MIN_OVERLAP=0 and DEBUG_HIP_GRAPH_SEGMENT_SCHEDULING=1
  and 2: graph pool sets, including AcquireSet fallbacks, come from
  batches; all results correct with the flag off and on.
- The moved tracker bodies (AcquireSlot, WaitNext, ResetCurrentSignal)
  are textually identical to the code they replace.
- hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical
  with the flag off and on (all pre-existing).
- rocrctx.cpp, rocdevice.cpp and rocvirtual.cpp compile with
  ROCR_DYN_DLL defined.
@saleelk
saleelk requested a review from a team as a code owner October 13, 2026 16:54
@saleelk
saleelk force-pushed the users/saleelk/amdSignalV2 branch from 1b6cf43 to 4bde386 Compare October 13, 2026 17:24
@saleelk
saleelk requested review from a team as code owners October 13, 2026 17:24

@bwelton bwelton left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes, mainly for the non-atomic host RMW on device-memory signals and HIP failing to load against an older ROCr. See the inline comments.

Comment thread projects/clr/rocclr/utils/flags.hpp Resolved
Add per-use handle hints for AQL completion signals, and an API that
allocates many signals from one arena in a host fine-grained memory pool
or in large-BAR GPU memory.

Signal handle hints (amd_hsa_signal.h):
- amd_signal_t is unchanged. A signal is 64-byte aligned, so the low 6
  bits of its handle are zero, and amd_signal_hint_t names hints a
  producer may set there per use: interrupt kind (bits 1-2), binary
  value update (bit 3) and timestamp mode INHERIT, ON or OFF (bits 4-5).
  Bit 0 is reserved. Zero in every field means today's behaviour.
- INTR_DEFAULT interrupts only if the signal's event_id is non-zero;
  INTR_ON_ZERO only on a non-zero to zero transition, and only if
  event_id is non-zero.
- Hints are gated on a CP capability and binding once advertised. CP
  firmware that consumes them sets AMD_QUEUE_CAPS_CP_SIGNAL_HINTS (bit 2
  of amd_queue_t::caps) and must then honour every defined hint; firmware
  without it takes a hinted handle as the signal address. The CP writes
  caps only once it has processed a packet on the queue, so the new GPU
  agent query HSA_AMD_AGENT_INFO_SIGNAL_HINTS_SUPPORTED submits one empty
  barrier-AND (no fence scopes, a busy-wait completion signal) to the
  utility queue the first time it is asked, waits for it, and reports
  the bit.
  Nothing in ROCr emits hints.
- AMD_SIGNAL_HINT_MASK and amd_signal_from_handle() strip hints.
  hsa_ext_amd.h adds inline hsa_amd_signal_set_hints(), which drops bits
  outside the mask, and hsa_amd_signal_get_hints(). A hinted handle is
  only for an AQL completion_signal.

SharedSignal:
- The SDMA timestamp sidecar is removed. SDMA writes amd_signal.start_ts
  and end_ts directly (SDMA V4+ only needs 8-byte alignment), and the
  32-byte alignment asserts go away.
- core_signal and id stay at 0x48 and 0x50 for IPC compatibility.

Batch API (interface version 1.36, AmdExtTable step 0x17):
- hsa_amd_signal_batch_create(count, initial_values, num_consumers,
  consumers, attributes, pool, batch, signals)
- hsa_amd_signal_batch_destroy(batch)
- core::SignalBatch (signal.h/.cpp) allocates one NonPaged arena of
  128-byte slots. Each slot backs an ordinary DefaultSignal or
  InterruptSignal, so waits, copies and profiling are unchanged.
- LocalSignal can wrap a batch-owned slot. It holds a batch reference,
  so the arena is freed only after the batch handle and every member
  signal are released. Members can be destroyed individually.
- HSA_AMD_MEMORY_POOL_INFO_SIGNAL_CAPABLE reports whether batch create
  accepts a pool, so runtimes pick a device pool without hard-coding
  GPU targets. AMD::MemoryRegion::SignalCapable holds the rule for both.
- The interrupt/busy-wait choice is shared with hsa_amd_signal_create.
- IPC attributes and other pool kinds are rejected.

Device memory:
- The pool may also be a device pool of a GPU with a large BAR, one
  where every queue's CP sees a completion: the uncached
  (extended-scope fine-grained) pool on gfx94x/gfx95x, and the
  coarse-grained pool on gfx125x. On MI300X the
  coarse-grained and fine-grained pools are mapped MTYPE RW, where the
  CP's binary completion store is not seen by another queue's CP; the
  uncached pool is MTYPE CC. Every other pool is rejected. The arena is
  made accessible to the CPU and to the consumers, or to every GPU if
  there are none.
- Two new GPU agent queries describe how that memory is reached:
  HSA_AMD_AGENT_INFO_PCIE_STORES_NEED_FLUSH (true over PCIe, for the host
  and for peers) and HSA_AMD_AGENT_INFO_HOST_ATOMICS_SUPPORTED (true for
  A+A, CPU and GPU coherently connected over xGMI). Runtimes
  that write signal values directly use them to batch their own flushes.
- When host stores go through HDP, every host store from the ROCr
  signal API is followed by a readback (store fence, re-store the value
  just written, full fence, read it back, with the portable fences from
  core/util/utils.h), as CLR does for device kernel
  arguments. Re-storing the known value avoids reading it over PCIe
  first. Host read-modify-write (add, subtract, and, or, xor, exchange,
  cas) is not supported on device-memory members unless the GPU reports
  host atomics: there it becomes load, modify, store, which is not atomic,
  even between host threads. The signal caches this at creation, so
  host operations do not read the signal over PCIe to decide.

rocprofiler-sdk traces the two new AmdExtTable entries (step 0x17): API
ids, argument records, enum labels, ABI slots 96 and 97, and a handle
formatter for hsa_amd_signal_batch_t.

Test: standalone HSA test on MI300X, with SDMA and blit-kernel copies.
Covers batches with interrupt and busy-wait members from host
fine-grained and GPU uncached pools (GPU coarse-grained and fine-grained
rejected on MI300X, matching HSA_AMD_MEMORY_POOL_INFO_SIGNAL_CAPABLE),
stride/alignment, handle hint set/get/stripping, the two agent queries,
blocked waits, async copy completion with profiling timestamps, early
member destroy, error paths and 2000 batch create/destroy cycles. A C99
translation unit checks the header compiles as C. A caps probe on MEC
firmware 1019 and 1062 shows bit 2 set only after the queue's first
packet, the hint query true, and hinted TS_ON timestamps on a queue
without profiling.

rocrtst adds rocrtstFunc.Signal_Batch_{Host_Pool,Device_Pools,
Async_Copy_Completion,Negative,Agent_Queries} and Signal_Handle_Hints
(standard tier). Expectations follow the queries rather than a GPU
target: device pools must be accepted exactly when SIGNAL_CAPABLE is
set, and PCIE_STORES_NEED_FLUSH must be the inverse of
HOST_ATOMICS_SUPPORTED. All six pass on MI300X.
@saleelk
saleelk force-pushed the users/saleelk/amdSignalV2 branch from d2649bc to ce04fbb Compare October 15, 2026 23:41
@arvindcheru

Copy link
Copy Markdown
Contributor

ce04fbb: Ruleset 9297053 is already green:
rocprofiler-sdk CI and therock-pr-bot are outside that ruleset.

@arvindcheru
arvindcheru merged commit 60df11d into develop Oct 17, 2026
998 of 1017 checks passed
@arvindcheru
arvindcheru deleted the users/saleelk/amdSignalV2 branch October 17, 2026 00:38
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

saleelk added a commit that referenced this pull request Oct 21, 2026
…QueueTracker (#13424)

Builds on #12988 (ROCr batch signal API), now merged. This replaces
#12990, which was merged into `amdSignalV2` and split back out so #12988
stayed ROCr-only. It includes the HwQueueTracker split reviewed in
#12991.

Part 1 of a series moving CLR completion signals to the ROCr batch API
and device memory. This part places the hip graph pool signals in device
memory, and prepares HwQueueTracker for a second signal reuse strategy,
which a later part adds for queue completion signals in device memory.

Graph pool signals:

Use the ROCr batch signal API for the signals CLR pre-creates for the
segmented hip graph path, so each pool set comes from one allocation,
and place those signals in device memory where the host can reach it.

- rocrctx: load hsa_amd_signal_batch_create/destroy and add
  Hsa::signal_batch_create/destroy wrappers. GET_ROCR_OPTIONAL_SYMBOL
  now casts through decltype like GET_ROCR_SYMBOL, so
  hsa_amd_queue_create no longer needs a hand-written function typedef,
  and GET_ROCR_OPTIONAL_SYMBOL_DECLTYPE, which did the same, goes away.
  When ROCr is loaded dynamically (ROCR_DYN_DLL) the batch entry points
  are optional: against a ROCr older than interface 1.36,
  Hsa::signal_batch_create fails and CreateSignals falls back to
  per-signal creation. A build linked against ROCr requires them, as it
  already requires hsa_amd_queue_create.
- Device::CreateSignals(count, initial_value, attributes, batched,
  device_memory, signals, in_device_memory): when batched, create v1
  signals as one batch and release the batch handle immediately;
  device_memory takes the batch from Device::device_signal_pool_, which
  is only reachable through a batch. That is the first device pool ROCr
  reports with HSA_AMD_MEMORY_POOL_INFO_SIGNAL_CAPABLE: the uncached
  pool on gfx94x/gfx95x, the coarse-grained pool on gfx125x. Without
  one, or with a ROCr that lacks the query, signals stay in host
  memory. ProfilingSignal still destroys each signal individually,
  which ROCr allows in any order. Falls back to per-signal host signals
  if the batch fails, and reports where the signals were placed.
- amd::Device::CreateHwEvents gains a batched argument. Graph pool sets
  are batched: GraphSignalManager::Prepopulate and the AcquireSet
  fallback, whose set joins the pool when released. The non-pooled
  launch path (legacy recursive child graphs) creates its set on every
  launch, so it keeps per-signal host signals rather than pay for a
  device batch each time.
- DEBUG_CLR_GRAPH_DEV_SIGNALS (default on) places pooled graph signals
in device memory when the device has a large BAR, so the barrier packets
  between graph segments poll device-local memory instead of host
  memory. Each ProfilingSignal records where it lives. ResetHwEvents
  re-arms device signals with direct stores and one HDP flush
  (Device::FlushSignalStores), issued only when ROCr's
  HSA_AMD_AGENT_INFO_PCIE_STORES_NEED_FLUSH reports that the device's
  memory is reached over PCIe; host signals keep signal_silent_store.
- Graph signals stay in host memory, and AQL rings in system memory
  (AqlRingsInDeviceMemory), while a profiler tool is attached, i.e. when
  rocprofiler-register accepts HIP's API tables
  (amd::activity_prof::tool_attached). The tool scans and rewrites
  packets from the host, and rocprofiler-sdk's queue
  interposition adds to and subtracts from an application's completion
  signals with host atomics, which are not atomic on device memory over
  PCIe, and its completion monitor can still poll a signal after the
  graph destroys it, when the batch's device-memory arena is unmapped.
  hip-graph-forkjoin-deadlock segfaulted that way.
- The flush is skipped when the device's AQL rings are in device memory
  (Device::AqlRingsInDeviceMemory, now also used by acquireQueue). Only
  packets on this device's rings wait on graph signals, and they reach
  a device-memory ring through the same BAR after the stores, in order,
  as dispatch already relies on for kernel arguments. The predicate is
  the ring placement every such queue actually has: acquireQueue
  requests it, and ROCr fails the request rather than fall back to a
  system-memory ring.

Queue completion signals are unchanged. All batches are v1 (128-byte
slots); no signal hints are emitted.

A host batch costs about 60 us regardless of count on MI300X and a
device batch 86-158 us, versus under 1 us per pooled signal, so only
pooled sets are batched; the AcquireSet fallback pays it once per pool
growth.

HwQueueTracker split (no behaviour change):

HwQueueTracker becomes an abstract base that keeps the flow shared by
any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal,
WaitCurrent, the interrupt/non-interrupt pools and the external and
dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the
parts that decide how a signal is proven free and reset:

- AcquireSlot: move to the next slot and make its signal reusable.
- BeginUse: start the use at init_val.
- SetAsyncHandler: register the completion handler for a callback.
- IsBusy: whether a signal to wait on may still be in use.
- ResetCurrentSignal: roll back a slot after a ROCr failure.

LegacySignalTracker implements them with the existing code, moved
unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext,
replace a signal shared with an event, then signal_silent_store at
hand-out.

VirtualGPU owns the tracker through std::unique_ptr, so the tracker no
longer derives from amd::EmbeddedObject, which forbids heap allocation.
Barriers() still returns a reference, so call sites are unchanged.

Performance (MI300X, end to end, median of 3 alternating rounds, host
vs device graph signals, AQL rings in device memory):
- graph_bench --sync: paths4 98 -> 64 us (64 nodes), 318 -> 214 us
  (256), 1263 -> 815 us (1024); paths2 -19% to -21%; full4 up to -13%;
  full2 and straight unchanged.
- MoE dual-stream graph: 5 layers 344 -> 299 us (original order),
  308 -> 284 us (reversed); 93 layers 6289 -> 5347 us and
  5531 -> 5079 us; single-stream unchanged.

Test (MI300X):
- graph_bench --verify and hipGraphCollapseCorrectness (collapsed and
  multi-stream) pass with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1, with AQL
  rings in device and in system memory.
- HIP stream/graph test with an LD_PRELOAD counter, under
  DEBUG_HIP_GRAPH_MIN_OVERLAP=0 and DEBUG_HIP_GRAPH_SEGMENT_SCHEDULING=1
  and 2: graph pool sets, including AcquireSet fallbacks, come from
  batches; all results correct with the flag off and on.
- The moved tracker bodies (AcquireSlot, WaitNext, ResetCurrentSignal)
  are textually identical to the code they replace.
- hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical
  with the flag off and on (all pre-existing).
- rocrctx.cpp, rocdevice.cpp and rocvirtual.cpp compile with
  ROCR_DYN_DLL defined.
dgaliffiAMD pushed a commit that referenced this pull request Oct 21, 2026
…QueueTracker (#13424)

Builds on #12988 (ROCr batch signal API), now merged. This replaces
#12990, which was merged into `amdSignalV2` and split back out so #12988
stayed ROCr-only. It includes the HwQueueTracker split reviewed in
#12991.

Part 1 of a series moving CLR completion signals to the ROCr batch API
and device memory. This part places the hip graph pool signals in device
memory, and prepares HwQueueTracker for a second signal reuse strategy,
which a later part adds for queue completion signals in device memory.

Graph pool signals:

Use the ROCr batch signal API for the signals CLR pre-creates for the
segmented hip graph path, so each pool set comes from one allocation,
and place those signals in device memory where the host can reach it.

- rocrctx: load hsa_amd_signal_batch_create/destroy and add
  Hsa::signal_batch_create/destroy wrappers. GET_ROCR_OPTIONAL_SYMBOL
  now casts through decltype like GET_ROCR_SYMBOL, so
  hsa_amd_queue_create no longer needs a hand-written function typedef,
  and GET_ROCR_OPTIONAL_SYMBOL_DECLTYPE, which did the same, goes away.
  When ROCr is loaded dynamically (ROCR_DYN_DLL) the batch entry points
  are optional: against a ROCr older than interface 1.36,
  Hsa::signal_batch_create fails and CreateSignals falls back to
  per-signal creation. A build linked against ROCr requires them, as it
  already requires hsa_amd_queue_create.
- Device::CreateSignals(count, initial_value, attributes, batched,
  device_memory, signals, in_device_memory): when batched, create v1
  signals as one batch and release the batch handle immediately;
  device_memory takes the batch from Device::device_signal_pool_, which
  is only reachable through a batch. That is the first device pool ROCr
  reports with HSA_AMD_MEMORY_POOL_INFO_SIGNAL_CAPABLE: the uncached
  pool on gfx94x/gfx95x, the coarse-grained pool on gfx125x. Without
  one, or with a ROCr that lacks the query, signals stay in host
  memory. ProfilingSignal still destroys each signal individually,
  which ROCr allows in any order. Falls back to per-signal host signals
  if the batch fails, and reports where the signals were placed.
- amd::Device::CreateHwEvents gains a batched argument. Graph pool sets
  are batched: GraphSignalManager::Prepopulate and the AcquireSet
  fallback, whose set joins the pool when released. The non-pooled
  launch path (legacy recursive child graphs) creates its set on every
  launch, so it keeps per-signal host signals rather than pay for a
  device batch each time.
- DEBUG_CLR_GRAPH_DEV_SIGNALS (default on) places pooled graph signals
in device memory when the device has a large BAR, so the barrier packets
  between graph segments poll device-local memory instead of host
  memory. Each ProfilingSignal records where it lives. ResetHwEvents
  re-arms device signals with direct stores and one HDP flush
  (Device::FlushSignalStores), issued only when ROCr's
  HSA_AMD_AGENT_INFO_PCIE_STORES_NEED_FLUSH reports that the device's
  memory is reached over PCIe; host signals keep signal_silent_store.
- Graph signals stay in host memory, and AQL rings in system memory
  (AqlRingsInDeviceMemory), while a profiler tool is attached, i.e. when
  rocprofiler-register accepts HIP's API tables
  (amd::activity_prof::tool_attached). The tool scans and rewrites
  packets from the host, and rocprofiler-sdk's queue
  interposition adds to and subtracts from an application's completion
  signals with host atomics, which are not atomic on device memory over
  PCIe, and its completion monitor can still poll a signal after the
  graph destroys it, when the batch's device-memory arena is unmapped.
  hip-graph-forkjoin-deadlock segfaulted that way.
- The flush is skipped when the device's AQL rings are in device memory
  (Device::AqlRingsInDeviceMemory, now also used by acquireQueue). Only
  packets on this device's rings wait on graph signals, and they reach
  a device-memory ring through the same BAR after the stores, in order,
  as dispatch already relies on for kernel arguments. The predicate is
  the ring placement every such queue actually has: acquireQueue
  requests it, and ROCr fails the request rather than fall back to a
  system-memory ring.

Queue completion signals are unchanged. All batches are v1 (128-byte
slots); no signal hints are emitted.

A host batch costs about 60 us regardless of count on MI300X and a
device batch 86-158 us, versus under 1 us per pooled signal, so only
pooled sets are batched; the AcquireSet fallback pays it once per pool
growth.

HwQueueTracker split (no behaviour change):

HwQueueTracker becomes an abstract base that keeps the flow shared by
any strategy: Create, ActiveSignal, WaitingSignal, CpuWaitForSignal,
WaitCurrent, the interrupt/non-interrupt pools and the external and
dynamic-queue waits. ActiveSignal and WaitingSignal call hooks for the
parts that decide how a signal is proven free and reset:

- AcquireSlot: move to the next slot and make its signal reusable.
- BeginUse: start the use at init_val.
- SetAsyncHandler: register the completion handler for a callback.
- IsBusy: whether a signal to wait on may still be in use.
- ResetCurrentSignal: roll back a slot after a ROCr failure.

LegacySignalTracker implements them with the existing code, moved
unchanged: peek two slots ahead and grow, WaitCurrent and WaitNext,
replace a signal shared with an event, then signal_silent_store at
hand-out.

VirtualGPU owns the tracker through std::unique_ptr, so the tracker no
longer derives from amd::EmbeddedObject, which forbids heap allocation.
Barriers() still returns a reference, so call sites are unchanged.

Performance (MI300X, end to end, median of 3 alternating rounds, host
vs device graph signals, AQL rings in device memory):
- graph_bench --sync: paths4 98 -> 64 us (64 nodes), 318 -> 214 us
  (256), 1263 -> 815 us (1024); paths2 -19% to -21%; full4 up to -13%;
  full2 and straight unchanged.
- MoE dual-stream graph: 5 layers 344 -> 299 us (original order),
  308 -> 284 us (reversed); 93 layers 6289 -> 5347 us and
  5531 -> 5079 us; single-stream unchanged.

Test (MI300X):
- graph_bench --verify and hipGraphCollapseCorrectness (collapsed and
  multi-stream) pass with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1, with AQL
  rings in device and in system memory.
- HIP stream/graph test with an LD_PRELOAD counter, under
  DEBUG_HIP_GRAPH_MIN_OVERLAP=0 and DEBUG_HIP_GRAPH_SEGMENT_SCHEDULING=1
  and 2: graph pool sets, including AcquireSet fallbacks, come from
  batches; all results correct with the flag off and on.
- The moved tracker bodies (AcquireSlot, WaitNext, ResetCurrentSignal)
  are textually identical to the code they replace.
- hip-tests StreamTest, GraphsTest1, GraphsTest2: failure sets identical
  with the flag off and on (all pre-existing).
- rocrctx.cpp, rocdevice.cpp and rocvirtual.cpp compile with
  ROCR_DYN_DLL defined.
saleelk added a commit that referenced this pull request Oct 25, 2026
…lot (#14115)

## Motivation

`hsa_amd_signal_batch_create` (#12988) can place signals in a GPU pool
the host reaches
through the large BAR. Every signal API call starts with
`Signal::Convert`, which reads the
slot's `id` (validation) and `core_signal` (the `core::Signal*` behind
the handle) before the
operation reads `amd_signal_t::value`. In device memory each of those is
an uncached PCIe
round trip, so one `hsa_signal_load_relaxed` costs three bus reads
instead of one:

| MI300X | Host signal | Device batch signal |
|---|---|---|
| raw 8-byte read of the value | 4 ns | 1.2 us |
| `hsa_signal_load_relaxed` before | 4 ns | 3.5 us |
| `hsa_signal_load_relaxed` after | 7 ns | 1.1 us |
| `hsa_signal_wait_scacquire` (satisfied) before | 42 ns | 3.4 us |
| `hsa_signal_wait_scacquire` (satisfied) after | 43 ns | 1.2 us |
| `hsa_amd_signal_wait_any` / `wait_all` (1 signal) before | 115 ns |
15-17 us |
| `hsa_amd_signal_wait_any` / `wait_all` (1 signal) after | 125-140 ns |
1.2 us |

Every wait paid the same two extra reads on entry; `wait_any`/`wait_all`
read the slot's `id`
and `core_signal` several times per signal. CLR's `hipEventSynchronize`
on a device-memory
event signal goes through `Convert` and the wait.

## Technical Details

The handle has to stay the `amd_signal_t` address (packets carry it and
the CP reads it), so
the handle-to-object lookup can't become a cast. Instead, batches whose
arena the host reaches
across the bus get a host-side copy of what `Convert` needs:

- `SignalBatch::shadow_members_`: host array of each member's `Signal*`,
filled in `Create`,
  cleared in `ReleaseSlot`.
- A page map from address to `SignalBatch*`: a three-level radix tree on
the page number
(12 bits per level, 48-bit addresses), as in tcmalloc's PageMap. Arenas
are page aligned and
own their pages, so a page belongs to at most one batch. Writers (batch
create/destroy) take
a mutex; readers take no lock and write nothing shared. Nodes are
created on first use and
  never freed, so a walk always completes.
- `Signal::Convert` walks the map first when any batch is registered and
falls back to reading
the slot for every other handle (host signals, IPC, non-batch device
memory).
- `hsa_amd_signal_wait_any/all` validate handles through the same path
  (`Signal::IsValidHandle`).

`SharedSignal` is unchanged; `core_signal` and `id` are still written
for every slot and stay
at 0x48/0x50 for IPC peers.

### Example

A batch of 40 signals whose arena lands at `0x7F12_3456_7000`. 40 x 128
bytes = 5120 bytes,
so it spans two pages. Each 36-bit page number splits into three 12-bit
indices:

| Page | Page number | Root | Mid | Leaf |
|---|---|---|---|---|
| `0x7F1234567000` | `0x7F1234567` | `0x7F1` | `0x234` | `0x567` |
| `0x7F1234568000` | `0x7F1234568` | `0x7F1` | `0x234` | `0x568` |

**Create.** Under the mutex, `root[0x7F1]` and its `leaves[0x234]` are
allocated if missing,
then both leaf entries are set to the batch:

```
root[0x7F1] -> mid
               leaves[0x234] -> leaf
                                batches[0x567] = batch
                                batches[0x568] = batch
```

**Lookup.** Member 35's handle is `0x7F1234567000 + 35 * 128 =
0x7F1234568180`:

1. Page number `0x7F1234568`: root `0x7F1`, mid `0x234`, leaf `0x568`
gives the batch.
2. Offset `0x1180` = 4480: a multiple of 128, slot 35, below the batch's
40 slots.
3. `shadow_members_[35]` is the `Signal*`. No device memory has been
read; the load itself
   then reads `amd_signal_t::value`, its one bus read.

**Misses.** A host signal at `0x55D012345040` stops at `root[0x55D] ==
nullptr` and resolves
through its slot as before. A handle past the last slot
(`0x7F1234568400`, slot 40) or off a
slot boundary also falls back.

**Destroy.** `hsa_signal_destroy` on member 35 clears
`shadow_members_[35]`; a later call on
that handle fails with `HSA_STATUS_ERROR_INVALID_SIGNAL`. Destroying the
batch clears both
leaf entries; the nodes stay for the next batch in that range.

### Behaviour change

A destroyed batch member now fails with
`HSA_STATUS_ERROR_INVALID_SIGNAL`. Before, the slot's
`id` was never cleared (`SharedSignal` is trivially destructible for
IPC), so `Convert` returned
the stale `core_signal`. Using a destroyed handle was already undefined.

## Issue Tracking

<!-- JIRA ID: -->

## Test Plan

- rocrtst `*Signal*` and IPC tests.
- Stress (300 device batches, enough to grow the map across several
leaves; 4 reader
threads resolving members while the main thread destroys and recreates
batches for 5 s).
- Per-thread host-signal load cost with 300 batches registered, 1 to 8
threads.
- CLR with device-memory signals (`DEBUG_CLR_DEV_SIGNALS=0/1`): #13818
repro, fork graph.

## Test Result

MI300X (gfx942):
- rocrtst: 15/15 pass.
- Stress: ~53k batch recreations, ~14M concurrent member loads, 0
mismatches; `wait_any` over a
  device and a host signal returns the device signal.
- Host-signal load: 6.8 ns at 1, 2, 4 and 8 threads (4 ns with no
batches registered).
- Device-signal load: 1.0-1.2 us, equal to a raw read of the value.
- Against `develop` (925f3d5), satisfied signals, 1 and 300 extra
device batches: host-signal
load +2-3 ns, wait +1-2 ns, `wait_any`/`wait_all` +10-25 ns;
device-signal load and wait
  3.3 us -> 1.1-1.2 us, `wait_any`/`wait_all` ~16 us -> ~1.2 us.
- CLR repro and fork graph pass with device signals on and off.

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
lmoriche added a commit that referenced this pull request Nov 5, 2026
rocr: add HSA_AMD_AQL_PROPERTY_SIGNAL_HINTS (1 << 1) to
hsa_amd_agent_aql_properties_t, next to
HSA_AMD_AQL_PROPERTY_EXT_DISPATCH. GetAqlInfoProperties sets it in
HSA_AMD_AGENT_INFO_AQL_EXTENSIONS when SignalHintsSupported(): the CP
firmware reports AMD_QUEUE_CAPS_CP_SIGNAL_HINTS, read once from the
utility queue's caps. As for EXT_DISPATCH, the enumerator's value is
the bit index (setFlag, hsa_flag_isset64), not a mask; the header now
documents this for the enum and the attribute.

The bit replaces HSA_AMD_AGENT_INFO_SIGNAL_HINTS_SUPPORTED (0xA129,
added in 1.36 by #12988), which is removed without an alias:
1.36-1.38 were never in a release, as the version history now says.
agent_get_info returns HSA_STATUS_ERROR_INVALID_ARGUMENT for 0xA129,
which is left unused. Interface version 1.39.

clr: the device reads signal-hint support from AQL_EXTENSIONS with
hsa_flag_isset64. With a runtime that does not report the bit, hints
stay off. Hints stay off with a profiler tool attached.

rocrtst: signal_hints and signal_hint_cost skip unless the agent
reports the bit; signal_hints checks that the bit matches the caps
the CP reports on a queue of its own.
lmoriche added a commit that referenced this pull request Nov 5, 2026
rocr: add HSA_AMD_AQL_PROPERTY_SIGNAL_HINTS (1 << 1) to
hsa_amd_agent_aql_properties_t, next to
HSA_AMD_AQL_PROPERTY_EXT_DISPATCH. GetAqlInfoProperties sets it in
HSA_AMD_AGENT_INFO_AQL_EXTENSIONS when SignalHintsSupported(): the CP
firmware reports AMD_QUEUE_CAPS_CP_SIGNAL_HINTS, read once from the
utility queue's caps. As for EXT_DISPATCH, the enumerator's value is
the bit index (setFlag, hsa_flag_isset64), not a mask; the header now
documents this for the enum and the attribute.

The bit replaces HSA_AMD_AGENT_INFO_SIGNAL_HINTS_SUPPORTED (0xA129,
added in 1.36 by #12988), which is removed without an alias:
1.36-1.38 were never in a release, as the version history now says.
agent_get_info returns HSA_STATUS_ERROR_INVALID_ARGUMENT for 0xA129,
which is left unused. Interface version 1.39.

clr: the device reads signal-hint support from AQL_EXTENSIONS with
hsa_flag_isset64. With a runtime that does not report the bit, hints
stay off. Hints stay off with a profiler tool attached.

rocrtst: signal_hints and signal_hint_cost skip unless the agent
reports the bit; signal_hints checks that the bit matches the caps
the CP reports on a queue of its own.
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.

5 participants