Sitelet https://github.com/o3de/o3de/pull/20107
Skip to content

DX12: give each shader entry point its own intermediate files - #20107

Open
Grimwarrior wants to merge 1 commit into
o3de:developmentfrom
Grimwarrior:Shader_Compile_Overlap
Open

Grimwarrior wants to merge 1 commit into
o3de:developmentfrom
Grimwarrior:Shader_Compile_Overlap

Conversation

@Grimwarrior

@Grimwarrior Grimwarrior commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes three problems in how a shader's entry points are compiled. All three are present when stages
compile one after another, which is how the builder runs them today, and this PR keeps it that way.

1. DX12 intermediates collided across entry points

CompilePlatformInternal derived every intermediate file name from the source file alone. Every entry
point of a shader compiles from the same source into the same temp folder, so each stage wrote the same
files:

  • <stem>.dxil.bin and <stem>.dxil.txt
  • the prepended source
  • with specialization constants, <stem>.dxil.patched.bin and <stem>.offsets.json

Each stage overwrote what the previous one wrote, so only one stage's intermediates survived. It also
means a tool calling the DX12 interface directly cannot compile two stages of one shader at the same
time, because they write to the same paths.

Intermediate files are now named per entry point (<stem>.<entry>.dxil.bin and so on), and the prepended
source gets the same suffix. RayTracing has no entry point and uses the profile name instead, the same
way the .pdb name was already made unique.

2. Only the last stage's byproducts reached the caller

ShaderVariantAssetBuilder called outputByproducts.emplace() once per entry point. outputByproducts
is an AZStd::optional, and emplace on an optional replaces its current value, so every stage
discarded the previous stage's intermediate paths. The byproducts are now merged.

3. Entry point order depended on hash order

Entry points were iterated in unordered_map order. Which stage ran last, and therefore which dynamic
branch count was reported, depended on the hash rather than on the shader. Entry points are now compiled
in name order.

Not in this PR: concurrent stage compilation

An earlier revision compiled entry points in parallel in ShaderVariantAssetBuilder. That code is
shared by every backend, and only DX12 was made safe:

  • Vulkan names <stem>.spirv.bin, <stem>.spirv.txt and its prepended source from the source file
    alone, so its stages collide in the same way DX12's did.
  • Metal has the same file name collision (.spirv, .metal, .air, .metallib). It also keeps
    per-compile state on its ShaderPlatformInterface (m_srgLayouts, m_argBufferEntries), which is
    written during compilation.
  • RHI::PrependFile lazily initializes a function-local static without synchronization. This is
    shared by all backends, including DX12.

Concurrent stage compilation can come back in its own PR once every backend is safe.

Testing

  • DX12: shaders with vertex and pixel entry points process, and the intermediate files in the temp
    folder are named per entry point.
  • Specialization constants: the per-stage .dxil.patched.bin and .offsets.json are produced and the
    shader renders correctly.
  • Vulkan and Metal: unchanged by this PR apart from the entry-point ordering and byproduct merge in the
    shared builder.

@Grimwarrior
Grimwarrior requested review from a team as code owners September 9, 2026 03:03
@Grimwarrior
Grimwarrior force-pushed the Shader_Compile_Overlap branch from 927381e to f17473a Compare September 9, 2026 03:04

@EnterTheArcane EnterTheArcane 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.

Thanks for the work here. I'm going to block this for now because the common shader builder implements parallelization, but only DX12 is safe for concurrent stage compilation. Vulkan and Metal still reuse temporary files and shared state between stages. I reproduced a Vulkan file-open race during asset processing, and Metal has similar thread-safety concerns. I think this should only land once concurrent compilation is safe and supported across all backends.

…p every stage's byproducts

DX12's CompilePlatformInternal derived every intermediate name from the source
file alone, so each entry point of a shader wrote the same files in the same
temp folder: <stem>.dxil.bin, <stem>.dxil.txt, the prepended source, and with
specialization constants <stem>.dxil.patched.bin and <stem>.offsets.json. Each
stage overwrote what the previous stage wrote, so only one stage's
intermediates survived, and a caller driving the DX12 interface directly could
not compile two stages of one shader at once. Intermediates are now named per
entry point, falling back to the profile name for RayTracing, which has no
entry point.

ShaderVariantAssetBuilder called outputByproducts.emplace() once per entry
point. outputByproducts is an optional, and emplace on an optional replaces its
value, so only the last stage's byproducts reached the caller. They are now
merged.

The builder iterated entry points in unordered_map order, so which stage ran
last, and with it the reported dynamic branch count, depended on hash order.
Entry points are now compiled in name order.

Stage compilation in the builder stays sequential. Vulkan and Metal still share
intermediate file names between stages, and Metal keeps per-compile state on
its ShaderPlatformInterface, so concurrent stage compilation is not yet safe
for every backend.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Grimwarrior <143121582+Grimwarrior@users.noreply.github.com>
@Grimwarrior
Grimwarrior force-pushed the Shader_Compile_Overlap branch from f17473a to c8c71dc Compare September 15, 2026 15:59
@Grimwarrior

Copy link
Copy Markdown
Contributor Author

Thanks for the work here. I'm going to block this for now because the common shader builder implements parallelization, but only DX12 is safe for concurrent stage compilation. Vulkan and Metal still reuse temporary files and shared state between stages. I reproduced a Vulkan file-open race during asset processing, and Metal has similar thread-safety concerns. I think this should only land once concurrent compilation is safe and supported across all backends.

Hello, so is it good now ? I did some changes

@byrcolin byrcolin added the sig/graphics-audio Categorizes an issue or PR as relevant to SIG graphics-audio. label Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sig/graphics-audio Categorizes an issue or PR as relevant to SIG graphics-audio.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants