Prevent DXC from adding Geometry or MultiViewport when the shader already declares one of these capabilities by aclysma · Pull Request #9076 · microsoft/DirectXShaderCompiler

Philip Degarmo (aclysma) · 2026-10-28T05:37:04Z

I stumbled across an issue with combining PrimitiveId usage with mesh shaders when the Vulkan device does NOT have geometry shaders enabled.

DXC adds OpCapability Geometry to every pixel or mesh shader that reads or writes SV_PrimitiveID or SV_RenderTargetArrayIndex, and OpCapability MultiViewport for SV_ViewportArrayIndex. Geometry requires geometry shaders to be enabled on the device.

Other capabilities also cover these values. Mesh shading covers all three (VUID-PrimitiveId-Fragment-04333 for primitive ID). ShaderViewportIndexLayerEXT, ShaderLayer and ShaderViewportIndex cover the other two.

This change stops DXC from adding Geometry or MultiViewport when the shader already declares one of these capabilities. Mesh shaders never get it. A pixel shader gets it only if it declares none of them (for example, it can declare mesh shading with [[vk::ext_capability(5283)]] and [[vk::ext_extension("SPV_EXT_mesh_shader")]] on the entry point). A plain pixel shader is unchanged.

…ds or writes SV_PrimitiveID or SV_RenderTargetArrayIndex, and OpCapability MultiViewport for SV_ViewportArrayIndex. Geometry requires geometry shaders to be enabled on the device. Other capabilities also cover these values. Mesh shading covers all three (VUID-PrimitiveId-Fragment-04333 for primitive ID). ShaderViewportIndexLayerEXT, ShaderLayer and ShaderViewportIndex cover the other two. This change stops DXC from adding Geometry or MultiViewport when the shader already declares one of these capabilities. Mesh shaders never get it. A pixel shader gets it only if it declares none of them (for example, it can declare mesh shading with [[vk::ext_capability(5283)]] and [[vk::ext_extension("SPV_EXT_mesh_shader")]] on the entry point). A plain pixel shader is unchanged.

azure-pipelines · 2026-10-28T05:37:16Z

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

Copilot

🟡 Changes recommended

The user-visible fix removing unnecessary Geometry/MultiViewport capabilities lacks the release-note entry required by the contribution policy.

1 open issue

Add release note for removing unsupported geometry shader capabilities · New

What changed in this PR

This PR updates DXC’s SPIR-V capability selection to avoid unnecessary geometry-shader requirements when declared capabilities already support the affected mesh and fragment shader builtins.

Changes:

Defers fragment builtin capability selection until all module capabilities are known.
Uses existing mesh, tessellation, and viewport/layer capabilities instead of adding redundant Geometry or MultiViewport.
Adds regression tests for alternative capabilities, late capability declarations, and preservation of geometry-stage requirements.

File	Description
`tools/clang/test/CodeGenSPIRV/semantic.viewport-array-index.shader-viewport-index.ps.hlsl`	Tests `ShaderViewportIndex` as an alternative capability.
`tools/clang/test/CodeGenSPIRV/semantic.viewport-array-index.shader-viewport-index-layer.ps.hlsl`	Tests viewport-index support through the layer extension.
`tools/clang/test/CodeGenSPIRV/semantic.viewport-array-index.mesh-shading-ext.ps.hlsl`	Tests fragment viewport-index support through mesh shading.
`tools/clang/test/CodeGenSPIRV/semantic.render-target-array-index.shader-viewport-index-layer.ps.hlsl`	Tests layer support through the viewport/layer extension.
`tools/clang/test/CodeGenSPIRV/semantic.render-target-array-index.shader-layer.ps.hlsl`	Tests `ShaderLayer` as an alternative capability.
`tools/clang/test/CodeGenSPIRV/semantic.render-target-array-index.mesh-shading-ext.ps.hlsl`	Tests fragment layer support through mesh shading.
`tools/clang/test/CodeGenSPIRV/semantic.primitive-id.mesh-shading-nv.ps.hlsl`	Tests fragment primitive IDs with NV mesh shading.
`tools/clang/test/CodeGenSPIRV/semantic.primitive-id.mesh-shading-ext.ps.hlsl`	Tests EXT mesh shading and late capability declarations.
`tools/clang/test/CodeGenSPIRV/semantic.primitive-id.mesh-shading-ext.gs.hlsl`	Verifies geometry stages retain `Geometry`.
`tools/clang/test/CodeGenSPIRV/meshshading.nv.primitive-builtins.mesh.hlsl`	Tests NV mesh primitive outputs without redundant capabilities.
`tools/clang/test/CodeGenSPIRV/meshshading.ext.primitive-builtins.mesh.hlsl`	Tests EXT mesh primitive outputs without redundant capabilities.
`tools/clang/lib/SPIRV/CapabilityVisitor.h`	Declares deferred capability selection and pending builtin storage.
`tools/clang/lib/SPIRV/CapabilityVisitor.cpp`	Implements capability reuse and deferred fragment handling.

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The added fragment-shader tests bypass the Tessellation exception, leaving it without regression coverage verifying that Tessellation is retained and Geometry is absent both with and without -fcgl.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Add pixel-shader regression test for Tessellation capability handling

tools/clang/lib/SPIRV/CapabilityVisitor.cpp:919

The new Tessellation exception is not covered by the added fragment-shader tests, which declare mesh-shading capabilities and bypass this branch. Please add an SV_PrimitiveID pixel-shader regression declaring [[vk::ext_capability(3)]] that checks Tessellation is retained and Geometry is absent, both with and without -fcgl.

1 resolved since last review

Add release note for removing unsupported geometry shader capabilities

_{Review effort: Balanced}

Copilot

🟢 Approval recommended

No active review issues are reported for the SPIR-V capability-selection changes.

No open issues 🎉

_{Review effort: Balanced}

Philip Degarmo (aclysma) · 2026-10-28T23:54:56Z

I received an email that indicated there was a comment on this PR. I don't see the comment here, but maybe GitHub is just broken right now. It asked for more info and a godbolt link.

I expected to be able to compile and use a mesh shader with vulkan that writes to PrimitiveId, without enabling geometry shaders on the device. This change fixes that specific problem and a few similar problems.

The root cause is that DXC puts unnecessary capabilities in the spirv, which limits the conditions under which the shader can be used. There are several examples in the included tests, but here is a specific case of a mesh shader that writes SV_PrimitiveID, and it results in being unable to use it without enabling geometry shaders.

https://godbolt.org/z/1oYoz4P8s

The produced spirv has "OpCapability Geometry". In Vulkan, a module that declares Geometry is invalid unless the device enables the geometryShader feature. So this mesh shader, as compiled by DXC today, can't be used on a device without geometry shader support.

The capability is unnecessary because the spec (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#BuiltIn) states that the MeshShadingEXT capability is sufficient to make PrimitiveId available.

Jonathan Zakharov (jzakharovnv) · 2026-10-29T00:09:51Z

Philip Degarmo (@aclysma) Yeah that was me deleting my message after I had understood a little better and started reviewing (was having a brain fart). If for this PR and future ones, you could file an issue with a body just like you've done here, that would be greatly appreciated and make reviewing easier and quicker. Otherwise, thanks for this PR, I'm still reviewing but it looks good so far!

Jonathan Zakharov (jzakharovnv)

Thanks for this PR and thanks for your patience. It took me a while to get context, and I still might have gaps in my understanding, but the code looks pretty good. I just have some nits and follow ups on documentation such that it best reflects the new changes.

… add a footnote in SPIR-V that describes the behavior this change introduces.

Copilot

🟢 Approval recommended

No active review issues block the change to avoid redundant SPIR-V capability requirements.

No open issues 🎉

_{Review effort: Balanced}

Philip Degarmo (aclysma) · 2026-10-30T21:59:25Z

Thanks for taking a look. I think this revision addresses all of the feedback and has significantly more readable comment/doc changes.

Jonathan Zakharov (jzakharovnv) · 2026-11-02T15:40:15Z

Thanks for the changes. One small, optional idea: a test where a capability is declared that doesn't enable the builtin (e.g. ShaderLayer with SV_PrimitiveID), to assert that Geometry is still added. Otherwise LGTM!

Gregory Roth (pow2clk)

Nothing worth holding up the change, just some better potential clarity and test consolidation.

In the meantime, the merge conflicts need to be resolved. I can't even see what they are I think because you didn't allow maintainers to push to your branch. No shame in that, but it means I can't do anything to merge it until they are resolved.

Gregory Roth (pow2clk) · 2026-11-03T15:27:26Z

+.. [1] ``Geometry`` or ``MultiViewport`` is not added if the module declares
+   one of these capabilities, which also enable the builtin:


Just a little more suggested detail

Suggested change

.. [1] ``Geometry`` or ``MultiViewport`` is not added if the module declares

one of these capabilities, which also enable the builtin:

.. [1] When the following semantic values with a PSIn Signature point,

``Geometry`` or ``MultiViewport`` is not added if the module declares

one of these capabilities, which also enable the builtin:

Gregory Roth (pow2clk) · 2026-11-03T16:00:18Z

+// RUN: %dxc -T ps_6_0 -E main -fspv-target-env=vulkan1.3 -DCAPABILITY_ON_INSTRUCTION %s -spirv | FileCheck %s
+// RUN: %dxc -T ps_6_0 -E main -fspv-target-env=vulkan1.3 -DCAPABILITY_ON_INSTRUCTION -fcgl %s -spirv | FileCheck %s


This includes two entry points with different annotations using the same run lines. The following three test files also involve entry points with different annotations, the same run lines, and only slightly different CHECK directives. I won't insist, but you might consider consolidating the entry points into one file, renaming the entry functions differently, and adding run lines with corresponding -E main2 flags as well as --check-prefixes flags to FileCheck where needed.

Suggested change

// RUN: %dxc -T ps_6_0 -E main -fspv-target-env=vulkan1.3 -DCAPABILITY_ON_INSTRUCTION %s -spirv | FileCheck %s

// RUN: %dxc -T ps_6_0 -E main -fspv-target-env=vulkan1.3 -DCAPABILITY_ON_INSTRUCTION -fcgl %s -spirv | FileCheck %s

// RUN: %dxc -T ps_6_0 -E main -fspv-target-env=vulkan1.3 -E main2 %s -spirv | FileCheck %s

// RUN: %dxc -T ps_6_0 -E main -fspv-target-env=vulkan1.3 -E main2 -fcgl %s -spirv | FileCheck %s

Gregory Roth (pow2clk) · 2026-11-03T16:00:41Z

+
+// CHECK:      OpDecorate %gl_PrimitiveID BuiltIn PrimitiveId
+
+#ifdef CAPABILITY_ON_INSTRUCTION


Suggested change

#ifdef CAPABILITY_ON_INSTRUCTION

Gregory Roth (pow2clk) · 2026-11-03T16:00:50Z

+float4 main(uint id : SV_PrimitiveID) : SV_Target {
+    return copyId(id);
+}
+#else


Suggested change

#else

Gregory Roth (pow2clk) · 2026-11-03T16:01:01Z

+#else
+[[vk::ext_capability(/* MeshShadingEXT */ 5283)]]
+[[vk::ext_extension("SPV_EXT_mesh_shader")]]
+float4 main(uint id : SV_PrimitiveID) : SV_Target {


Suggested change

float4 main(uint id : SV_PrimitiveID) : SV_Target {

float4 main2(uint id : SV_PrimitiveID) : SV_Target {

Gregory Roth (pow2clk) · 2026-11-03T16:01:09Z

+float4 main(uint id : SV_PrimitiveID) : SV_Target {
+    return id;
+}
+#endif


Suggested change

#endif

Copilot AI balanced review requested due to automatic review settings October 28, 2026 05:37

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

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

Copilot started reviewing on behalf of Philip Degarmo (aclysma) October 28, 2026 05:37

View session

Copilot AI reviewed Oct 28, 2026

View reviewed changes

Comment thread tools/clang/lib/SPIRV/CapabilityVisitor.cpp Resolved

Add a release note

063ac90

Copilot AI balanced review requested due to automatic review settings October 28, 2026 05:43

Copilot started reviewing on behalf of Philip Degarmo (aclysma) October 28, 2026 05:44

View session

Copilot AI reviewed Oct 28, 2026

View reviewed changes

Another test

25ebeed

Copilot AI balanced review requested due to automatic review settings October 28, 2026 06:16

Copilot started reviewing on behalf of Philip Degarmo (aclysma) October 28, 2026 06:17

View session

Copilot AI reviewed Oct 28, 2026

View reviewed changes

Jonathan Zakharov (jzakharovnv) suggested changes Oct 30, 2026

View reviewed changes

github-project-automation Bot moved this from New to In progress in HLSL Roadmap Oct 30, 2026

Code and comment clarity improvements. Better prose for ReleaseNotes,…

0bc340c

… add a footnote in SPIR-V that describes the behavior this change introduces.

Copilot AI balanced review requested due to automatic review settings October 30, 2026 21:51

Copilot started reviewing on behalf of Philip Degarmo (aclysma) October 30, 2026 21:52

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

Jonathan Zakharov (jzakharovnv) approved these changes Nov 2, 2026

View reviewed changes

Gregory Roth (pow2clk) approved these changes Nov 3, 2026

View reviewed changes

		.. [1] ``Geometry`` or ``MultiViewport`` is not added if the module declares
		one of these capabilities, which also enable the builtin:

		// RUN: %dxc -T ps_6_0 -E main -fspv-target-env=vulkan1.3 -DCAPABILITY_ON_INSTRUCTION %s -spirv \| FileCheck %s
		// RUN: %dxc -T ps_6_0 -E main -fspv-target-env=vulkan1.3 -DCAPABILITY_ON_INSTRUCTION -fcgl %s -spirv \| FileCheck %s


		// CHECK: OpDecorate %gl_PrimitiveID BuiltIn PrimitiveId

		#ifdef CAPABILITY_ON_INSTRUCTION

	float4 main(uint id : SV_PrimitiveID) : SV_Target {
	float4 main2(uint id : SV_PrimitiveID) : SV_Target {

Conversation

Philip Degarmo (aclysma) commented Oct 28, 2026

Uh oh!

azure-pipelines Bot commented Oct 28, 2026

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

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Philip Degarmo (aclysma) commented Oct 28, 2026

Uh oh!

Jonathan Zakharov (jzakharovnv) commented Oct 29, 2026

Uh oh!

Jonathan Zakharov (jzakharovnv) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Philip Degarmo (aclysma) commented Oct 30, 2026

Uh oh!

Jonathan Zakharov (jzakharovnv) commented Nov 2, 2026

Uh oh!

Gregory Roth (pow2clk) left a comment

Choose a reason for hiding this comment

Uh oh!

Gregory Roth (pow2clk) Nov 3, 2026

Choose a reason for hiding this comment

Uh oh!

Gregory Roth (pow2clk) Nov 3, 2026

Choose a reason for hiding this comment

Uh oh!

Gregory Roth (pow2clk) Nov 3, 2026

Choose a reason for hiding this comment

Uh oh!

Gregory Roth (pow2clk) Nov 3, 2026

Choose a reason for hiding this comment

Uh oh!

Gregory Roth (pow2clk) Nov 3, 2026

Choose a reason for hiding this comment

Uh oh!

Gregory Roth (pow2clk) Nov 3, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants