Add API to create queue in device memory #284
Open
+340
−214
Conversation
This file contains 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
|
FYI @benvanik this is still WIP, but let me know if this works for you and if the cacheable flag works/has benefits. |
dayatsin-amd
requested changes
Jan 23, 2025
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
dayatsin-amd
force-pushed
the
amd-staging
branch
from
ea66d58 to
9971e7b
Compare
atgutier
force-pushed
the
atgutier/queue-struct-dev-mem
branch
from
76a3db2 to
fe3307c
Compare
atgutier
commented
Jan 24, 2025
| @@ -1272,7 +1272,7 @@ void BlitKernel::PopulateQueue(uint64_t index, uint64_t code_handle, void* args, | |||
| std::atomic_thread_fence(std::memory_order_acquire); | |||
| queue_buffer[index & queue_bitmask_] = packet; | |||
| std::atomic_thread_fence(std::memory_order_release); | |||
| if (core::Runtime::runtime_singleton_->flag().dev_mem_queue() && !queue_->needsPcieOrdering()) { | |||
| if (queue_->IsDeviceMem() && !queue_->needsPcieOrdering()) { | |||
There was a problem hiding this comment.
@saleelk I'm curious if the logic is correct for the original needsPcieOrdering() method you added. Shouldn't this really be:
queue_->needsPcieOrdering()
Meaning we need to change the logic of that call internally?
atgutier
force-pushed
the
atgutier/queue-struct-dev-mem
branch
from
fe3307c to
6bbccb0
Compare
atgutier
force-pushed
the
atgutier/queue-struct-dev-mem
branch
from
6bbccb0 to
436b64a
Compare
dayatsin-amd
approved these changes
Jan 28, 2025
atgutier
force-pushed
the
atgutier/queue-struct-dev-mem
branch
8 times, most recently
from
1825907 to
51dc622
Compare
This builds on a prior change that allowed for allocating a user-mode queue's packet buffer in device memory to also allocate the queue struct in device memory. This provides additional latency benefits particularly for cases where dispatches are performed from the GPU itself. An AMD extension API for queue creation is added to specify if the queue should be created in device memory and whether or not it should be cacheable. This is for use by higher level libraries when doing queue creation. This provides the added benefit of allowing device memory to be used on a per-queue basis.
atgutier
force-pushed
the
atgutier/queue-struct-dev-mem
branch
from
51dc622 to
4533c8d
Compare
saleelk
reviewed
Jan 29, 2025
| * The queue packet buffer and the queue struct should be allocated in | ||
| * the agent's device memory. | ||
| */ | ||
| HSA_AMD_QUEUE_FLAG_DEVICE_MEM = (1 << 0), |
There was a problem hiding this comment.
We should explicitly mention whether its cached on uncached. Like HSA_AMD_QUEUE_HOST = 0, HSA_AMD_QUEUE_DEV_UNCACHED = 1 << 0,
saleelk
reviewed
Jan 29, 2025
| * Used to indicate if the queue created in device memory should be | ||
| * cacheable. | ||
| */ | ||
| HSA_AMD_QUEUE_FLAG_CACHEABLE = (1 << 1), |
There was a problem hiding this comment.
We should probably discuss if this is even doable at the moment, to flush L2 is costly and if we are writing a packet (aka 64bytes), we can probably also have false sharing issues/
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Adds an API to create a user-mode queue in device memory.
Now the queue struct is also allocated in device memory.
A flag is added to the API to specify whether or not the memory should be cacheable.