Add 7 new difftests and push constants support #321

LegNeato · 2025-07-04T09:18:30Z

I wanted to increase coverage, so I worked with AI to bang out some tests and infra changes.

New tests:

bitwise_ops: bit manipulation operations
trig_ops: trigonometric functions
control_flow_complex: nested loops and complex control flow
vector_swizzle: vector component access and swizzling
memory_barriers: workgroup memory synchronization
vector_extract_insert: dynamic vector element access
push_constants: push constants in compute shaders

Infrastructure changes:

Add WgpuComputeTestPushConstants for push constant support
Enable PUSH_CONSTANTS feature in wgpu when needed

Firestar99 · 2025-07-04T10:39:14Z

I'd love to have these for testing my const folding in #317

Firestar99 · 2025-07-04T10:47:58Z

CI Ubuntu reports these failures:
difftests::lang::core::ops::math_ops
difftests::lang::core::ops::matrix_ops
difftests::lang::core::ops::vector_ops

On my machine I get these failures:
difftests::lang::core::ops::math_ops
difftests::lang::core::ops::matrix_ops

Could this be float inaccuracies between vendors?

LegNeato · 2025-07-04T16:21:16Z

Yeah, I think it is floats because they all passed locally after I took care of some existing float issues. I'll take a look and either round or omit.

LegNeato · 2025-07-06T15:45:33Z

I added a compat_round! macro that handled most of the differences, but could not get all of them. So I added epsilon support when comparing. And then because I was debugging with python scripts I added better debug output when dealing with f32s. And because I did that I reworked all the output cases to match.

This is a lot of code and I don't really expect you to review it...AI wrote a lot of it and I reviewed and tweaked. I figure we'll iterate on main as it is super self-contained. I can probably break it up if you want though!

As an aside, I think some of these should be runtests, as I am not sure what value we get diffing with wgsl for all of them. BTW, if test times get long we can shard in CI as theoretically we could have 1 job for each test as they are unrelated.

LegNeato · 2025-07-06T15:46:08Z

(note it is not set to automerge as I'll want to squash first)

tests/difftests/lib/src/scaffold/compute/vulkano.rs

LegNeato · 2025-07-07T22:11:30Z

Ok, the windows issue is definitely a wgpu + naga (dxc) bug.

LegNeato · 2025-07-08T15:36:26Z

I've pulled out the repo for the wgpu/naga bug: https://github.com/LegNeato/wgpu-workgroup-memory-bug

Firestar99

What I love about this PR:

we print any detected changes into the logs, so you can easily see in CI the detected difference without having to boot up that platform
epsilon for floating point
the ComputeShader trait to make it easy to switch out shaders
Skip cause we'll likely need it in more places than this

I wonder if we should separate out the difftest improvements from the actual difftests in separate PRs?

The only major concern I have is that there's a lot of copy-paste between the different variants of a single test case. They usually only differ by the impl ComputeShader given as a arg. Doesn't have to be this PR, can be a followup, but I'd like to see those almost identical mains collapse. Make all variants of a single difftest a single crate. Some ideas around how that could be done:

Features. The runner reads all features of the single difftest crate (via cargo_metadata) and runs the test with one feature being active at a time.
Args. Read from some metadata toml file the variants you want and pass them in as args, via struct Config. Then switching between shaders is just a match, not some #[cfg(...)] monster. Would require every variant to depend on wgpu, naga and ash, but we need to compile all of them anyway.
Metadata could be like github runners to allow cartesian product:

# skip wgsl and cpu variant
[variants]
shader = ["rust", "wgsl"]
runner = ["wgpu", "ash", "cpu"]

I'd perfer metadata + args.

.github/workflows/ci.yaml

tests/difftests/README.md

Firestar99 · 2025-07-09T11:04:48Z

tests/difftests/lib/src/config.rs

+        Self {
+            epsilon: Some(epsilon),
+            ..Default::default()
+        }


Default output_type is Raw, where epsilon doesn't make any sense. Rename it to with_f32(epsilon: f32) -> Self and adjust the current docs?

tests/difftests/lib/src/scaffold/compute/ash.rs

Firestar99 · 2025-07-09T11:15:09Z

tests/difftests/lib/src/scaffold/compute/ash.rs

+
+    fn run_compute(
+        &self,
+        spirv_bytes: &[u8],


why does this take spirv_bytes as a &[u8] and not something like impl ComputeShader like wgpu does? Would need to adjust the ComputeShader trait to support just returning like a spirv_bytes: &[u8]. That would also remove a lot of the custom code in the single ash test we have.

I'd be fine deferring this to a followup as well

LegNeato · 2025-07-09T13:37:04Z

I wonder if we should separate out the difftest improvements from the actual difftests in separate PRs?

I'll look into how hard this is.

The only major concern I have is that there's a lot of copy-paste between the different variants of a single test case. They usually only differ by the impl ComputeShader given as a arg. Doesn't have to be this PR, can be a followup, but I'd like to see those almost identical mains collapse. Make all variants of a single difftest a single crate. Some ideas around how that could be done:

Yeah, I think once we get the kinks worked out I want to do stuff like compiletest where we just drop a .rs file with magic comments and everything else is taken care of. There is too much boiler plate right now, but I didn't want to get too fancy at the start.

The reason why I started it this way is I wanted to make sure we could test things like 3rd party deps, setting values in Cargo.toml, using SpirvBuilder from build.rs, etc. When I thought about all the possibilities I wanted to support, I realized I was just doing a giant DSL and it would be simpler to just use standard Rust 😄 . We can always use standard rust tools to reduce boilerplate like the include!() I used in one of the tests so the same shader code is used between ash and wgpu variants or macros and such.

Firestar99 · 2025-07-09T18:23:17Z

It's probably just too little ε but this fails on my machine (Ryzen 680M RDNA2 iGPU):

test difftests::lang::core::ops::matrix_ops ... thread 'main' panicked at tests/difftests/bin/src/main.rs:108:49:
Outputs differ:

matrix_ops (lang/core/ops/matrix_ops)
F32, ε=0.00001 • 8 differences • max: 1.526e-5 (0.00%)
─────────────────────────────────────────────────────────────────

▪ matrix_ops-rust
  → Raw:  /tmp/.tmpgPQ1J2 (6.2 KiB)
  → Text: /tmp/.tmpgPQ1J2.txt

▪ matrix_ops-wgsl
  → Raw:  /tmp/.tmpHEcalv (6.2 KiB)
  → Text: /tmp/.tmpHEcalv.txt

┌──────┬─────────────────┬─────────────────┬──────────┬───────┐
│  #   │ matrix_ops-rust │ matrix_ops-wgsl │  Δ abs   │  Δ %  │
├──────┼─────────────────┼─────────────────┼──────────┼───────┤
│ 185  │ -93.107101440   │ -93.107086182   │ 1.526e-5 │ 0.00% │
├──────┼─────────────────┼─────────────────┼──────────┼───────┤
│ 385  │ -93.107101440   │ -93.107086182   │ 1.526e-5 │ 0.00% │
├──────┼─────────────────┼─────────────────┼──────────┼───────┤
│ 585  │ -93.107101440   │ -93.107086182   │ 1.526e-5 │ 0.00% │
├──────┼─────────────────┼─────────────────┼──────────┼───────┤
│ 785  │ -93.107101440   │ -93.107086182   │ 1.526e-5 │ 0.00% │
├──────┼─────────────────┼─────────────────┼──────────┼───────┤
│ 985  │ -93.107101440   │ -93.107086182   │ 1.526e-5 │ 0.00% │
├──────┼─────────────────┼─────────────────┼──────────┼───────┤
│ 1185 │ -93.107101440   │ -93.107086182   │ 1.526e-5 │ 0.00% │
├──────┼─────────────────┼─────────────────┼──────────┼───────┤
│ 1385 │ -93.107101440   │ -93.107086182   │ 1.526e-5 │ 0.00% │
├──────┼─────────────────┼─────────────────┼──────────┼───────┤
│ 1585 │ -93.107101440   │ -93.107086182   │ 1.526e-5 │ 0.00% │
└──────┴─────────────────┴─────────────────┴──────────┴───────┘

(did I mention that I love this diff? ❤️ )

LegNeato · 2025-07-09T19:59:56Z

@Firestar99 can you try the latest on your machine to confirm bumping the epsilon makes the tests pass?

I tried to slice this up into more logical commits but it is painful. I'll try to at least clean the history up a bit.

New tests: - bitwise_ops: bit manipulation operations - trig_ops: trigonometric functions - control_flow_complex: nested loops and complex control flow - vector_swizzle: vector component access and swizzling - memory_barriers: workgroup memory synchronization - vector_extract_insert: dynamic vector element access - push_constants: push constants in compute shaders Infrastructure changes: - Add WgpuComputeTestPushConstants for push constant support - Enable PUSH_CONSTANTS feature in wgpu when needed - Register all existing unregistered tests in workspace

- Add a round macro for cross-plat compat - Add epsilon-based floating point comparison - Add human-readable output for float/int data - Add test coverage for new stuff - Update documentation

Also don't show stats/counts when delaing with raw diffs

Firestar99 · 2025-07-10T11:04:20Z

The larger epsilon did fix that test, everything passes now!

Firestar99 · 2025-07-10T11:24:55Z

Just FYI, difftest are not clippy checked in CI, fixed in #329

Firestar99 · 2025-07-11T15:55:39Z

Apart from a rebase and #321 (comment), this would be ready to be merged, right?

LegNeato requested review from eddyb, Firestar99 and schell as code owners July 4, 2025 09:18

LegNeato marked this pull request as draft July 5, 2025 16:44

LegNeato force-pushed the moardifftests branch from 32f23b2 to d8a7f8b Compare July 6, 2025 15:17

LegNeato marked this pull request as ready for review July 6, 2025 15:45

Firestar99 reviewed Jul 7, 2025

View reviewed changes

tests/difftests/lib/src/scaffold/compute/vulkano.rs Outdated Show resolved Hide resolved

Firestar99 requested changes Jul 9, 2025

View reviewed changes

LegNeato force-pushed the moardifftests branch from 59909d2 to 27b9ee6 Compare July 9, 2025 15:02

Firestar99 mentioned this pull request Jul 9, 2025

better const folding for int and bool #317

Open

LegNeato force-pushed the moardifftests branch from 37d879c to 1810d2a Compare July 9, 2025 21:01

LegNeato added 9 commits July 10, 2025 12:03

Round floats to prevent deltas across platforms

0dd898b

Add epsilon support and improve difftest framework

75c3229

- Add a round macro for cross-plat compat - Add epsilon-based floating point comparison - Add human-readable output for float/int data - Add test coverage for new stuff - Update documentation

Fix cargo deny by turning off unused tabled features

45a6c04

Use Default impl

7b6cc69

Switch to static DXC

b225bbd

Add support for U32

1912ffa

Also don't show stats/counts when delaing with raw diffs

Add final barrier to (hopefully) fix windows

47d80a2

Add Skip and add a vulkano test to try to debug windows

7878d06

LegNeato and others added 8 commits July 10, 2025 12:03

Fix pdb/path issue on windows

6218534

Ash variant of a difftest

e30e6bd

Vulkan on windows via swiftshader

88d07bf

Add support for diffing Raw output

84fd829

grab spirv-builder from difftest to pick up features

c62de95

difftest ash: cleanup code

7e4dbd9

difftest ash: allocate descriptor pool just as required

ef99192

Rustfmt after rebasing on new rust version

4bdf355

LegNeato force-pushed the moardifftests branch from 1faa42f to 4bdf355 Compare July 10, 2025 10:09

Add 7 new difftests and push constants support #321

Are you sure you want to change the base?

Add 7 new difftests and push constants support #321

Conversation

LegNeato commented Jul 4, 2025

Uh oh!

Firestar99 commented Jul 4, 2025

Uh oh!

Firestar99 commented Jul 4, 2025

Uh oh!

LegNeato commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LegNeato commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LegNeato commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

LegNeato commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LegNeato commented Jul 8, 2025

Uh oh!

Firestar99 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Firestar99 Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Firestar99 Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Firestar99 Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

LegNeato commented Jul 9, 2025

Uh oh!

Firestar99 commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LegNeato commented Jul 9, 2025

Uh oh!

Firestar99 commented Jul 10, 2025

Uh oh!

Firestar99 commented Jul 10, 2025

Uh oh!

Firestar99 commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

LegNeato commented Jul 4, 2025 •

edited

Loading

LegNeato commented Jul 6, 2025 •

edited

Loading

LegNeato commented Jul 6, 2025 •

edited

Loading

LegNeato commented Jul 7, 2025 •

edited

Loading

Firestar99 left a comment •

edited

Loading

Firestar99 commented Jul 9, 2025 •

edited

Loading

Firestar99 commented Jul 11, 2025 •

edited

Loading