Skip to content

Signal when SPIR-V legalization needs targeted cleanup#8283

Open
AnastaZIuk wants to merge 2 commits intomicrosoft:mainfrom
Devsh-Graphics-Programming:unroll
Open

Signal when SPIR-V legalization needs targeted cleanup#8283
AnastaZIuk wants to merge 2 commits intomicrosoft:mainfrom
Devsh-Graphics-Programming:unroll

Conversation

@AnastaZIuk
Copy link
Contributor

@AnastaZIuk AnastaZIuk commented Mar 20, 2026

Summary

  • add dedicated SpirvEmitter bits for legalization-time loop unroll and legalization-time SSA rewrite
  • set those bits only for the lowering paths that still need those transforms for correctness
  • pass those signals into the narrowed SPIR-V legalization overload
  • update external/SPIRV-Headers and external/SPIRV-Tools submodule pointers to the matching branch state
  • adapt DXC to the current SPIRV-Tools legalization API and the current MSVC getFunctionType signature
  • fix vector splat of spec constants so specialization-constant composites emit the legal instruction form
  • update 7 CodeGenSPIRV checks to match the accepted SPIR-V shape

Root cause

The expensive blanket policy lives in the SPIR-V optimizer recipe, not in HLSL parsing or the DXC CLI front-end.

DXC is still the producer that knows when a specific lowering path genuinely needs those expensive legalization transforms for correctness, so the optimizer should not have to guess that globally.

Two confirmed correctness-sensitive cases are:

  • variable image sample offsets, where a producer-side signal is still needed for lowerings that must end in constant-image-operand forms such as ConstOffset
  • combined image sampler conversion, which does not need loop unroll but still needs legalize-time SSA rewrite cleanup to avoid invalid SPIR-V such as storing sampled image types

This branch also fixes one separate producer-side correctness bug: vector splat of bool spec constants was being emitted as OpConstantComposite instead of the legal OpSpecConstantComposite.

The same narrowing also exposed one existing image-atomic cleanup dependency. The matching LocalSingleStoreElim follow-up lives in the companion KhronosGroup/SPIRV-Tools#6612 branch and is rolled here through the updated submodule pointer.

Spec basis

The hard legality rules here are much narrower than the old blanket cleanup.

Image, sampler, and sampled image objects must not appear as operands to OpPhi instructions, or OpSelect instructions, or any instructions other than the image or sampler instructions specified to operate on them.

All OpSampledImage instructions must be in the same block in which their Result <id> are consumed.

Spec:

The involved types are explicitly opaque:

This type is opaque: values of this type have no defined physical size or bit pattern.

Spec:

For the image-offset case, the relevant SPIR-V rule is also explicit:

ConstOffset ... It must be an <id> of a constant instruction

Spec:

For the retained-loop case, OpSelect is also explicitly allowed on pointer-typed objects:

Before version 1.4, Result Type must be a pointer, scalar, or vector.

The types of Object 1 and Object 2 must be the same as Result Type.

Spec:

For the image-atomic follow-up, OpImageTexelPointer is also explicit:

Image must have a type of OpTypePointer with Type OpTypeImage.

Spec:

For the spec-constant splat fix, the composite rules are also explicit:

The Constituents must all be <id>s of non-specialization constant-instruction declarations or an OpUndef.

The Constituents must be the <id> of other specialization constants, constant declarations, or an OpUndef.

Spec:

Vulkan tightens this further:

Objects of types OpTypeImage, OpTypeSampler, OpTypeSampledImage ... must not be stored to or modified

If the DescriptorHeapEXT capability is not declared, structure types must not contain opaque types

Spec:

Benchmark

Validation

  • fresh full local CodeGenSPIRV on this branch pair passes with 1438 expected passes, 2 expected failures, and 0 unexpected failures

Updated test notes

  • vk.spec-constant.reuse.hlsl: the old check expected OpConstantComposite from a bool spec-constant splat. That shape is invalid. This branch now emits the legal OpSpecConstantComposite.
  • 15-loop-var-unroll-ok.hlsl: the old check implicitly depended on eager optimizer-side full unroll. This branch keeps LoopControl::Unroll in the IR and uses a legal pointer OpSelect path instead.
  • cast.flat-conversion.matrix.hlsl: the old check assumed the matrix-to-struct path stayed as pre-folded constant composites. This branch rebuilds the target values from the loaded matrix via OpCompositeExtract and OpCompositeConstruct. The stored value is the same and the row-major or column-major decoration still carries the memory layout rule.
  • type.rwstructured-buffer.array.counter.indirect.hlsl and type.rwstructured-buffer.array.counter.indirect2.hlsl: the old checks assumed a single flattened access chain directly to the counter or data element. This branch keeps an intermediate structured access chain and then indexes the member. The resource, binding, atomic, and data access semantics are unchanged.
  • vk.binding.global-struct-of-resources.optimized.hlsl: the old regex overconstrained generated ids. This branch preserves the same resource names and bindings but uses different legal ids, so the test now checks the stable semantic parts instead.
  • max_id.hlsl: the old check overconstrained an exact Bound value. This branch still exercises the same larger-limit success path, but the exact bound is no longer stable after the accepted optimizer recipe changes.

Companion SPIR-V optimizer PR:
KhronosGroup/SPIRV-Tools#6612

@github-actions
Copy link
Contributor

github-actions bot commented Mar 20, 2026

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 54afd2c6a5f1a5bcbef66967515fa69f5a3650fd 9ac57982725c4264348b20ad3fb271acc0993754 -- tools/clang/lib/Frontend/Rewrite/RewriteObjC.cpp tools/clang/lib/SPIRV/SpirvEmitter.cpp tools/clang/lib/SPIRV/SpirvEmitter.h
View the diff from clang-format here.
diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
index 4a616a00..0920fc1c 100644
--- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp
+++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
@@ -593,8 +593,7 @@ SpirvEmitter::SpirvEmitter(CompilerInstance &ci)
       constEvaluator(astContext, spvBuilder), entryFunction(nullptr),
       curFunction(nullptr), curThis(nullptr), seenPushConstantAt(),
       isSpecConstantMode(false), needsLegalization(false),
-      needsLegalizationLoopUnroll(false),
-      needsLegalizationSsaRewrite(false),
+      needsLegalizationLoopUnroll(false), needsLegalizationSsaRewrite(false),
       beforeHlslLegalization(false), mainSourceFile(nullptr) {
 
   // Get ShaderModel from command line hlsl profile option.
@@ -957,8 +956,7 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) {
       !dsetbindingsToCombineImageSampler.empty() ||
       spirvOptions.signaturePacking;
   needsLegalizationSsaRewrite =
-      needsLegalizationSsaRewrite ||
-      !dsetbindingsToCombineImageSampler.empty();
+      needsLegalizationSsaRewrite || !dsetbindingsToCombineImageSampler.empty();
 
   // Run legalization passes
   if (spirvOptions.codeGenHighLevel) {
@@ -16679,9 +16677,9 @@ bool SpirvEmitter::spirvToolsLegalize(std::vector<uint32_t> *mod,
   } else if (needsLegalizationSsaRewrite) {
     legalizationSsaRewriteMode = spvtools::SSARewriteMode::OpaqueOnly;
   }
-  optimizer.RegisterLegalizationPasses(
-      spirvOptions.preserveInterface, needsLegalizationLoopUnroll,
-      legalizationSsaRewriteMode);
+  optimizer.RegisterLegalizationPasses(spirvOptions.preserveInterface,
+                                       needsLegalizationLoopUnroll,
+                                       legalizationSsaRewriteMode);
   // Add flattening of resources if needed.
   if (spirvOptions.flattenResourceArrays) {
     optimizer.RegisterPass(
  • Check this box to apply formatting changes to this branch.

@AnastaZIuk AnastaZIuk marked this pull request as ready for review March 20, 2026 18:18
@AnastaZIuk AnastaZIuk changed the title Signal when SPIR-V legalization needs loop unroll Signal when SPIR-V legalization needs targeted cleanup Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

1 participant