feat: More comprehensive tracking of possible side effects#2695
feat: More comprehensive tracking of possible side effects#2695cieplypolar wants to merge 25 commits into
Conversation
- Mark function arguments as side-effect-free - Mark tgpu.const reads as side-effect-free - Propagate operand side-effects in pure binary operators - Add side-effect tracking to dualImpl (matching callableSchema) - Mark buffer reads (uniform/readonly/mutable) as side-effect-free - Add comprehensive ternary runtime tests from issue #2587 - Update existing ternary test for runtime select() generation
- Mark function arguments as side-effect-free - Mark tgpu.const reads as side-effect-free - Propagate operand side-effects in pure binary operators - Add side-effect tracking to dualImpl (matching callableSchema) - Mark buffer reads (uniform/readonly/mutable) as side-effect-free - Add comprehensive ternary runtime tests from issue #2587 - Update existing ternary test for runtime select() generation
… discard sideEffects
7435193 to
befe8de
Compare
|
pkg.pr.new packages benchmark commit |
befe8de to
6d5ee16
Compare
Bundle size comparison (
|
| 🟢 Decreased | ➖ Unchanged | 🔴 Increased (max 2.93%) | ❔ Unknown |
|---|---|---|---|
| 0 | 36 | 285 | 0 |
import * as ... in PR vs import * as ... in target (did bundle size increase?):
Click to reveal the results table (148 entries).
| Test | tsdown |
|---|---|
| d_ref.ts | 4.94 kB ( |
| d_bool.ts | 13.20 kB ( |
| d_f16.ts | 13.20 kB ( |
| d_f32.ts | 13.20 kB ( |
| d_i32.ts | 13.20 kB ( |
| d_u32.ts | 13.20 kB ( |
| d_u16.ts | 13.22 kB ( |
| std_discard.ts | 14.49 kB ( |
| std_copy.ts | 14.73 kB ( |
| std_arrayLength.ts | 14.74 kB ( |
| d_textureDepth2d.ts | 13.64 kB ( |
| d_textureDepthCube.ts | 13.65 kB ( |
| d_texture1d.ts | 13.65 kB ( |
| d_texture2d.ts | 13.65 kB ( |
| d_texture3d.ts | 13.65 kB ( |
| d_textureCube.ts | 13.66 kB ( |
| d_textureDepth2dArray.ts | 13.66 kB ( |
| d_textureDepthCubeArray.ts | 13.66 kB ( |
| d_textureDepthMultisampled2d.ts | 13.67 kB ( |
| d_texture2dArray.ts | 13.67 kB ( |
| d_textureCubeArray.ts | 13.67 kB ( |
| d_textureMultisampled2d.ts | 13.68 kB ( |
| std_dpdx.ts | 15.45 kB ( |
| std_dpdxCoarse.ts | 15.46 kB ( |
| std_dpdxFine.ts | 15.46 kB ( |
| std_dpdy.ts | 15.46 kB ( |
| std_dpdyCoarse.ts | 15.46 kB ( |
| std_dpdyFine.ts | 15.46 kB ( |
| std_fwidth.ts | 15.46 kB ( |
| std_fwidthCoarse.ts | 15.46 kB ( |
| std_fwidthFine.ts | 15.46 kB ( |
| std_atomicLoad.ts | 16.25 kB ( |
| std_atomicStore.ts | 16.26 kB ( |
| std_textureBarrier.ts | 16.26 kB ( |
| std_atomicAdd.ts | 16.26 kB ( |
| std_atomicAnd.ts | 16.26 kB ( |
| std_atomicMax.ts | 16.26 kB ( |
| std_atomicMin.ts | 16.26 kB ( |
| std_atomicOr.ts | 16.26 kB ( |
| std_atomicSub.ts | 16.26 kB ( |
| std_atomicXor.ts | 16.26 kB ( |
| std_storageBarrier.ts | 16.26 kB ( |
| std_workgroupBarrier.ts | 16.26 kB ( |
| d_vec2b.ts | 19.63 kB ( |
| d_vec2f.ts | 19.63 kB ( |
| d_vec2h.ts | 19.63 kB ( |
| d_vec2i.ts | 19.63 kB ( |
| d_vec2u.ts | 19.63 kB ( |
| d_vec3b.ts | 19.63 kB ( |
| d_vec3f.ts | 19.63 kB ( |
| d_vec3h.ts | 19.63 kB ( |
| d_vec3i.ts | 19.63 kB ( |
| d_vec3u.ts | 19.63 kB ( |
| d_vec4b.ts | 19.63 kB ( |
| d_vec4f.ts | 19.63 kB ( |
| d_vec4h.ts | 19.63 kB ( |
| d_vec4i.ts | 19.63 kB ( |
| d_vec4u.ts | 19.63 kB ( |
| d_formatToWGSLType.ts | 21.12 kB ( |
| d_uint8.ts | 21.13 kB ( |
| d_float16.ts | 21.13 kB ( |
| d_float16x2.ts | 21.13 kB ( |
| d_float16x4.ts | 21.13 kB ( |
| d_float32.ts | 21.13 kB ( |
| d_float32x2.ts | 21.13 kB ( |
| d_float32x3.ts | 21.13 kB ( |
| d_float32x4.ts | 21.13 kB ( |
| d_sint16.ts | 21.13 kB ( |
| d_sint16x2.ts | 21.13 kB ( |
| d_sint16x4.ts | 21.13 kB ( |
| d_sint32.ts | 21.13 kB ( |
| d_sint32x2.ts | 21.13 kB ( |
| d_sint32x3.ts | 21.13 kB ( |
| d_sint32x4.ts | 21.13 kB ( |
| d_sint8.ts | 21.13 kB ( |
| d_sint8x2.ts | 21.13 kB ( |
| d_sint8x4.ts | 21.13 kB ( |
| d_snorm16.ts | 21.13 kB ( |
| d_snorm16x2.ts | 21.13 kB ( |
| d_snorm16x4.ts | 21.13 kB ( |
| d_snorm8.ts | 21.13 kB ( |
| d_snorm8x2.ts | 21.13 kB ( |
| d_snorm8x4.ts | 21.13 kB ( |
| d_uint16.ts | 21.13 kB ( |
| d_uint16x2.ts | 21.13 kB ( |
| d_uint16x4.ts | 21.13 kB ( |
| d_uint32.ts | 21.13 kB ( |
| d_uint32x2.ts | 21.13 kB ( |
| d_uint32x3.ts | 21.13 kB ( |
| d_uint32x4.ts | 21.13 kB ( |
| d_uint8x2.ts | 21.13 kB ( |
| d_uint8x4.ts | 21.13 kB ( |
| d_unorm10_10_10_2.ts | 21.13 kB ( |
| d_unorm16.ts | 21.13 kB ( |
| d_unorm16x2.ts | 21.13 kB ( |
| d_unorm16x4.ts | 21.13 kB ( |
| d_unorm8.ts | 21.13 kB ( |
| d_unorm8x2.ts | 21.13 kB ( |
| d_unorm8x4.ts | 21.13 kB ( |
| d_unorm8x4_bgra.ts | 21.13 kB ( |
| d_packedFormats.ts | 21.15 kB ( |
| d_isPackedData.ts | 21.19 kB ( |
| d_alignmentOf.ts | 22.08 kB ( |
| std_subgroupAdd.ts | 24.58 kB ( |
| std_subgroupAll.ts | 24.58 kB ( |
| std_subgroupAnd.ts | 24.58 kB ( |
| std_subgroupAny.ts | 24.58 kB ( |
| std_subgroupBallot.ts | 24.58 kB ( |
| std_subgroupBroadcast.ts | 24.58 kB ( |
| std_subgroupBroadcastFirst.ts | 24.58 kB ( |
| std_subgroupElect.ts | 24.58 kB ( |
| std_subgroupExclusiveAdd.ts | 24.58 kB ( |
| std_subgroupExclusiveMul.ts | 24.58 kB ( |
| std_subgroupInclusiveAdd.ts | 24.58 kB ( |
| std_subgroupInclusiveMul.ts | 24.58 kB ( |
| std_subgroupMax.ts | 24.58 kB ( |
| std_subgroupMin.ts | 24.58 kB ( |
| std_subgroupMul.ts | 24.58 kB ( |
| std_subgroupOr.ts | 24.58 kB ( |
| std_subgroupShuffle.ts | 24.58 kB ( |
| std_subgroupShuffleDown.ts | 24.58 kB ( |
| std_subgroupShuffleUp.ts | 24.58 kB ( |
| std_subgroupShuffleXor.ts | 24.58 kB ( |
| std_subgroupXor.ts | 24.58 kB ( |
| std_textureDimensions.ts | 26.19 kB ( |
| std_textureGather.ts | 26.19 kB ( |
| std_textureLoad.ts | 26.19 kB ( |
| std_textureSample.ts | 26.19 kB ( |
| std_textureSampleBaseClampToEdge.ts | 26.19 kB ( |
| std_textureSampleBias.ts | 26.19 kB ( |
| std_textureSampleCompare.ts | 26.19 kB ( |
| std_textureSampleCompareLevel.ts | 26.19 kB ( |
| std_textureSampleGrad.ts | 26.19 kB ( |
| std_textureSampleLevel.ts | 26.19 kB ( |
| std_textureStore.ts | 26.19 kB ( |
| d_isBuiltin.ts | 24.81 kB ( |
| d_sizeOf.ts | 24.86 kB ( |
| d_isContiguous.ts | 24.86 kB ( |
| d_getLongestContiguousPrefix.ts | 24.87 kB ( |
| d_size.ts | 26.54 kB ( |
| d_align.ts | 26.54 kB ( |
| d_location.ts | 26.55 kB ( |
| d_interpolate.ts | 26.55 kB ( |
| d_invariant.ts | 26.92 kB ( |
| std_pack2x16float.ts | 36.29 kB ( |
| std_pack4x8unorm.ts | 36.29 kB ( |
| std_unpack2x16float.ts | 36.29 kB ( |
| std_unpack4x8unorm.ts | 36.29 kB ( |
import { ... } in PR vs import * as ... in PR (is the library tree-Shakeable?):
| Test | tsdown |
|---|---|
| tgpu_init.ts | 256.85 kB ( |
| tgpu_initFromDevice.ts | 256.33 kB ( |
| tgpu_resolve.ts | 187.56 kB ( |
| tgpu_resolveWithContext.ts | 187.50 kB ( |
| tgpu_bindGroupLayout.ts | 69.09 kB ( |
| tgpu_mutableAccessor.ts | 66.09 kB ( |
| tgpu_accessor.ts | 66.08 kB ( |
| tgpu_privateVar.ts | 65.38 kB ( |
| tgpu_workgroupVar.ts | 65.38 kB ( |
| tgpu_const.ts | 64.60 kB ( |
| tgpu_fragmentFn.ts | 38.04 kB ( |
| tgpu_fn.ts | 38.01 kB ( |
| tgpu_vertexFn.ts | 37.85 kB ( |
| tgpu_computeFn.ts | 37.56 kB ( |
| tgpu_vertexLayout.ts | 26.78 kB ( |
| tgpu_comptime.ts | 14.47 kB ( |
| tgpu_unroll.ts | 1.66 kB ( |
| tgpu_slot.ts | 1.57 kB ( |
| tgpu_lazy.ts | 1.22 kB ( |
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.
Resolution Time Benchmark---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.91, 1.88, 4.14, 6.33, 7.40, 11.52, 22.06, 23.11]
line [0.98, 1.91, 4.04, 5.82, 7.72, 11.60, 21.99, 23.88]
line [0.86, 1.79, 3.82, 6.46, 7.70, 10.18, 21.75, 23.31]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.30, 0.50, 0.69, 0.84, 1.12, 1.13, 1.39, 1.60]
line [0.32, 0.56, 0.69, 0.81, 1.13, 1.20, 1.42, 1.57]
line [0.28, 0.52, 0.73, 0.84, 1.15, 1.23, 1.38, 1.59]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.82, 2.08, 4.15, 6.54, 12.55, 27.12, 54.52, 111.19]
line [0.87, 2.13, 4.11, 6.17, 12.11, 26.31, 52.73, 108.80]
line [0.83, 2.06, 3.52, 6.64, 12.23, 26.34, 54.24, 109.77]
|
There was a problem hiding this comment.
Pull request overview
This PR extends the TGSL snippet system to track possibleSideEffects more precisely, enabling additional safe codegen optimizations (notably allowing runtime-value ternaries when both branches are side-effect-free) and adds/expands tests around side-effect classification.
Changes:
- Thread
possibleSideEffectsthrough snippet creation/conversion paths (generator, conversion helpers, resource handles, raw snippets, etc.). - Relax ternary restrictions by permitting runtime conditions when both branches are proven side-effect-free (compiled to
select()). - Expand side-effect test coverage across literals, resources, accessors, indexing, conversions, refs, and logical operators.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/typegpu/tests/utils/parseResolved.ts | Improves return-expression extraction by using typed expression generation when return type is known; broadens expectSideEffects input types. |
| packages/typegpu/tests/tgsl/sideEffects.test.ts | Adds extensive side-effect classification tests for many TGSL constructs and resource reads. |
| packages/typegpu/src/tgsl/wgslGenerator.ts | Propagates possibleSideEffects across generated snippets and updates conditional/select and operator logic. |
| packages/typegpu/src/tgsl/generationHelpers.ts | Preserves possibleSideEffects through concretization/coercion helpers. |
| packages/typegpu/src/tgsl/forOfUtils.ts | Marks range/default iteration snippets as side-effect-free constants. |
| packages/typegpu/src/tgsl/conversion.ts | Preserves possibleSideEffects across snippet conversions/actions. |
| packages/typegpu/src/std/boolean.ts | Updates default precision snippet to be explicitly side-effect-free. |
| packages/typegpu/src/resolutionCtx.ts | Ensures snippet resolution preserves possibleSideEffects. |
| packages/typegpu/src/data/snippet.ts | Clarifies semantics/docs for possibleSideEffects and documents snip’s default behavior. |
| packages/typegpu/src/data/ref.ts | Threads possibleSideEffects through ref/deref operations. |
| packages/typegpu/src/core/variable/tgpuVariable.ts | Marks variable .$ snippet generation as side-effect-free. |
| packages/typegpu/src/core/unroll/tgpuUnroll.ts | Preserves possibleSideEffects when resolving/unrolling iterables. |
| packages/typegpu/src/core/texture/texture.ts | Marks texture-view handle snippets as side-effect-free. |
| packages/typegpu/src/core/texture/externalTexture.ts | Marks external texture handle snippets as side-effect-free. |
| packages/typegpu/src/core/slot/accessor.ts | Avoids eagerly executing GPU-callable functions during accessor resolution; propagates possibleSideEffects when calling functions. |
| packages/typegpu/src/core/sampler/sampler.ts | Marks sampler handle snippets as side-effect-free. |
| packages/typegpu/src/core/rawCodeSnippet/tgpuRawCodeSnippet.ts | Adds possibleSideEffects parameter to raw snippets and preserves it in .$ and resolution. |
| packages/typegpu/src/core/function/dualImpl.ts | Sets function call snippet possibleSideEffects using options.sideEffects and/or argument side effects. |
| apps/typegpu-docs/src/content/docs/apis/utils.mdx | Documents ternary relaxation for side-effect-free branches and the new possibleSideEffects raw snippet parameter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes — this PR threads possibleSideEffects through TypeGPU's snippet resolution and codegen so runtime ternaries can safely lower to select() when both branches are pure.
- Propagate side-effect bits across
snip()call sites in accessors, refs, conversions, struct/array constructors, unary/binary operators, loops, raw snippets, andDualImplcalls. - Mark pure sources as side-effect-free: literals, constants, parameters, variable reads, buffer/texture/sampler handles, and pure builtins.
- Keep impure calls conservative: user-defined functions and builtins like
discard/atomics remainpossibleSideEffects: true. - Expand test coverage in
sideEffects.test.tsfor pure and impure expression shapes, and update docs to reflect that runtime ternary conditions are allowed when branches are pure.
Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in the latest push; prior feedback still stands.
Reviewed changes since last review — the latest push restores the atomicLoad example in DualImplOptions.sideEffects docs and switches the punctuation back to ASCII arrows/hyphens.
- Docs-only polish in
packages/typegpu/src/core/function/dualImpl.ts.
Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Important
The latest push addresses the prior ! constant-fallback feedback, but the updated float NaN guard now misclassifies 0.0 as NaN.
Reviewed changes — the latest commit switches the unary ! constant fallback to possibleSideEffects: false and polishes the side-effect test suite, including renaming tests, using std.isBeingTranspiled, broadening expectSideEffects, and routing returns through _typedExpression in the test helper.
- Addressed prior feedback in
packages/typegpu/src/tgsl/wgslGenerator.tsby returningpossibleSideEffects: falsefor the constant!fallback. - Polished
sideEffects.test.tswith clearer test names, reorganized cases, additional!and conversion coverage, andstd.*builtins. - Updated
parseResolved.tsto type return expressions against the function return type and acceptTgpuFninexpectSideEffects.
Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in the latest push; prior feedback still stands.
Reviewed changes since last review — added coverage for zero-argument dualFn accessors and polished a side-effect test name.
- Added
allows zero-argument dualFntest inpackages/typegpu/tests/accessor.test.tsto verifytgpu.accessor(d.bool, std.subgroupElect).$resolves correctly. - Renamed side-effect test in
packages/typegpu/tests/tgsl/sideEffects.test.tsfrom "pure function access from accessor" to "pure function call from accessor".
Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — since the last review the branch picked up the latest main build-output changes and finished the mechanical possibleSideEffects refactor.
- Merged latest
mainbuild-output changes into the branch, switching thetypegpupackage to a Bun-basedbuild.mjsand removingtsdown. - Refactored snippet construction to use
withValueandwithDataTypehelpers acrossaccessor.ts,resolutionCtx.ts,tgpuUnroll.ts,ref.ts,conversion.ts, andgenerationHelpers.ts. - Updated
RefOperatorandderefSnippetto propagate the operand'spossibleSideEffectsbit instead of hard-coding it. - Addressed
oxlintfeedback by moving an inline disable comment inresolutionCtx.ts.
Kimi K2 (free via Pullfrog for OSS) | 𝕏
| } | ||
|
|
||
| return snip(stitch`(*${snippet})`, innerType, snippet.origin); | ||
| return snip(stitch`(*${snippet})`, innerType, snippet.origin, false); |
There was a problem hiding this comment.
is this always an identifier? why not repeat the snippet possibleSideEffects?
There was a problem hiding this comment.
maybe it is not
let ptr = *(&(array[impureInt()]))There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the last review — the latest commit refined side-effect propagation in accessor resolution and ref dereferencing.
- Used
ctx.resolveSnippet(snippet)inAccessorBase[$resolve]so the resolved snippet preservesdataType,origin, andpossibleSideEffectsthrough the central helper. - Updated
derefSnippetto propagate the operand'spossibleSideEffectsbit instead of forcingfalse, matching theRefOperatorresolution paths.
Targeted tests and the typecheck both pass.
Kimi K2 (free via Pullfrog for OSS) | 𝕏
iwoplaza
left a comment
There was a problem hiding this comment.
Awesome! 💜
Left a few nits
| return new SnippetImpl(snippet.value, dataType, snippet.origin, snippet.possibleSideEffects); | ||
| } | ||
|
|
||
| export function withValue(value: string, snippet: Snippet): ResolvedSnippet; |
There was a problem hiding this comment.
It could be that snippet is a comptime-known string
| export function withValue(value: string, snippet: Snippet): ResolvedSnippet; | |
| export function withValue(value: string, snippet: ResolvedSnippet): ResolvedSnippet; |
| @@ -1048,11 +1054,7 @@ export class ResolutionCtxImpl implements ResolutionCtx { | |||
| } | |||
|
|
|||
| resolveSnippet(snippet: Snippet): ResolvedSnippet { | |||
There was a problem hiding this comment.
Could you update the documentation for resolveSnippet in types.ts? I believe it no longer reflects the new implementation.
| const impureVec = () => { | ||
| 'use gpu'; | ||
| return d.vec3f(6, 6, 6); | ||
| }; | ||
| const impureInt = () => { | ||
| 'use gpu'; | ||
| return 666; | ||
| }; | ||
| const impureMat = () => { | ||
| 'use gpu'; | ||
| return d.mat2x2f(6, 6, 6, 6); | ||
| }; | ||
| const impureStruct = () => { | ||
| 'use gpu'; | ||
| return Boid(); | ||
| }; | ||
| const impureBool = () => { | ||
| 'use gpu'; | ||
| return true; | ||
| }; |
There was a problem hiding this comment.
These might soon become pure, once we can determine that a function call is side-effect free. I would make these truly impure by just updating a shared private variable.
| } | ||
|
|
||
| export function expectSideEffects(cb: () => unknown): Assertion<boolean> { | ||
| export function expectSideEffects(cb: TgpuFn | (() => unknown)): Assertion<boolean> { |
There was a problem hiding this comment.
| export function expectSideEffects(cb: TgpuFn | (() => unknown)): Assertion<boolean> { | |
| export function expectSideEffects(cb: () => unknown): Assertion<boolean> { |

possibleSideEffects$gpuCallableto theaccessorstpgu.rawCodeSnippetoptionalpossibleSideEffectsfieldCloses #2646.