Diagnose non-bool SV_CullPrimitive mesh outputs in Sema by apollo-2006 · Pull Request #9073 · microsoft/DirectXShaderCompiler

Abir Deol (apollo-2006) · 2026-10-27T15:16:09Z

A non-bool SV_CullPrimitive on a mesh shader primitive output used to crash SPIR-V codegen with the "please file a bug report" error. DXIL only caught it at validation. It's now a Sema error for both:

error: SV_CullPrimitive must be applied to a scalar bool, but 'cull' has type 'uint'

Following Gregory Roth (@pow2clk)'s suggestion, the check runs in DiagnoseMeshEntry on out primitives params (nested structs and arrays included). bool1 and bool1x1 count as scalar bools, and pixel shader inputs are left alone since DXIL replaces them with 0.

Tests:

SemaHLSL/cullprimitive-type.hlsl covers the rejected and accepted types (fails without the change)
meshshading.ext.cullprimitive.bool1.hlsl checks that bool1 still lowers to the builtin
meshshading.ext.triangle.mesh.hlsl used int (it only passed because -fcgl skips validation), so it's bool now

Happy to change anything :)

The DXIL path rejects a non-bool SV_CullPrimitive with "SV_CullPrimitive must be bool.", but the SPIR-V backend passed the declared type through to the CullPrimitiveEXT builtin, so spirv-val failed with VUID-CullPrimitiveEXT-CullPrimitiveEXT-07036 and DXC asked the user to file a bug. Emit an error for a non-bool SV_CullPrimitive, and make meshshading.ext.triangle.mesh.hlsl declare it as bool. That test only passed with an int because it runs with -fcgl.

azure-pipelines · 2026-10-27T15:16:20Z

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

Copilot

🟡 Changes recommended

The new check rejects valid scalar-equivalent bool1 and bool1x1 declarations that already lower to SPIR-V boolean scalars.

1 open issue

Accept bool1 and bool1x1 SV_CullPrimitive declarations · New

What changed in this PR

This PR adds an early SPIR-V diagnostic for non-boolean SV_CullPrimitive declarations, replacing a fatal validation failure with a normal compiler error. The related NV mesh-shader limitations remain outside its scope.

Changes:

Validate the semantic’s element type before stage-specific handling.
Add a negative regression test and correct the existing mesh test to use bool.
Document the behavior change in the upcoming release notes.

File	Description
tools/clang/test/CodeGenSPIRV/meshshading.ext.triangle.mesh.hlsl	Use boolean cull values and update SPIR-V checks.
tools/clang/test/CodeGenSPIRV/meshshading.ext.cullprimitive.non-bool.hlsl	Test the diagnostic for a `uint` cull declaration.
tools/clang/lib/SPIRV/DeclResultIdMapper.cpp	Check `SV_CullPrimitive` types before creating stage variables.
docs/ReleaseNotes.md	Document the new diagnostic.

_{Review effort: Balanced}

Damyan Pepper (damyanp) · 2026-10-28T17:13:18Z

/azp run

azure-pipelines · 2026-10-28T17:13:30Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Jonathan Zakharov (jzakharovnv) · 2026-10-28T18:29:29Z

    int layer  : SV_RenderTargetArrayIndex;                 // -> Builtin Layer
    int vpIdx  : SV_ViewportArrayIndex;                     // -> Builtin ViewportIndex
-    int cullPrim : SV_CullPrimitive;                        // -> BUiltin CullPrimitiveEXT
+    bool cullPrim : SV_CullPrimitive;                        // -> BUiltin CullPrimitiveEXT


super nit:

Suggested change

bool cullPrim : SV_CullPrimitive; // -> BUiltin CullPrimitiveEXT

bool cullPrim : SV_CullPrimitive; // -> Builtin CullPrimitiveEXT

Jonathan Zakharov (jzakharovnv) · 2026-10-28T21:23:57Z

  case hlsl::Semantic::Kind::CullPrimitive: {
+    QualType elemType = type;
+    while (const auto *arrayType = astContext.getAsConstantArrayType(elemType))
+      elemType = arrayType->getElementType();


I tried this code on aggregate types, and it looks like it doesn't quite handle that case.

From what I see, this fails to catch bool cull[2]: DXIL gives the error "rows for system value semantic SV_CullPrimitive must be 1" meanwhile SPIR-V gives "invalid SPIR-V, please file a bug"

If you could fix this and add a test, that be great.

Jonathan Zakharov (jzakharovnv) · 2026-10-28T21:29:44Z

  // be used as MSOut.
  case hlsl::Semantic::Kind::CullPrimitive: {
+    QualType elemType = type;
+    while (const auto *arrayType = astContext.getAsConstantArrayType(elemType))


The bool check should apply only to MSPOut, I'll make a longer explanation below

Jonathan Zakharov (jzakharovnv) · 2026-10-28T22:07:45Z

One interesting side effect of the current implementation is that it rejects uint SV_CullPrimitive on PS inputs, which is not what DXIL does.

Per hctdb.py and docs/DXIL.rst, CullPrimitive has the interpretations: PSIn = NotInSig, MSPOut = NotPacked, and NA elsewhere. NotInSig means the element isn't in the signature and is accessed through an intrinsic. For CullPrimitive specifically, HLSignatureLower.cpp replaces all uses of CullPrimitive with constant 0 regardless of the type. This all happens before DxilValidation.cpp so the bool check never actually get enforced in this case (hence why CullPrimitive is allowed to be uint in PSin).

However, the SPIRV backend doesn't implement this CullPrimitive PSIn replaced with constant 0 stuff, since to use CullPrimitiveEXT requires MeshShadingEXT which is illegal in fragment shaders per VUID 07034.

So, the 2 options are to mirror DXIL's replacement with constant 0 or conform to VUID 07034, but that is an issue out of scope for this PR. What do you think Diego Novillo (@dnovillo).

Suffice to say though Abir Deol (@apollo-2006), if you could just keep the bool check to only apply to MSPOut, that'd be great, thanks!

DXIL replaces SV_CullPrimitive on pixel shader inputs with a constant 0, so only MSPOut has to be bool. Check the type before the per-primitive array is added, so a bool[2] is rejected like DXIL does, and accept bool1 and bool1x1.

Abir Deol (apollo-2006) · 2026-10-29T01:02:10Z

Thanks a lot for this (and for the PSIn explanation, that cleared up what I was missing). Here's what changed:

MSPOut only now, so PS inputs are back to the old behavior (I left the 07034 question for its own discussion).
bool cull[2]: good catch, I missed this completely 😞 . The old check was stripping the field's own array along with the per-primitive one. It now checks the type before that array is added, so arrays are rejected like in DXIL (new test: cullprimitive.array.hlsl).
bool1 / bool1x1: accepted now through isScalarType() (new test: cullprimitive.bool1.hlsl).
Alignment nit: applied your suggestion (it fixed the "BUiltin" typo too).

The mesh output cases match DXIL now, and tests and clang-format are clean. Thanks again for making it so easy to fix lol

Copilot

🟢 Approval recommended

No active review issues are reported for the change that replaces fatal validation failures for invalid SV_CullPrimitive types with normal compiler errors.

No open issues 🎉

1 resolved since last review

Accept bool1 and bool1x1 SV_CullPrimitive declarations

_{Review effort: Balanced}

Jonathan Zakharov (jzakharovnv) · 2026-10-29T05:12:27Z

Great! If you could add a bool2 negative test just to make certain aggregates don't get accepted, then it'd LGTM. Thanks!

Diego Novillo (dnovillo) · 2026-10-29T14:13:21Z

So, the 2 options are to mirror DXIL's replacement with constant 0 or conform to VUID 07034, but that is an issue out of scope for this PR. What do you think Diego Novillo (Diego Novillo (@dnovillo)).

Agreed. I would mirror DXIL and replace it with false. It doesn't make sense for the SPIR-V backend to error out here since DXIL accepts it and we can do the obvious thing. Abir Deol (@apollo-2006), could you file an issue for it?

Diego Novillo (dnovillo)

Thanks for this fix. I agree with Jonathan Zakharov (@jzakharovnv) about negative tests for bool2.

Chris B (@llvm-beanz) or Gregory Roth (@pow2clk) or Damyan Pepper (@damyanp) could you please approve the CI bot? We still don't have maintainer access to trigger those. Thanks.

Diego Novillo (dnovillo) · 2026-10-29T14:16:52Z

+  if (stageVarData.semantic->getKind() == hlsl::Semantic::Kind::CullPrimitive &&
+      stageVarData.sigPoint->GetKind() == hlsl::SigPoint::Kind::MSPOut) {
+    QualType scalarType;
+    if (!isScalarType(stageVarData.type, &scalarType) ||
+        !scalarType->isBooleanType()) {
+      emitError("semantic CullPrimitive must be a bool scalar",
+                stageVarData.semantic->loc);
+      return nullptr;
+    }
+  }


nit: Move this to validateShaderStageVarType? Next to SV_InnerCoverage so we do semantic type checks in one place.

…arType Keeps the semantic type checks together, next to SV_InnerCoverage. Adds a bool2 negative test.

Abir Deol (apollo-2006) · 2026-10-29T19:05:09Z

Moved the check into validateShaderStageVarType next to SV_InnerCoverage, and added a bool2 negative test (cullprimitive.vector.hlsl). Filed #9084 for the PSIn side. Thanks both :)

Copilot

🟢 Approval recommended

No active review issues were identified for the mesh-output SV_CullPrimitive type-validation change.

No open issues 🎉

_{Review effort: Balanced}

Gregory Roth (pow2clk) · 2026-10-30T19:53:48Z

/azp run

azure-pipelines · 2026-10-30T19:54:01Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Gregory Roth (pow2clk)

Hi Abir Deol (@apollo-2006)! I'm afraid I have a rather drastic alternative approach to suggest. While DXC often violates this, there are a lot of advantages to producing errors in Sema. I looked at the way DXIL errors for this and it actually isn't caught until validation, which is far from ideal.

If the error were produced in Sema, it would produce a common error for both DXIL and SPIR-V. I had Claude work up a quick change with a detailed description of how I envisioned the solution: https://github.com/pow2clk/DirectXShaderCompiler/tree/boolcullprim. You are free to take as much or as little of that commit into your change. If you do, please move your excellent tests for expected failures to the SemaHLSL subdirectory with appropriate changes to convert them to validation tests.

The branch: https://github.com/pow2clk/DirectXShaderCompiler/tree/boolcullprim
The commit: pow2clk@1098f70

Note that the test included is in the wrong place. It does demonstrate how you might use different entry point values to consolidate some of your tests though.

Move the check from the SPIR-V backend to Sema so DXIL and SPIR-V give the same error. DXIL only caught it at validation before. Only mesh shader primitive outputs are checked, bool1 and bool1x1 still count as scalar bools, and pixel shader inputs are left alone since DXIL replaces them with 0. The error tests move to SemaHLSL. Co-authored-by: Greg Roth <groth@nvidia.com>

# Conflicts: # docs/ReleaseNotes.md

Abir Deol (apollo-2006) · 2026-11-03T18:39:16Z

Thanks, this is way cleaner than doing it per backend. Moved the check into Sema (DXIL and SPIR-V now give the same error) and kept your diagnostic, with you as co-author.

Two small differences from your commit:

it only checks mesh primitive outputs (PS inputs are valid in DXIL since they get replaced with 0)
bool1 and bool1x1 still count as scalar bools

Error tests are now one -verify file in SemaHLSL. Let me know if that's not what you had in mind :)

Copilot

🟡 Changes recommended

The SV_CullPrimitive validation misses inherited fields and parameter-level semantics while incorrectly rejecting nested fields with enclosing semantic overrides.

3 open issues

_{Review effort: Balanced}

Look through base classes, check a semantic declared directly on the primitives parameter, and stop at a semantic on an enclosing field or parameter since it overrides the ones inside.

Copilot

🟡 Changes recommended

SV_CullPrimitive validation rejects valid struct-wrapped boolean outputs because it checks record types instead of propagating the effective semantic and validating scalar-bool leaves.

1 open issue

Accept bool leaf outputs with enclosing cull-primitive semantics · New

3 resolved since last review

_{Review effort: Balanced}

+    bool IsScalarBool =
+        hlsl::IsHLSLVecMatType(ElemTy)
+            ? hlsl::GetElementCount(ElemTy) == 1 &&
+                  hlsl::GetElementTypeOrType(ElemTy)->isBooleanType()
+            : ElemTy->isBooleanType();


Copilot AI balanced review requested due to automatic review settings October 27, 2026 15:16

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

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

Copilot started reviewing on behalf of Abir Deol (apollo-2006) October 27, 2026 15:16

View session

Copilot AI reviewed Oct 27, 2026

View reviewed changes

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

Jonathan Zakharov (jzakharovnv) suggested changes Oct 28, 2026

View reviewed changes

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

Copilot AI balanced review requested due to automatic review settings October 29, 2026 01:02

Copilot started reviewing on behalf of Abir Deol (apollo-2006) October 29, 2026 01:02

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Diego Novillo (dnovillo) suggested changes Oct 29, 2026

View reviewed changes

[SPIR-V] Move the SV_CullPrimitive type check to validateShaderStageV…

0aeba46

…arType Keeps the semantic type checks together, next to SV_InnerCoverage. Adds a bool2 negative test.

Copilot AI balanced review requested due to automatic review settings October 29, 2026 19:05

Abir Deol (apollo-2006) mentioned this pull request Oct 29, 2026

[SPIR-V] SV_CullPrimitive as a pixel shader input produces invalid SPIR-V #9084

Open

Copilot started reviewing on behalf of Abir Deol (apollo-2006) October 29, 2026 19:05

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Gregory Roth (pow2clk) enabled auto-merge (squash) October 30, 2026 19:56

Diego Novillo (dnovillo) approved these changes Oct 30, 2026

View reviewed changes

Jonathan Zakharov (jzakharovnv) approved these changes Oct 30, 2026

View reviewed changes

Gregory Roth (pow2clk) reviewed Nov 2, 2026

View reviewed changes

Abir Deol (apollo-2006) and others added 3 commits November 2, 2026 17:41

Merge remote-tracking branch 'origin/main' into spirv-cullprimitive-bool

801f05f

# Conflicts: # docs/ReleaseNotes.md

Move the SV_CullPrimitive release note to Bug Fixes

120c19b

auto-merge was automatically disabled November 3, 2026 18:39
Head branch was pushed to by a user without write access

Copilot AI balanced review requested due to automatic review settings November 3, 2026 18:39

Abir Deol (apollo-2006) changed the title ~~[SPIR-V] Reject non-bool SV_CullPrimitive~~ Diagnose non-bool SV_CullPrimitive mesh outputs in Sema Nov 3, 2026

Copilot started reviewing on behalf of Abir Deol (apollo-2006) November 3, 2026 18:44

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Resolved

Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Resolved

Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Resolved

Check base classes and parameter semantics for SV_CullPrimitive

75b015f

Look through base classes, check a semantic declared directly on the primitives parameter, and stop at a semantic on an enclosing field or parameter since it overrides the ones inside.

Copilot AI balanced review requested due to automatic review settings November 5, 2026 07:35

Copilot started reviewing on behalf of Abir Deol (apollo-2006) November 5, 2026 07:36

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

Comment thread tools/clang/lib/Sema/SemaHLSL.cpp

Comment on lines +17295 to +17299

bool IsScalarBool =

hlsl::IsHLSLVecMatType(ElemTy)

? hlsl::GetElementCount(ElemTy) == 1 &&

hlsl::GetElementTypeOrType(ElemTy)->isBooleanType()

: ElemTy->isBooleanType();

	bool cullPrim : SV_CullPrimitive; // -> BUiltin CullPrimitiveEXT
	bool cullPrim : SV_CullPrimitive; // -> Builtin CullPrimitiveEXT

Conversation

Abir Deol (apollo-2006) commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines Bot commented Oct 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Damyan Pepper (damyanp) commented Oct 28, 2026

Uh oh!

azure-pipelines Bot commented Oct 28, 2026

Uh oh!

Jonathan Zakharov (jzakharovnv) Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Jonathan Zakharov (jzakharovnv) Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Jonathan Zakharov (jzakharovnv) Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Jonathan Zakharov (jzakharovnv) commented Oct 28, 2026

Uh oh!

Abir Deol (apollo-2006) commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Jonathan Zakharov (jzakharovnv) commented Oct 29, 2026

Uh oh!

Diego Novillo (dnovillo) commented Oct 29, 2026

Uh oh!

Diego Novillo (dnovillo) left a comment

Choose a reason for hiding this comment

Uh oh!

Diego Novillo (dnovillo) Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

Abir Deol (apollo-2006) commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Gregory Roth (pow2clk) commented Oct 30, 2026

Uh oh!

azure-pipelines Bot commented Oct 30, 2026

Uh oh!

Gregory Roth (pow2clk) left a comment

Choose a reason for hiding this comment

Uh oh!

Abir Deol (apollo-2006) commented Nov 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

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

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Abir Deol (apollo-2006) commented Oct 27, 2026 •

edited

Loading

Abir Deol (apollo-2006) commented Oct 29, 2026 •

edited

Loading