fix: ignore [numthreads] on non-entry compute methods (match HLSL) - #3304
Open
sasvdw wants to merge 2 commits into
Open
fix: ignore [numthreads] on non-entry compute methods (match HLSL)#3304sasvdw wants to merge 2 commits into
sasvdw wants to merge 2 commits into
Conversation
[numthreads] on a method other than the compute entry point (CSMain) was emitted as an OpExecutionMode LocalSize on that method's own function id, but the SPIR-V entry point is a generated CSMain_Wrapper. The mode was left stranded on a non-entry function, producing invalid SPIR-V that failed SPIRV-Cross HLSL legalization with a bare spvOptimizerRun InternalError (Direct3D11 path). HLSL/FXC/DXC treat [numthreads] on a non-entry function as legal and silently ignored; the thread group size comes only from the entry point. Match that: skip the LocalSize on a non-entry compute method and warn, pointing to ThreadNumberX/Y/Z (C#-overridable via ComputeEffectShader.ThreadNumbers, which also drives dispatch). Drain SymbolTable.Warnings to the log in MergeSDSL so the warning reaches the user. Also validate the module when LegalizeForHlsl throws, surfacing the real invalid instruction instead of a bare InternalError. Adds a regression test (compiles + valid SPIR-V + warning; verified red/green). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ThreadNumbers sets the compute shader's thread group size (its [numthreads], via the ThreadNumberX/Y/Z macros on CSMain); ThreadGroupCounts is the dispatch count. Documenting this points users at the supported mechanism rather than a literal [numthreads] in the shader body (which is ignored on non-entry methods). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
🤖 Draft PR — automatic CI is skipped to save runner minutes.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Details
Summary — a compute shader with
[numthreads]on a non-entry method (e.g.override void Compute()) emitted invalid SPIR-V and failed to compile on Direct3D11; this ignores the mis-placedattribute (as HLSL does) and warns.
Description — the
[numthreads]OpExecutionMode LocalSizewas emitted on the attribute-bearingmethod, but the compute entry point is a generated
CSMain_Wrapper, so the mode was stranded on anon-entry function → invalid SPIR-V →
LegalizeForHlslfailed with a barespvOptimizerRun InternalError. HLSL/FXC/DXC treat[numthreads]on a non-entry function as legal and silently ignored(size comes only from the entry point). Match that:
MethodOrMember.cs— emitLocalSizeonly when the attribute is on the compute entry(
EntryPoint == ComputeShader); otherwise skip it (valid SPIR-V) and warn, pointing toThreadNumberX/Y/Z.ShaderMixer.cs— drainSymbolTable.Warningstolog.WarninginMergeSDSL(only errors werebefore), so the warning reaches the user.
EffectCompiler.cs— whenLegalizeForHlslthrows, validate the module and surface the realinstruction instead of a bare
InternalError.ComputeEffectShader.cs— clarify the XML docs:ThreadNumberssets the compute shader's threadgroup size (its
[numthreads]);ThreadGroupCountsis the dispatch count.Not a regression (the classic ≤4.3 path also dropped it).
Motivation and context — see #3303. Repro: https://github.com/sasvdw/stride-spirv-legalize-repro
Related Issue
Fixes #3303.
Types of changes
Checklist
Validation status (honest):
StrideShaderTests.NumThreadsOnNonEntryMethodIsIgnoredWithWarning+CompilerTests/CSNumThreadsOnNonEntry.sdsl— compiles, valid SPIR-V, warning emitted; verifiedred/green.
Stride.Shaders.Parsers/Compilersclean;StrideShaderTests13/13. CI pending.ComputeEffectShader.ThreadNumbers/ThreadGroupCountsXML docs (flow to the APIreference). The manual has no compute-shader guide — tracked as a separate
stride-docsfollow-up.