[HLSL][SPIRV] Use 0 to represent unbounded arrays on shader flags by joaosaffran · Pull Request #187174 · llvm/llvm-project

joaosaffran · 2026-03-18T02:18:24Z

this patch updates the shader flags to account for 0 being used to represent unbounded arrays. This was a missed updated from the previous pr #186022. This change is required to make sure the following offload test pass dxv validation:

  OffloadTest-clang-d3d12 :: Feature/ResourceArrays/multi-dim-unbounded-array-nuri.test
  OffloadTest-clang-d3d12 :: Feature/ResourceArrays/multi-dim-unbounded-array.test
  OffloadTest-clang-d3d12 :: Feature/ResourceArrays/unbounded-array-nuri.test
  OffloadTest-clang-d3d12 :: Feature/ResourceArrays/unbounded-array.test

llvmbot · 2026-03-18T02:18:56Z

@llvm/pr-subscribers-backend-directx

Author: None (joaosaffran)

Changes

this patch updates the shader flags to account for 0 being used to represent unbounded arrays. This was a missed updated from the previous pr #186022

Full diff: https://github.com/llvm/llvm-project/pull/187174.diff

1 Files Affected:

(modified) llvm/lib/Target/DirectX/DXILShaderFlags.cpp (+5-3)

diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
index 50d08b3a66dc1..49cfcdf615c55 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
@@ -269,10 +269,12 @@ ModuleShaderFlags::gatherGlobalModuleFlags(const Module &M,
   // Set the Max64UAVs flag if the number of UAVs is > 8
   uint32_t NumUAVs = 0;
   for (auto &UAV : DRM.uavs())
-    if (MMDI.ValidatorVersion < VersionTuple(1, 6))
+    if (MMDI.ValidatorVersion < VersionTuple(1, 6)) {
       NumUAVs++;
-    else // MMDI.ValidatorVersion >= VersionTuple(1, 6)
-      NumUAVs += UAV.getBinding().Size;
+    } else { // MMDI.ValidatorVersion >= VersionTuple(1, 6)
+      const uint32_t Size = UAV.getBinding().Size;
+      NumUAVs += Size == 0 ? ~0u : Size;
+    }
   if (NumUAVs > 8)
     CSF.Max64UAVs = true;

bogner · 2026-03-18T03:47:15Z

-      NumUAVs += UAV.getBinding().Size;
+    } else { // MMDI.ValidatorVersion >= VersionTuple(1, 6)
+      const uint32_t Size = UAV.getBinding().Size;
+      NumUAVs += Size == 0 ? ~0u : Size;


Uh, this will overflow if NumUAVs is anything but zero coming into this. What happens if we have multiple unbounded UAVs (that is, in different spaces)? What happens if we have a bound UAV followed by an unbounded one?

I guess this is a pre-existing problem, since UAV.getBinding().Size would have been ~0U in that case anyway, but I think that this change is probably pointing out that we have a bug here.

@tex3d, do you know what DXV expects in the scenario bogner is describing?

I've updated the PR comment to make it explicit, but this issue was found in the offload test suite, some tests were not passing dxv validation. Changing the flag value to account for unbounded fixed the issue. However, I am not sure if any tests account for such scenario.

DXC/DXV adds a maximum of 9 to the count for each UAV range in an attempt to avoid overflow. It's not fool-proof, as nearly 500 million separate resource array decls of size 9 or greater could overflow it, but realistically, it would be virtually impossible to compile a shader that used that many decls.

See where this NumUAVs is gathered for DXC/DXV:
https://github.com/microsoft/DirectXShaderCompiler/blob/05de2c9fa9a4fec8e4a14b1ed605b81322680bd8/lib/DXIL/DxilModule.cpp#L319-L320

And where it's used to set the flag:
https://github.com/microsoft/DirectXShaderCompiler/blob/05de2c9fa9a4fec8e4a14b1ed605b81322680bd8/lib/DXIL/DxilModule.cpp#L333-L337

If that number is > 8 (kSmallUAVCount), it sets the flag.

You could add 9 or (kSmallUAVCount+1) when size is unbounded or > 8 like DXC does, and you could just set the flag and stop counting after NumUAVs > 8, for an easy overflow avoidance.

Saturating NumUAVs to UINT32_MAX should also do the trick

tex3d

LGTM.

bogner

Please add two tests here. One that demonstrates the case that regressed (a single unbounded array of resources), and one that demonstrates the pre-existing bug this fixes (a resource followed by an unbounded array of resources).

bogner

Looks good. A few mostly aesthetic comments about the tests (they all apply to both).

bogner · 2026-03-18T21:33:05Z

@@ -0,0 +1,41 @@
+; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s
+; RUN: llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC


I guess this is just being consistent with the other files in this directory, but I don't understand the "DXC" check prefix... these checks have nothing to do with dxc. Can we use something more sensible, like CHECK-OBJ or CHECK-LLC or something?

bogner · 2026-03-18T21:35:37Z

+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"dx.resmayalias", i32 1}


I don't think resmayalias is relevant to the test here, we should drop this.

I suspect this is because the shader flag is ResMayNotAlias, so excluding this module flag will result in this shader flag being set, which adds noise to the output being checked.

bogner · 2026-03-18T21:39:01Z

+; DXC: - Name:            SFI0
+; DXC-NEXT:     Size:            8
+; DXC-NEXT:     Flags:
+; DXC:       Max64UAVs:         true


Can we align these a bit more consistently? If we indent everything to the length of the -NEXT checks the YAML is easier to read.

Suggested change

; DXC: - Name: SFI0

; DXC-NEXT: Size: 8

; DXC-NEXT: Flags:

; DXC: Max64UAVs: true

; DXC: - Name: SFI0

; DXC-NEXT: Size: 8

; DXC-NEXT: Flags:

; DXC: Max64UAVs: false

Did you mean to invert the Max64UAVs flag in your suggestion?

bogner · 2026-03-18T21:39:34Z

+; DXC:       NextUnusedBit:   false
+; DXC: ...


I don't think these last two checks really do anything useful

llvm-ci · 2026-03-19T00:35:40Z

LLVM Buildbot has detected a new failure on builder arc-builder running on arc-worker while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/30288

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: CAS/./CASTests/20/24' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/buildbot/worker/arc-folder/build/unittests/CAS/./CASTests-LLVM-Unit-21935-20-24.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=24 GTEST_SHARD_INDEX=20 /buildbot/worker/arc-folder/build/unittests/CAS/./CASTests
--

Script:
--
/buildbot/worker/arc-folder/build/unittests/CAS/./CASTests --gtest_filter=CASProgramTest.MappedFileRegionArenaTest
--
Note: Google Test filter = CASProgramTest.MappedFileRegionArenaTest
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from CASProgramTest
/buildbot/worker/arc-folder/llvm-project/llvm/unittests/CAS/ProgramTest.cpp:148: Failure
Value of: PI.ReturnCode == 0
  Actual: false
Expected: true


/buildbot/worker/arc-folder/llvm-project/llvm/unittests/CAS/ProgramTest.cpp:148
Value of: PI.ReturnCode == 0
  Actual: false
Expected: true



********************

joaosaffran-zz added 2 commits March 17, 2026 15:31

update size to fix metadata being generated

6731843

update code formating

0b37305

llvmbot added the backend:DirectX label Mar 18, 2026

bogner reviewed Mar 18, 2026

View reviewed changes

fix overflow bug when declaring many unbounded descriptors

3734f75

joaosaffran requested review from bogner and tex3d March 18, 2026 19:22

tex3d approved these changes Mar 18, 2026

View reviewed changes

bogner reviewed Mar 18, 2026

View reviewed changes

joaosaffran added 2 commits March 18, 2026 14:12

adding tests

760f91f

update comments

d9c2488

joaosaffran requested a review from bogner March 18, 2026 21:17

bogner approved these changes Mar 18, 2026

View reviewed changes

update tests

8590cd5

joaosaffran enabled auto-merge (squash) March 18, 2026 23:54

tex3d approved these changes Mar 19, 2026

View reviewed changes

joaosaffran merged commit 8176bc0 into llvm:main Mar 19, 2026
9 of 11 checks passed

		@@ -0,0 +1,41 @@
		; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s \| FileCheck %s
		; RUN: llc %s --filetype=obj -o - \| obj2yaml \| FileCheck %s --check-prefix=DXC

		!llvm.module.flags = !{!0}
		!0 = !{i32 1, !"dx.resmayalias", i32 1}

Conversation

joaosaffran commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tex3d left a comment

Choose a reason for hiding this comment

Uh oh!

bogner left a comment

Choose a reason for hiding this comment

Uh oh!

bogner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

joaosaffran commented Mar 18, 2026 •

edited

Loading