[SPIRV] Add -fspv-flatten-resource-array-bindings-only by l3rp · Pull Request #8705 · microsoft/DirectXShaderCompiler

Youssef Abusamra (l3rp) · 2026-07-28T02:15:28Z

Reserves 1 binding slot per array element for resource arrays matching -fspv-flatten-resource-arrays but without emitting separate SPIRV variables/bindings.
Adds CodeGenSPIRV tests covering the non optimized and -O3 cases.
The motive is to have identical bindings to the DX path for a shared binding system.

azure-pipelines · 2026-07-28T02:15:37Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Youssef Abusamra (l3rp) · 2026-07-28T02:16:50Z

@microsoft-github-policy-service agree

Gregory Roth (pow2clk)

My main confusion here is how the previous -fspv-flatten-resource-arrays flag was useful. The way the commit message for #2397 reads, it sounds like it's trying to solve this exact problem, but I don't see why flattening the arrays was necessary. Perhaps the concern was without individual variables, they might be consolidated to fill in the gaps? Is there anywhere that might happen?

The only way this is relevant to your change is if the actual solution is to remove the old behavior and replace it with this.

Gregory Roth (pow2clk) · 2026-07-29T23:26:29Z

+  array size) instead of splitting it into one variable per element. Use this
+  when you want later-declared resources to avoid overlapping the numbers an
+  array would need under DX, without requiring the array itself to be
+  split up or unrolled.


I hate to bikeshed over naming, but I don't think what this does can be described as flattening even though it shares some behavior with the flag that does.

Perhaps something like spv-dx-resource-array-binding? I'm open to other suggestions.

-fspv-flatten-resource-arrays is for when non uniform resource accesses are unsupported AFAIK, an OpSwitch with an array element per case is generated when a non compile time constant is used.
I think -fspv-dx-resource-array-binding would be less "obvious" though?

What about something that more explicitly describes what this does? -fspv-reserve-resource-array-bindings?

Damyan Pepper (damyanp) · 2026-07-30T16:48:08Z

[Auto-generated note from Damyan Pepper (@damyanp)]

This looks like a user-visible bug fix/feature change. Please add (or point to) the corresponding entry in docs/ReleaseNotes.md.

If release-note coverage is planned in a related PR (including one that hasn’t been submitted yet), please mention that plan/link so we can avoid duplicate notes.

Copilot

Pull request overview

Adds a SPIR-V option that reserves binding slots for resource-array elements without splitting arrays into separate variables.

Changes:

Adds and parses the new compiler option.
Extends SPIR-V binding allocation.
Adds documentation, release notes, and optimized/unoptimized tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`include/dxc/Support/HLSLOptions.td`	Defines the command-line option.
`include/dxc/Support/SPIRVOptions.h`	Stores the option state.
`lib/DxcSupport/HLSLOptions.cpp`	Parses and validates option availability.
`tools/clang/lib/SPIRV/DeclResultIdMapper.cpp`	Reserves array-sized binding ranges.
`tools/clang/test/CodeGenSPIRV/vk.binding.cl.flatten-array-bindings-only.example1.hlsl`	Tests unoptimized binding allocation.
`tools/clang/test/CodeGenSPIRV/vk.binding.cl.flatten-array-bindings-only.example1-optimized.hlsl`	Tests optimized output without array splitting.
`docs/SPIR-V.rst`	Documents behavior and usage.
`docs/ReleaseNotes.md`	Announces the option.

Suppressed comments (1)

tools/clang/test/CodeGenSPIRV/vk.binding.cl.flatten-array-bindings-only.example1-optimized.hlsl:18

[unroll] is attached to the main declaration here, but this attribute only applies to loop statements, so the compiler diagnoses/ignores it and the loop is not explicitly unrolled. Move it immediately before the for statement.

[unroll]

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

tools/clang/test/CodeGenSPIRV/vk.binding.cl.flatten-array-bindings-only.example1-optimized.hlsl:22

[unroll] is attached to main here, not to the loop, so it is ignored and produces an attribute warning instead of marking the loop. Move it immediately before the for statement.

[unroll]

Diego Novillo (dnovillo) · 2026-09-15T14:07:34Z

+// CHECK: OpDecorate %NextTexture Binding 6
+// CHECK: OpDecorate %MySamplers Binding 7
+// CHECK-NOT: MyTextures_0_
+// CHECK-NOT: MySamplers_0_


These CHECK-NOTs will not trigger here. Moving them before the CHECKs is probably the easiest way to handle them.

- Reserves 1 binding slot per array element for resource arrays matching -fspv-flatten-resource-arrays but without emitting separate SPIRV variables/bindings. - Adds CodeGenSPIRV tests covering the non optimized and -O3 cases.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…nded arrays - Rename -fspv-flatten-resource-array-bindings-only to -fspv-reserve-resource-array-bindings, since the option does not flatten anything; it only reserves one binding per array element. - Return a single binding for unbounded (IncompleteArrayType) resource arrays in getNumBindingsUsedByResourceType instead of asserting, and add a regression test. - Fix the optimized test: attach [unroll] to the loop instead of main and move the CHECK-NOTs ahead of the CHECKs so they actually apply. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Typedef arrays under-reserve bindings, and implicit-array reservation lacks effective regression coverage.

2 open issues

2 resolved since last review

This path now crashes for explicitly bound unbounded resource arrays. Their AST type is…
The implicit-binding pass has the same crash for unbounded resource arrays: enabling the new option…

_{Review effort: Balanced}

…tests - Canonicalize the type in getNumBindingsUsedByResourceType so arrays declared through a typedef reserve one binding per element instead of being treated as a single resource. - Add a typedef regression test covering explicitly and implicitly bound typedef'd arrays. - Add a trailing implicitly bound texture to the example tests so under-reservation in the implicit-binding path is caught. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

The new counting path can crash compilation for non-resource inline-SPIR-V globals.

1 open issue

Avoid resource-only assertion for non-resource globals · New

2 resolved since last review

_{Review effort: Balanced}

createExternVar registers every extern global in resourceVars, including non-resources such as variables with [[vk::ext_storage_class]]. With -fspv-reserve-resource-array-bindings these reached getNumBindingsUsedByResourceType and tripped its resource-only assertion. Return a single binding for such types instead, and add a regression test. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Implicit runtime arrays can still receive bindings that differ from DXIL, undermining shared binding layouts.

1 open issue

Allocate implicit runtime arrays after the last reserved binding · New

1 resolved since last review

Avoid resource-only assertion for non-resource globals

_{Review effort: Balanced}

+  if (type->isIncompleteArrayType())
+    return arrayFactor;


github-project-automation Bot added this to HLSL Roadmap Jul 28, 2026

github-project-automation Bot moved this to New in HLSL Roadmap Jul 28, 2026

Gregory Roth (pow2clk) reviewed Jul 29, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings August 9, 2026 04:46

Copilot started reviewing on behalf of Youssef Abusamra (l3rp) August 9, 2026 04:47

View session

Copilot AI reviewed Aug 9, 2026

View reviewed changes

Comment thread tools/clang/test/CodeGenSPIRV/vk.binding.cl.flatten-array-bindings-only.example1.hlsl Outdated

Comment thread tools/clang/test/CodeGenSPIRV/vk.binding.cl.flatten-array-bindings-only.example1-optimized.hlsl Outdated

Copilot AI review requested due to automatic review settings August 13, 2026 16:01

Copilot started reviewing on behalf of Youssef Abusamra (l3rp) August 13, 2026 16:01

View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Comment thread tools/clang/lib/SPIRV/DeclResultIdMapper.cpp Outdated

Comment thread tools/clang/lib/SPIRV/DeclResultIdMapper.cpp Outdated

Diego Novillo (dnovillo) added the spirv Work related to SPIR-V label Sep 11, 2026

Diego Novillo (dnovillo) reviewed Sep 15, 2026

View reviewed changes

YoussefAbusamra and others added 4 commits October 17, 2026 12:29

[SPIRV] Add -fspv-flatten-resource-array-bindings-only

10ee56e

- Reserves 1 binding slot per array element for resource arrays matching -fspv-flatten-resource-arrays but without emitting separate SPIRV variables/bindings. - Adds CodeGenSPIRV tests covering the non optimized and -O3 cases.

Add to release notes

af1f1aa

Apply suggestions from code review

295306f

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI balanced review requested due to automatic review settings October 17, 2026 09:31

Youssef Abusamra (l3rp) force-pushed the spv-array-flatten-binding-only branch from 2e81951 to d3c470b Compare October 17, 2026 09:31

Copilot started reviewing on behalf of Youssef Abusamra (l3rp) October 17, 2026 09:32

View session

Copilot AI reviewed Oct 17, 2026

View reviewed changes

Comment thread tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

Comment thread tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.example1.hlsl Outdated

Copilot AI balanced review requested due to automatic review settings October 17, 2026 09:43

Copilot started reviewing on behalf of Youssef Abusamra (l3rp) October 17, 2026 09:43

View session

Copilot AI reviewed Oct 17, 2026

View reviewed changes

Comment thread tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

Copilot AI balanced review requested due to automatic review settings October 17, 2026 14:55

Copilot started reviewing on behalf of Youssef Abusamra (l3rp) October 17, 2026 14:56

View session

Copilot AI reviewed Oct 17, 2026

View reviewed changes

Comment thread tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

Comment on lines +163 to +164

if (type->isIncompleteArrayType())

return arrayFactor;

Conversation

Youssef Abusamra (l3rp) commented Jul 28, 2026

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

Youssef Abusamra (l3rp) commented Jul 28, 2026

Uh oh!

Gregory Roth (pow2clk) left a comment

Choose a reason for hiding this comment

Uh oh!

Gregory Roth (pow2clk) Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Youssef Abusamra (l3rp) Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Diego Novillo (dnovillo) Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

Damyan Pepper (damyanp) commented Jul 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Diego Novillo (dnovillo) Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants