Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 7 additions & 20 deletions projects/clr/hipamd/src/hip_graph_internal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3483,15 +3483,7 @@ GraphSignalManager::~GraphSignalManager() {
for (auto& dev_pool : free_sets_) {
amd::Device* device = dev_pool.first;
for (auto& set : dev_pool.second) {
// Pooled signals rest armed (value 1); mark them idle before destroy so
// ~ProfilingSignal does not block waiting on an armed-but-idle signal.
device->QuiesceHwEvents(set);
for (void* sig : set) {
if (sig != nullptr) {
// Pair with CreateHwEvents() so non-ROCm devices can hook teardown.
device->DestroyHwEvent(sig);
}
}
device->ReturnHwEvents(set);
}
}
free_sets_.clear();
Expand All @@ -3506,16 +3498,11 @@ bool GraphSignalManager::Prepopulate(amd::Device* device, int count, int num_set

// BuildSyncPlan is re-runnable, so Prepopulate may be called more than once.
// If a prior run sized the sets for a different segment count, those sets are
// unusable -- destroy and rebuild. No launches are in flight at (re)instantiate
// unusable -- return them and rebuild. No launches are in flight at (re)instantiate
// time, so every set for this device is present in the free pool here.
if (!pool.empty() && static_cast<int>(pool.back().size()) != count) {
for (auto& set : pool) {
device->QuiesceHwEvents(set);
for (void* sig : set) {
if (sig != nullptr) {
device->DestroyHwEvent(sig);
}
}
device->ReturnHwEvents(set);
}
pool.clear();
}
Expand All @@ -3524,7 +3511,7 @@ bool GraphSignalManager::Prepopulate(amd::Device* device, int count, int num_set
// would grow the pool without bound across re-instantiations.
for (int i = static_cast<int>(pool.size()); i < num_sets; ++i) {
std::vector<void*> set;
if (!device->CreateHwEvents(count, /*batched=*/true, set)) {
if (!device->AcquireHwEvents(count, set)) {
return false;
}
pool.push_back(std::move(set));
Expand All @@ -3550,9 +3537,9 @@ bool GraphSignalManager::AcquireSet(amd::Device* device, int count,
}
}

// Fallback only: more launches in flight than pre-created sets. Create one
// (armed to 1 by CreateHwEvents); it joins the pool when released.
return device->CreateHwEvents(count, /*batched=*/true, out_set);
// Fallback only: more launches in flight than pre-created sets. Take another
// armed set from the device; it joins this pool when released.
return device->AcquireHwEvents(count, out_set);
Comment thread
saleelk marked this conversation as resolved.
Resolved
}

void GraphSignalManager::ReleaseSet(amd::Device* device, std::vector<void*>& set) {
Expand Down
7 changes: 4 additions & 3 deletions projects/clr/hipamd/src/hip_graph_internal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,18 +208,19 @@ class GraphKernelArgManager : public amd::ReferenceCountedObject,
using KernelArgImpl = device::Settings::KernelArgImpl;
};

//! Per-GraphExecBase pool of HW event sets.
//! Per-GraphExecBase pool of HW event sets, taken from the device's pool and given back to it
//! when the graph is destroyed.
class GraphSignalManager : public amd::ReferenceCountedObject {
public:
GraphSignalManager() : amd::ReferenceCountedObject() {}
~GraphSignalManager();

//! Pre-create `num_sets` signal sets of `count` signals each at instantiate
//! Take `num_sets` signal sets of `count` signals each at instantiate
//! time, so launches never pay signal creation on the hot path.
bool Prepopulate(amd::Device* device, int count, int num_sets);

//! Acquire a ready signal set for a single launch. Pops from the free pool;
//! only creates a new set as a fallback if the pool is unexpectedly empty.
//! takes a new set from the device only if the pool is unexpectedly empty.
bool AcquireSet(amd::Device* device, int count, std::vector<void*>& out_set);

//! Re-arm a used set and return it to the free pool. Called from the launch
Expand Down
13 changes: 13 additions & 0 deletions projects/clr/rocclr/device/device.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2305,6 +2305,19 @@ class Device : public RuntimeObject {
}
virtual void DestroyHwEvent(void* hw_event) const {}

//! Takes count armed HW events for a pooled graph set
virtual bool AcquireHwEvents(int count, std::vector<void*>& hw_events) const {
return CreateHwEvents(count, /*batched=*/true, hw_events);
}
//! Gives back armed HW events from AcquireHwEvents
virtual void ReturnHwEvents(std::vector<void*>& hw_events) const {
QuiesceHwEvents(hw_events);
for (void* hw_event : hw_events) {
DestroyHwEvent(hw_event);
}
hw_events.clear();
}
Comment thread
saleelk marked this conversation as resolved.
Resolved

//! Re-arm already-allocated HW event signals so they can be reused by a new
//! graph launch (resets the signal value and cached timing). Used by the
//! graph signal pool to avoid create/destroy on every launch.
Expand Down
38 changes: 38 additions & 0 deletions projects/clr/rocclr/device/rocm/rocdevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,12 @@ Device::~Device() {
// so this is a safe no-op in that path.
WaitForHsaAsyncHandlersIdle();

QuiesceHwEvents(graph_hw_events_);
for (void* hw_event : graph_hw_events_) {
DestroyHwEvent(hw_event);
}
graph_hw_events_.clear();

// Release cached map targets
for (uint i = 0; mapCache_ != nullptr && i < mapCache_->size(); ++i) {
if ((*mapCache_)[i] != nullptr) {
Expand Down Expand Up @@ -4261,6 +4267,38 @@ void Device::DestroyHwEvent(void* hw_event) const {
ReleaseGlobalSignal(hw_event);
}

// ================================================================================================
bool Device::AcquireHwEvents(int count, std::vector<void*>& hw_events) const {
// A batch is one allocation, which the last of its signals frees with an unmap from the GPU, so
// batches cover many sets rather than one each
constexpr int kHwEventBatch = 256;
std::unique_lock lock(graph_hw_events_lock_);
// Creation allocates device memory, so other graphs must not wait on the lock meanwhile, and
// another thread may take the new signals before this one relocks
while (static_cast<int>(graph_hw_events_.size()) < count) {
const int num_missing = count - static_cast<int>(graph_hw_events_.size());
lock.unlock();
std::vector<void*> batch;
if (!CreateHwEvents(std::max(num_missing, kHwEventBatch), /*batched=*/true, batch)) {
return false;
}
lock.lock();
graph_hw_events_.insert(graph_hw_events_.end(), batch.begin(), batch.end());
}
hw_events.assign(graph_hw_events_.end() - count, graph_hw_events_.end());
graph_hw_events_.resize(graph_hw_events_.size() - count);
return true;
}
Comment thread
saleelk marked this conversation as resolved.
Resolved

// ================================================================================================
void Device::ReturnHwEvents(std::vector<void*>& hw_events) const {
// Kept rather than destroyed: a queue that waited on a signal may hold the last other reference
// and release it inside a later launch
std::scoped_lock lock(graph_hw_events_lock_);
graph_hw_events_.insert(graph_hw_events_.end(), hw_events.begin(), hw_events.end());
hw_events.clear();
}

// ================================================================================================
void Device::FlushSignalStores(hsa_signal_t last_stored, hsa_signal_value_t value) const {
if (!pcie_stores_need_flush_) {
Expand Down
5 changes: 5 additions & 0 deletions projects/clr/rocclr/device/rocm/rocdevice.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,8 @@ class Device : public NullDevice {
!amd::activity_prof::tool_attached.load(std::memory_order_relaxed);
}
virtual void DestroyHwEvent(void* hw_event) const override;
virtual bool AcquireHwEvents(int count, std::vector<void*>& hw_events) const override;
virtual void ReturnHwEvents(std::vector<void*>& hw_events) const override;
virtual void ResetHwEvents(const std::vector<void*>& hw_events) const override;
virtual void QuiesceHwEvents(const std::vector<void*>& hw_events) const override;
virtual uint8_t* CreateBarrierPacket(int num_deps) const override;
Expand Down Expand Up @@ -807,6 +809,9 @@ class Device : public NullDevice {
hsa_amd_memory_pool_t device_signal_pool_{0}; //!< Pool for signals in device memory, if any
bool graph_device_signals_ = false; //!< Graph signals live in device_signal_pool_
bool queue_device_signals_ = false; //!< Queue signals live in device_signal_pool_
//! Armed HW events that graphs take their signal sets from and give back when destroyed
mutable std::vector<void*> graph_hw_events_;
mutable std::mutex graph_hw_events_lock_;
bool pcie_stores_need_flush_ = true; //!< Stores to device memory come over PCIe through HDP
hsa_signal_t prefetch_signal_; //!< Prefetch signal, used to explicitly prefetch SVM on device
std::atomic<int> cache_state_; //!< State of cache, kUnknown/kFlushedToDevice/kFlushedToSystem
Expand Down
Loading