Skip to content

Add Queue::submit() for command buffer submission#1057

Open
MarijnS95 wants to merge 1 commit intollvm:mainfrom
Traverse-Research:queue-submit
Open

Add Queue::submit() for command buffer submission#1057
MarijnS95 wants to merge 1 commit intollvm:mainfrom
Traverse-Research:queue-submit

Conversation

@MarijnS95
Copy link
Copy Markdown
Collaborator

@MarijnS95 MarijnS95 commented Apr 2, 2026

Depends on #1033

Move command buffer submission logic from each backend's Device into Queue::submit(), which takes ownership of the command buffers. Each backend uses its Fence abstraction (#1007) for GPU synchronization:

  • Metal: commit() + waitUntilCompleted()
  • Vulkan: vkQueueSubmit() signaling a timeline semaphore (VulkanFence), then VulkanFence::waitForCompletion()
  • DX12: ExecuteCommandLists() + Queue::Signal() on the queue-owned DXFence, then DXFence::waitForCompletion()

Co-Authored-By: Claude Opus 4.6 (1M context) [email protected]

@MarijnS95 MarijnS95 force-pushed the queue-submit branch 2 times, most recently from b77026f to b8bd9ac Compare April 2, 2026 14:15
Comment thread lib/API/DX/Device.cpp Outdated
@MarijnS95

This comment was marked as resolved.

@MarijnS95 MarijnS95 force-pushed the queue-submit branch 5 times, most recently from 5e4f6d2 to 3b4f680 Compare April 13, 2026 15:09
@MarijnS95 MarijnS95 force-pushed the queue-submit branch 3 times, most recently from 0ed7539 to cfb2665 Compare April 14, 2026 15:04
@MarijnS95 MarijnS95 marked this pull request as draft April 14, 2026 15:25
@MarijnS95 MarijnS95 force-pushed the queue-submit branch 4 times, most recently from bdd258f to 83e0ad9 Compare April 16, 2026 17:31
@MarijnS95 MarijnS95 marked this pull request as ready for review April 17, 2026 12:13
@MarijnS95
Copy link
Copy Markdown
Collaborator Author

This PR is in a bit of an odd interim state. It aims to at least generalize the submit+wait paths behind the common CommandBuffer abstraction, but there are still two points of contention:

  1. Each backends' InvocationState still stores a "downcast" CommandBuffer. Storing the "trait" offloadtest::CommandBuffer requires extra casting in many places, or a casting helper that Claude generated here: Traverse-Research@8ddb409.
  2. This function is subject to change pending future plans. Right now it still waits for completion, but in the near future this function should default to submitting asynchronously and instead returning the relevant fence and "to-be-signalled value".
    That however also relies on a keepalive mechanism and less strictly defined resource ownership and destruction, something that was brought up for debate previously and likely requires some more thought and confirmation before committing to that pattern.

Comment thread lib/API/DX/Device.cpp
Move command buffer submission logic from each backend's Device into
Queue::submit(), which takes ownership of the command buffers. Each
backend uses its Fence abstraction for GPU synchronization:

- Metal: commit() + waitUntilCompleted()
- Vulkan: vkQueueSubmit() signaling a timeline semaphore (VulkanFence),
  then VulkanFence::waitForCompletion()
- DX12: ExecuteCommandLists() + Queue::Signal() on the queue-owned
  DXFence, then DXFence::waitForCompletion()

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Comment thread include/API/Device.h
Comment on lines +64 to +65
virtual llvm::Error
submit(llvm::SmallVector<std::unique_ptr<CommandBuffer>> CBs) = 0;
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.

This could take a llvm::SmallVectorImpl<std::unique_ptr<CommandBuffer>> &&, which would allow it to be called regardless of the actual size of the caller's SmallVector.

Comment thread include/API/Device.h
Comment on lines +69 to +71
llvm::SmallVector<std::unique_ptr<CommandBuffer>> CBs;
CBs.push_back(std::move(CB));
return submit(std::move(CBs));
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.

If we do the above, then this SmallVector can have a fixed size of 1, rather than using a bunch of extra stack space here.

Suggested change
llvm::SmallVector<std::unique_ptr<CommandBuffer>> CBs;
CBs.push_back(std::move(CB));
return submit(std::move(CBs));
return submit(
llvm::SmallVector<std::unique_ptr<CommandBuffer>, 1>{std::move(CB)});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants