fix: invalid SPIR-V that fails HLSL legalization ([numthreads] on non-entry methods, implicit-LOD sampling outside pixel/compute) - #3304
Open
sasvdw wants to merge 4 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.
|
Fails today: the sample lowers to OpImageSampleImplicitLod, which SPIR-V allows only in Fragment/GLCompute/Mesh/Task, so spirv-val rejects the module and the D3D11 legalization path dies with a bare InternalError. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
stride3d#3313) Texture.Sample() reachable from a vertex, hull, domain or geometry entry point emitted OpImageSampleImplicitLod, which SPIR-V allows only in Fragment, GLCompute, MeshEXT and TaskEXT. The module then failed validation, and the Direct3D11 path surfaced it as a bare spvOptimizerRun InternalError. fxc rejects the equivalent HLSL with X4532 and dxc with "Opcode Sample not valid in shader model vs_6_0", so such a shader was never valid; the pre-SPIR-V compiler reported it clearly. Restore that diagnostic by walking the call graph from each non-fragment entry point and reporting any implicit-LOD sample, with its source location and the explicit-LOD alternatives. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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
Two unrelated SDSL constructs emit invalid SPIR-V. On Direct3D11 both fail the same way.
LegalizeForHlslthrows a barespvOptimizerRun InternalError, because nothing validates the modulefirst. This PR fixes both causes and reports the shared failure clearly.
1.
[numthreads]on a non-entry compute method (#3303)The compiler emitted
OpExecutionMode LocalSizeon the method that carries the attribute. The computeentry point is a generated
CSMain_Wrapper, so the mode landed on a non-entry function. The result wasinvalid SPIR-V.
fxc and dxc accept
[numthreads]on a non-entry function and then ignore it. The thread group sizecomes only from the entry point. Stride now does the same.
MethodOrMember.cs: emitLocalSizeonly when the attribute is on the compute entry(
EntryPoint == ComputeShader). Otherwise skip it and log a warning that points toThreadNumberX/Y/Z.ShaderMixer.cs: drainSymbolTable.Warningstolog.WarninginMergeSDSL. Before this change thecode drained only the errors, so the warning never reached the user.
ComputeEffectShader.cs: clarify the XML documentation.ThreadNumberssets the thread group size ofthe compute shader, which is its
[numthreads].ThreadGroupCountsis the dispatch count.This is not a regression. The classic 4.3 path also dropped a misplaced
[numthreads].2. Implicit-LOD texture sampling outside the pixel and compute stages (#3313)
A
Texture.Sample(...)call reachable from a vertex, hull, domain or geometry entry point emittedOpImageSampleImplicitLod. SPIR-V allows that instruction only inFragment,GLCompute,MeshEXTand
TaskEXT. Implicit LOD needs screen-space derivatives, and those stages do not have them.This change reports an error. It does not lower the call to
SampleLevel(..., 0), because the shaderwas never valid. I checked both Microsoft compilers directly:
Texture0.Sample(s, uv)in a vertex shaderfxc /T vs_5_0error X4532: cannot map expression to vs_5_0 instruction setdxc -T vs_6_0error: Opcode Sample not valid in shader model vs_6_0→ "Function uses features incompatible with the shader stage (vs)"dxc -T vs_6_0 -spirverror: sampling with implicit lod is only allowed in fragment and compute shadersImplicitLod instructions require Fragment, GLCompute, MeshEXT or TaskEXT execution modelThe Microsoft documentation for
Sampleagrees: "This method can be invoked within a pixel shader, butit is not supported in a vertex shader or a geometry shader." The "Vertex-shader stage" page does list
Sampleas legal in a vertex shader. That entry is a long-standing documentation error and should readSampleLevel.SampleGradandSampleCmpLevelZerodo compile forvs_5_0.So this is not a functional regression, because such a shader never compiled. It is a diagnostics
regression. Version 4.3 reported
X4532. Version 4.4 reportedspvOptimizerRun InternalError.The engine has always treated this as illegal. That is why
MaterialDisplacementMapFeaturesetsIsNotPixelStageand routes texture reads throughComputeColorTextureLod, which usesSampleLevel.ShaderMixer.cs: after interface processing, walk the call graph from each non-fragment entry point.Report every implicit-LOD sample that the walk reaches, with its source location and the explicit-LOD
alternatives:
A silent lowering to LOD 0 would make Stride accept a shader that fxc, dxc and the SPIR-V validator all
reject. It would also change the sampling behavior without telling the author. I am happy to change this
if you prefer the permissive behavior.
3. Shared diagnostics
EffectCompiler.cs: whenLegalizeForHlslthrows, validate the module and report the instructionthat caused the failure, instead of a bare
InternalError. Issue Texture.Sample() in a vertex-stage material surface emits invalid SPIR-V (D3D11 compile fails with a bare InternalError) #3313 asks for this. It also coversfuture problems of the same family.
Related Issue
Fixes #3303.
Fixes #3313.
Repro for #3303: https://github.com/sasvdw/stride-spirv-legalize-repro
Repro for #3313: https://github.com/delustra/stride-vtf-spirv-repro
Types of changes
Checklist
Validation status
StrideShaderTestsgainsTextureSampleOutsideFragmentStageIsReported, which usesCompilerTests/VSTextureSample.sdsl. It also gainsTextureSampleInFragmentStageCompiles, which usesCompilerTests/PSTextureSample.sdsl. The second test is the control. It proves that pixel-stagesampling still compiles to valid SPIR-V.
NumThreadsOnNonEntryMethodIsIgnoredWithWarningstill passes.I checked each test red before the fix and green after it. The test for Texture.Sample() in a vertex-stage material surface emits invalid SPIR-V (D3D11 compile fails with a bare InternalError) #3313 is a separate commit that
comes before its fix, so the failure is on record.
parses floats from
.sdsltest headers with the current culture, so they fail on any comma-decimallocale. The same tests fail when I revert these changes. fix: parse and format shader numbers with the invariant culture #3325 fixes them separately.
ComputeEffectShader.ThreadNumbersandThreadGroupCountsis nowclearer, and it flows to the API reference. The manual has no compute-shader guide. I track that as a
separate
stride-docsfollow-up.