Skip to content

Move resource creation, readbacks, and InvocationState from DX backend to shared file.#1330

Merged
manon-traverse merged 3 commits into
llvm:mainfrom
Traverse-Research:move-chunks-out-of-dx12-to-shared-files
Jun 25, 2026
Merged

Move resource creation, readbacks, and InvocationState from DX backend to shared file.#1330
manon-traverse merged 3 commits into
llvm:mainfrom
Traverse-Research:move-chunks-out-of-dx12-to-shared-files

Conversation

@manon-traverse

Copy link
Copy Markdown
Contributor

Move code that doesn't need to live in the DX backend to a shared file.

Why make this change?

  1. All this code should eventually live the offloader application. However, it can't live there right now because the rendering backends are too divergent.
  2. In a follow-up PR we can reuse this code in the Vulkan and Metal backends resulting in a very significant reduction in code size.

This PR doesn't make any other code changes other than splitting the creation of the descriptor tables into a separate function from resource creation, due to no abstraction existing for building descriptor sets/tables and binding those yet.

@manon-traverse manon-traverse force-pushed the move-chunks-out-of-dx12-to-shared-files branch from 2514b33 to beeaa3d Compare June 24, 2026 13:52

@bogner bogner left a comment

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.

Will this stay in Support/ long term, or is the intent to eventually move it again once everything is migrated?

Overall LGTM on the assumption that the follow ups are where doing this will really shine.

Comment on lines -63 to -64
# Unimplemented: Clang + DX: https://github.com/llvm/llvm-project/issues/101558
# XFAIL: DirectX

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.

Shouldn't this change be NFC? Why does this start passing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This stems from the fact that DX12 already supported mip levels, but the mip levels were forced to 1. By using the mip level count from the Resource instead of clamping it this test started succeeding.

@manon-traverse

Copy link
Copy Markdown
Contributor Author

Will this stay in Support/ long term, or is the intent to eventually move it again once everything is migrated?

Overall LGTM on the assumption that the follow ups are where doing this will really shine.

The eventual goal is that all of this will live in the offload application, since this is all just code that is specific to running a .test file, i.e. what the offloader is for.

@manon-traverse

Copy link
Copy Markdown
Contributor Author

This PR is only failing on irrelevant tests due to a bug in the CI system itself. Safe to merge.

@manon-traverse manon-traverse merged commit 59dede8 into llvm:main Jun 25, 2026
41 of 51 checks passed
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.

2 participants