dasSpirv: broadcast scalar operands to vector result in GLSL.std.450 ops (+ regression test)#3243
Merged
Merged
Conversation
SPIR-V has no implicit scalar-to-vector broadcast, but several GLSL.std.450 ops accept a scalar operand against a vector result (mix / clamp / min / max / step / smoothstep) -- e.g. daslang lerp(vec, vec, float), where the FMix interpolant t is a scalar. The emitter passed such a scalar straight to OpExtInst, producing SPIR-V that the typer accepts and the emitter happily emits but which is INVALID: it surfaces only as a driver access-violation at vkCreateGraphicsPipelines. spirv-val flags it cleanly: "FMix: expected types of all operands to be equal to Result Type". Fix: splat any scalar operand of these ops to the result width via OpCompositeConstruct before the OpExtInst. ext_op_broadcasts_scalar() gates the op set; ops that require exact-match operands (pow, atan2, distance, ...) are not touched, and vector operands + scalar-result calls are left as-is. Verified: lerp(vec, vec, scalar) now passes spirv-val (was an invalid-SPIR-V pipeline crash); tutorial 10's deferred shaders (heavy lerp/clamp/min/max use) still render and pass their pixel oracle.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes invalid SPIR-V generation for certain GLSL.std.450 extended instructions when the result is a vector but one operand is a scalar (notably lerp(vecN, vecN, scalar) → OpExtInst FMix), by explicitly splatting scalar operands to vector width before emitting the ext-inst. Adds a regression test to ensure the emitted SPIR-V validates with spirv-val.
Changes:
- Add
ext_op_broadcasts_scalarhelper and broadcast logic in the genericOpExtInstemission path to splat scalar operands viaOpCompositeConstructwhen required. - Add a new SPIR-V test that compiles a fragment shader using
lerp(vec2/3/4, vec2/3/4, scalar)and validates the binary withspirv-val.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/spirv/test_ext_broadcast.das | Adds regression test fixture and validation for scalar→vector broadcast in GLSL.std.450 ops (FMix). |
| modules/dasSpirv/spirv/spirv_emit.das | Implements scalar operand splatting for select GLSL.std.450 ops when emitting OpExtInst with vector results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The emitter fix in the previous commit (splat a scalar operand to the
vector result width via OpCompositeConstruct before the OpExtInst)
shipped without a test. Add tests/spirv/test_ext_broadcast.das.
It asserts lerp(vecN, vecN, scalar) for N = 2/3/4 emits OpExtInst (FMix)
+ OpCompositeConstruct (the scalar->vec splat) and passes spirv-val --
the real oracle. Before the fix the scalar t went straight into FMix:
the blob still EMITTED (a bytes-only check passed) but was invalid
SPIR-V ("FMix: expected types of all operands to be equal to Result
Type") that crashed the driver at vkCreateGraphicsPipelines.
lerp is the only GLSL.std.450 op daslang exposes with a vec+scalar
overload, so it is the only reachable broadcast case; the other
ext_op_broadcasts_scalar entries (FClamp/FMin/FMax/Step/SmoothStep) are
defensive (no daslang overload reaches them yet).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
borisbat
force-pushed
the
bbatkin/dasspirv-vecscalar-iface
branch
from
a0e29eb to
46b6cca
Compare
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.
Problem
lerp(vecN, vecN, scalar)lowers toOpExtInst FMix, but SPIR-VFMixrequires all three operands to match the result type — there is no implicit scalar→vector broadcast. The emitter passed the scalartstraight intoFMix, producing invalid SPIR-V. It passed emission (a bytes-only check would not catch it) and only surfaced as a driver access-violation atvkCreateGraphicsPipelines.spirv-valflags it cleanly:Surfaced by tutorial 10's deferred shaders in the dasVulkan ladder (heavy
lerp/IBL use).Fix (
spirv_emit.das)ext_op_broadcasts_scalar(op)gates the GLSL.std.450 ops that allow a scalar operand against a vector result —FMix/FClamp/FMin/FMax/Step/SmoothStep. In the genericOpExtInstbranch, any scalar operand (component_count == 1) of one of those ops is splatted to the result width viaOpCompositeConstructbefore the instruction. Ops that require exact-match operands (pow,atan2,distance, …) are untouched, and vector-operand / scalar-result calls are left as-is.Test (
tests/spirv/test_ext_broadcast.das)The fix originally shipped without a regression test; this adds one. It asserts
lerp(vecN, vecN, scalar)for N = 2/3/4 emitsOpExtInst(FMix) +OpCompositeConstruct(the splat) and passesspirv-val— the real oracle that catches the original invalid blob. Self-contained inline-shader fixture (same pattern astest_math_breadth/test_math_step); auto-discovered by thetests/spirv/*.dasAOT glob.Note (dispatch audit)
lerpis the only GLSL.std.450 op daslang currently exposes with a vec+scalar overload, so it is the only reachable broadcast case. The other fiveext_op_broadcasts_scalarentries are defensive — daslang'smathhas only same-type overloads formin/max/clamp/step/smoothstep, so they never reach the emitter today, but the helper is ready if those overloads are added.🤖 Generated with Claude Code