Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebGPU code works in browser but not in Deno, divergence from specification? #25915

Open
emilfihlman opened this issue Sep 27, 2024 · 13 comments
Labels
bug Something isn't working correctly upstream Changes in upstream are required to solve these issues webgpu WebGPU API

Comments

@emilfihlman
Copy link

emilfihlman commented Sep 27, 2024

Version: deno 2.0.0-rc.6

The linked project
https://github.com/dezmou/SHA256-WebGPU
works in browser (latest Chrome stable on Windows), but doesn't work in Deno.

Deno fails in multiple ways:

Firstly

Device::create_shader_module error: 
Shader '' parsing error: failed to convert expression to a concrete type: the concrete type `i32` cannot represent the abstract value `3144134277` accurately
    ┌─ wgsl:183:20
    │
183 │     ctx.state[1] = 0xbb67ae85;
    │                    ^^^^^^^^^^ this expression has type {AbstractInt}
    │
    = note: the expression should have been converted to have i32 scalar type


00000000000000000000000000000000

If fixed by appending u to the end of these constants, Deno fails again with

Device::create_shader_module error: 
Shader validation error: 
   ┌─ :33:3
   │
33 │   fn EP0(x : u32) -> u32{return (ROTRIGHT(x,2) ^ ROTRIGHT(x,13) ^ ROTRIGHT(x,22));}
   │   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   │   │                              │          │
   │   │                              │          naga::Expression [2]
   │   │                              invalid function call
   │   naga::Function [5]


00000000000000000000000000000000

Deno shouldn't prevent code that works in browser from working in Deno.

Test was performed by concatenating sha256shader.js + ' \n' + sha256.js and the lines
const result = await sha256("");
console.log(result);

@crowlKats crowlKats added webgpu WebGPU API needs investigation requires further investigation before determining if it is an issue or not bug Something isn't working correctly upstream Changes in upstream are required to solve these issues and removed needs investigation requires further investigation before determining if it is an issue or not labels Sep 27, 2024
@crowlKats
Copy link
Member

This is stuck on upstream naga: gfx-rs/wgpu#4400

@emilfihlman
Copy link
Author

This is stuck on upstream naga: gfx-rs/wgpu#4400

Both issues I ran into?

@emilfihlman
Copy link
Author

This is stuck on upstream naga: gfx-rs/wgpu#4400

And thank you for figuring this out, I really appreciate it!

@ErichDonGubler
Copy link

ErichDonGubler commented Sep 28, 2024

Mm, after appending a u suffix to the numeric literals that run into gfx-rs/wgpu#4400, it appears that there's also a blocker on gfx-rs/wgpu#4337, when the k variable is indexed by the runtime-known i variable in the linked shader source (https://github.com/dezmou/SHA256-WebGPU/blob/ad75b542a9c761f0b7495b427093b4bd4920ff34/sha256Shader.js).

@emilfihlman
Copy link
Author

Yeah indeed that seems to be so

Device::create_shader_module error: 
Shader validation error: 
   ┌─ :38:3
   │  
38 │ ╭   fn sha256_transform(ctx : ptr<function, SHA256_CTX>)
39 │ │   {
40 │ │     var a : u32;
41 │ │     var b : u32;
   · │
77 │ │       t1 = h + EP1(e) + CH(e,f,g) + k[i] + m[i];
   │ │                                     ^^^^ naga::Expression [131]
   · │
96 │ │     (*ctx).state[6] += g;
97 │ │     (*ctx).state[7] += h;
   │ ╰─────────────────────────^ naga::Function [9]


00000000000000000000000000000000

@emilfihlman
Copy link
Author

emilfihlman commented Sep 28, 2024

Issue does not go away even if one makes a separate variable that's tightly bound compile time:

    for (var ii : u32 = 0; ii < 64; ii++) {
      t1 = h + EP1(e) + CH(e,f,g) + k[ii] + m[ii];
      t2 = EP0(a) + MAJ(a,b,c);
      h = g;
      g = f;
      f = e;
      e = d + t1;
      d = c;
      c = b;
      b = a;
      a = t1 + t2;
      i = ii;
    }
Device::create_shader_module error: 
Shader validation error: 
   ┌─ :38:3
   │  
38 │ ╭   fn sha256_transform(ctx : ptr<function, SHA256_CTX>)
39 │ │   {
40 │ │     var a : u32;
41 │ │     var b : u32;
   · │
77 │ │       t1 = h + EP1(e) + CH(e,f,g) + k[ii] + m[ii];
   │ │                                     ^^^^^ naga::Expression [133]
   · │
97 │ │     (*ctx).state[6] += g;
98 │ │     (*ctx).state[7] += h;
   │ ╰─────────────────────────^ naga::Function [9]


00000000000000000000000000000000

@ErichDonGubler
Copy link

@emilfihlman: The ii variable is still a runtime variable, and not constant. You need a const to get past that (very restrictive) error. 😓

@emilfihlman
Copy link
Author

@emilfihlman: The ii variable is still a runtime variable, and not constant. You need a const to get past that (very restrictive) error. 😓

How is this supposed to work? 😅

@ErichDonGubler
Copy link

@emilfihlman: It doesn't, which is why the standard states it should be allowed, and Naga has an issue to converge with the spec. 😅

PRs very welcome!

@emilfihlman
Copy link
Author

@ErichDonGubler thank you for explaining, and o7 That's really as much as I can say 😔

But what I'm hearing is that basically, I could manually unroll the loops and get it done that way maybe : D

@emilfihlman
Copy link
Author

Oh @ErichDonGubler and so does this mean that currently naga does not support loops? Or like, I'm having a hard time figuring out how this couldn't work and that surely there's a way to rewrite this to work for now.

@ErichDonGubler
Copy link

ErichDonGubler commented Sep 29, 2024

@emilfihlman: uh

Your question baffles me a bit, but your solution might be valid. The bug I linked to (gfx-rs/wgpu#4337) describes the issue: one can't index const arrays with non-const indices. Loops should otherwise fully supported minus maybe the question of uniformity analysis.

@emilfihlman
Copy link
Author

Ah, so then the solution might also be to just turn the array into a non const, though I wonder what perf cost that has, hopefully not much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly upstream Changes in upstream are required to solve these issues webgpu WebGPU API
Projects
None yet
Development

No branches or pull requests

3 participants