Skip to content

Remove unused pWaitDstStageMask from Vulkan submit#1034

Open
MarijnS95 wants to merge 1 commit intollvm:mainfrom
Traverse-Research:remove-pWaitDstStageMask
Open

Remove unused pWaitDstStageMask from Vulkan submit#1034
MarijnS95 wants to merge 1 commit intollvm:mainfrom
Traverse-Research:remove-pWaitDstStageMask

Conversation

@MarijnS95
Copy link
Copy Markdown
Contributor

@MarijnS95 MarijnS95 commented Mar 30, 2026

pWaitDstStageMask is only meaningful when wait semaphores are used, which this submit path does not use. This wasn't causing errors in practice because vkWaitForFences() is called after every submit, preventing concurrent in-flight command buffers.

The compute dispatch submit passed VK_PIPELINE_STAGE_TRANSFER_BIT, intending to wait for the prior buffer transfer submit to complete, but pWaitDstStageMask is a destination stage mask — it specifies which stages in the current submit to block until a wait semaphore signals, not which prior stages to wait on. Without semaphores this had no effect.

Copy link
Copy Markdown
Contributor

@manon-traverse manon-traverse left a comment

Choose a reason for hiding this comment

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

LGTM

@MarijnS95
Copy link
Copy Markdown
Contributor Author

Let my double-check why this suddenly "fixes" some tests on the Windows GPU:

Unexpectedly Passed Tests (3):
  OffloadTest-clang-vk :: Feature/StructuredBuffer/matrix.test
  OffloadTest-clang-vk :: Feature/StructuredBuffer/matrix_assign.test
  OffloadTest-clang-vk :: Feature/Textures/Texture2D.OperatorIndex.test.yaml

@MarijnS95 MarijnS95 force-pushed the remove-pWaitDstStageMask branch from d1e1277 to 095dd98 Compare April 2, 2026 18:51
@bogner
Copy link
Copy Markdown
Contributor

bogner commented Apr 6, 2026

Summary

  • Remove pWaitDstStageMask assignment from executeCommandBuffer since no wait semaphores are used
  • Remove the unused WaitMask parameter and the VK_PIPELINE_STAGE_TRANSFER_BIT argument at the call site

See commit message for details on why this was harmless but incorrect.

When putting up a PR the description should be the commit message. The git history during review inevitably collects various fixups, and burying the intended commit message there makes it hard to review the message itself.

@MarijnS95
Copy link
Copy Markdown
Contributor Author

@bogner you're right, I've configured Claude to not have the useless # Summary header especially if that's the only section, and to keep the commit message verbatim (GitHub's default anyway) for single-commit PR's, yet it still seems to create these meaningless rewrites of my original message (although I should have checked the final thing it put up). Going back to good-old gh pr create next time :)

`pWaitDstStageMask `is only meaningful when wait semaphores are used,
which this submit path does not use. This wasn't causing errors in
practice because` vkWaitForFences()` is called after every submit,
preventing concurrent in-flight command buffers. The compute dispatch
submit passed` VK_PIPELINE_STAGE_TRANSFER_BIT`, intending to wait for
the prior buffer transfer submit to complete, but `pWaitDstStageMask
`is a destination stage mask — it specifies which stages in the current
submit to block until a wait semaphore signals, not which prior stages
to wait on. Without semaphores this had no effect.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@MarijnS95 MarijnS95 force-pushed the remove-pWaitDstStageMask branch 2 times, most recently from 6f552a6 to ef175dd Compare April 7, 2026 06:30
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.

5 participants