Report duplicate [numthreads] attributes as conflicting modifiers

[nv-slang-bot]

Scope

Addresses #11881, which asks for three distinct things. This PR delivers only part (c) as a clean, standalone diagnostic fix:

• (c) [this PR] — error on conflicting duplicate [numthreads] on one entry point.
• (b) deferred follow-up — warn when a [numthreads] and a layout(local_size_*)-derived value disagree (see Process report for why).
• (a) not here — per-entry-point override of the global layout(local_size) size is already delivered by open PR Fixes GitHub issue #7894 (makes reflection and SPIR-V agree on layout(local_size) in) #11715 (NBickford-NV). Not re-implemented.

Because (a)/(b) are handled elsewhere/deferred, this uses Addresses #11881, not Fixes, so the issue stays open.

Motivation

Multiple [numthreads] attributes on a single entry point are silently accepted today — the compiler keeps one value and ignores the rest, with rc=0 and no diagnostic. Consider the example from the issue:

[numthreads(2,3,4)] [numthreads(5,6,7)] [numthreads(1,5,9)]
void main() {}

As reported in the issue and reproduced during triage, this compiles cleanly and produces a single group size — one attribute silently wins and the other two are discarded — so the author is never told that two of the three conflicting attributes were dropped.

Proposed solution

Register NumThreadsAttribute as its own modifier conflict group so the existing duplicate-modifier machinery reports it. In getModifierConflictGroupKind() (source/slang/slang-check-modifier.cpp), attributes that may legally appear at most once each map to their own node type as their conflict-group key (for example the work-graph Node* attributes just above the added case). NumThreadsAttribute was missing from that switch, so it fell through to the default: return ASTNodeType::NodeBase arm. The subsequent duplicate-detection loop only acts on modifiers whose group is not NodeBase, so [numthreads] was never checked. Adding the one case makes a second [numthreads] on the same entry point trip the existing duplicate-modifier error (31202).

This reuses one source of truth (the conflict-group classifier + the DuplicateModifier diagnostic) rather than adding a parallel numthreads-specific check, and it produces an error, matching the severity the maintainer requested. The "redundant or conflicting" wording of 31202 already covers both identical and value-conflicting duplicates.

Change summary

File	What
source/slang/slang-check-modifier.cpp	Add case ASTNodeType::NumThreadsAttribute: return modifierType; to getModifierConflictGroupKind() (its own exclusive group), beside the existing work-graph attribute self-groups.
tests/diagnostics/execution-model/duplicate-numthreads.slang	New regression test: the issue's three-[numthreads] entry point now reports E31202.

Concepts and vocabulary

• Modifier conflict group — a key computed by getModifierConflictGroupKind(ASTNodeType) grouping mutually-exclusive modifiers. checkModifiers() walks the checked modifier list and, when it sees a second modifier whose group is already recorded, emits Diagnostics::DuplicateModifier (code 31202). Returning ASTNodeType::NodeBase means "not tracked" — the loop skips it.
• Layout-synthesized NumThreadsAttribute — for GLSL layout(local_size_x = N) in; compute/mesh/amp entry points, validateEntryPoint() synthesizes a NumThreadsAttribute from sibling EmptyDecl GLSLLayoutLocalSizeAttributes, but only when the entry point has no user [numthreads].

Process report

Code trace. In getModifierConflictGroupKind() (source/slang/slang-check-modifier.cpp:1564) NumThreadsAttribute had no case, so it returned NodeBase via the default arm (:1672). The duplicate-detection loop reads conflictGroup = getModifierConflictGroupKind(modifier->astNodeType) and only records/compares the modifier when conflictGroup != ASTNodeType::NodeBase (:2469); so [numthreads] was never diagnosed. validateAttribute for NumThreadsAttribute (:396) validates each attribute's three args but does no duplicate detection. The added case puts NumThreadsAttribute in its own group, so the second (and each further) occurrence hits Diagnostics::DuplicateModifier — the duplicate-modifier diagnostic, error 31202, defined in source/slang/slang-diagnostics.lua:2909.

Input-shape / spurious-fire check (the one real risk). The layout-synthesized NumThreadsAttribute is created and added inside validateEntryPoint() (source/slang/slang-check-shader.cpp:1866 and :1900), guarded by !entryPointFuncDecl->findModifier<NumThreadsAttribute>() (:1851-1852) — it is synthesized only when the entry point has zero user [numthreads], and exactly one is ever added. The duplicate-modifier loop runs earlier, during the modifier-check pass (checkModifiers()), and addModifier does not re-run it. So the synthesized attribute can never form a duplicate, and this change cannot spuriously fire on the GLSL layout(local_size) path. Existing tests exercise those paths — the GLSL layout synthesis in tests/glsl/compute-shader-layout.slang and tests/glsl/compute-shader-layout-id.slang, and a single [numthreads] in tests/hlsl/simple/compute-numthreads.hlsl and tests/language-feature/execution-model/compute-numthreads-functional.slang (all must still compile).

Why (b) is deferred. A layout-vs-[numthreads] disagreement warning would live at the same merge site (source/slang/slang-check-shader.cpp:1849-1907), which currently early-outs when a user [numthreads] is present — so (b) needs a new branch there plus a new warning diagnostic and a spec-constant/default-1-aware extent comparison (GLSL defaults unspecified axes to 1; extents may be ConstantIntVal or spec-constant DeclRef, and a naive compare would emit false warnings). That region is also edited by open PR #11715, so bundling risks a merge conflict with the maintainer's in-flight PR, and (b) is conceptually a follow-up to the precedence #11715 establishes. Keeping (c) standalone lets this land independently; (b) is a clean follow-up.

Verification

The change plus the regression test are authored and reviewed by source inspection and by the diagnostic-annotation matcher's documented semantics (tools/slang-test/diagnostic-annotation-util.cpp). The test asserts the error code: a single err() emits two machine-readable rows (a title row and a span row), each carrying the code, and caretless //CHECK: E31202 annotations match against the errorCode field. Three [numthreads] produce two duplicate diagnostics (four rows), so four E31202 lines are expected under exhaustive mode. The existing tests tests/diagnostics/modifier-check.slang and tests/diagnostics/execution-model/non-positive-numthreads.slang show the same two-rows-per-diagnostic shape. CI is the empirical gate for the build and test run.

Labeling

Labeled pr: non-breaking: this only rejects a malformed construct (conflicting duplicate [numthreads]) that previously compiled by silently discarding all-but-one value. If you'd rather treat "now errors on previously-accepted source" as pr: breaking change, please relabel.

Addresses #11881.

_{🤖 Generated by an automated Slang coworker — may be inaccurate. A human maintainer should verify.}

[jkwak-work]

Looks good to me.