Repository navigation
feat(clr): batch hip graph pool signals in device memory and split HwQueueTracker - #13424
Conversation
1080d8a to
6b518ff
Compare
79063c6 to
5650633
Compare
5650633 to
4d18be7
Compare
6b518ff to
3fd7d00
Compare
4d18be7 to
de8a89a
Compare
3fd7d00 to
b9dfbbe
Compare
b9dfbbe to
d2649bc
Compare
cfreeamd
left a comment
There was a problem hiding this comment.
🤖 AI-assisted pre-review (for @cfreeamd)
The SignalBatch API wiring (AmdExtTable pointers, api_trace forwarding, ENFORCE_ABI at 96/97, MINOR 35→36, STEP 0x16→0x17), LocalSignal batch-vs-pool refcounting, UpdateValue/HDP-flush idiom, new agent/pool attributes, inline hints API, and rocrtst test coverage all look correct; one concern is that the SDMA timestamp refactor may leave amd_signal.end_ts at a 32-byte-misaligned offset on gfx7xx/gfx8xx hardware.
fcd5b5d to
42e5b1d
Compare
cfreeamd
left a comment
There was a problem hiding this comment.
🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after author responses
The rebuttal on the SDMA alignment [Important] is convincing: gfx7/gfx8 are no longer supported, so the 32-byte alignment requirement is gone and end_ts at offset 40 (8-byte aligned) is correct for gfx9+. Assert removal likewise accepted — those guards were gfx7/8-only. No remaining concerns on the ROCr side.
|
🤖 AI-assisted pre-review (for @mangupta) Re-reviewed the updated head ( |
d2649bc to
ce04fbb
Compare
42e5b1d to
157f83b
Compare
|
🤖 AI-assisted pre-review (for @mangupta) Re-reviewed the updated head ( |
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
6db9921 to
e43445a
Compare
|
🤖 AI-assisted pre-review (for @mangupta) Re-reviewed the updated head ( |
e43445a to
ec68634
Compare
…QueueTracker 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.
ec68634 to
c3699a5
Compare
|
🤖 AI-assisted pre-review (for @mangupta) Re-reviewed the updated head ( |
…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.
TheRock Submodule Bump ActivityNewest first
|
Builds on #12988 (ROCr batch signal API), now merged. This replaces #12990, which was merged into
amdSignalV2and 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.
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_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.
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.
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.
(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.
(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:
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):
(256), 1263 -> 815 us (1024); paths2 -19% to -21%; full4 up to -13%;
full2 and straight unchanged.
308 -> 284 us (reversed); 93 layers 6289 -> 5347 us and
5531 -> 5079 us; single-stream unchanged.
Test (MI300X):
multi-stream) pass with DEBUG_CLR_GRAPH_DEV_SIGNALS=0 and 1, with AQL
rings in device and in system memory.
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.
are textually identical to the code they replace.
with the flag off and on (all pre-existing).
ROCR_DYN_DLL defined.