denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.09k stars 5.31k forks source link

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

Open emilfihlman opened 2 weeks ago

emilfihlman commented 2 weeks ago

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 commented 2 weeks ago

This is stuck on upstream naga: https://github.com/gfx-rs/wgpu/issues/4400

emilfihlman commented 2 weeks ago

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

Both issues I ran into?

emilfihlman commented 2 weeks ago

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

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

ErichDonGubler commented 2 weeks ago

Mm, after appending a u suffix to the numeric literals that run into https://github.com/gfx-rs/wgpu/issues/4400, it appears that there's also a blocker on https://github.com/gfx-rs/wgpu/issues/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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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

emilfihlman commented 2 weeks ago

@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 commented 2 weeks ago

@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 commented 2 weeks ago

@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 commented 2 weeks ago

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 commented 2 weeks ago

@emilfihlman: uh

Your question baffles me a bit, but your solution might be valid. The bug I linked to (https://github.com/gfx-rs/wgpu/issues/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 commented 2 weeks ago

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.

ErichDonGubler commented 1 week ago

@emilfihlman: With the merging of https://github.com/gfx-rs/wgpu/pull/6188, I'm able to successfully compile the shader if I apply the following diff. to work around https://github.com/gfx-rs/wgpu/issues/4400:

diff --git a/sha256Shader.js b/sha256Shader.js
index 18df933..44f05b0 100644
--- a/sha256Shader.js
+++ b/sha256Shader.js
@@ -30,10 +30,10 @@ struct SHA256_CTX {

   fn CH(x : u32, y : u32, z : u32) -> u32{return (((x) & (y)) ^ (~(x) & (z)));}
   fn MAJ(x : u32, y : u32, z : u32) -> u32{return (((x) & (y)) ^ ((x) & (z)) ^ ((y) & (z)));}
-  fn EP0(x : u32) -> u32{return (ROTRIGHT(x,2) ^ ROTRIGHT(x,13) ^ ROTRIGHT(x,22));}
-  fn EP1(x : u32) -> u32{return (ROTRIGHT(x,6) ^ ROTRIGHT(x,11) ^ ROTRIGHT(x,25));}
-  fn SIG0(x : u32) -> u32{return (ROTRIGHT(x,7) ^ ROTRIGHT(x,18) ^ ((x) >> 3));}
-  fn SIG1(x : u32) -> u32{return (ROTRIGHT(x,17) ^ ROTRIGHT(x,19) ^ ((x) >> 10));}
+  fn EP0(x : u32) -> u32{return (ROTRIGHT(x,2u) ^ ROTRIGHT(x,13u) ^ ROTRIGHT(x,22u));}
+  fn EP1(x : u32) -> u32{return (ROTRIGHT(x,6u) ^ ROTRIGHT(x,11u) ^ ROTRIGHT(x,25u));}
+  fn SIG0(x : u32) -> u32{return (ROTRIGHT(x,7u) ^ ROTRIGHT(x,18u) ^ ((x) >> 3u));}
+  fn SIG1(x : u32) -> u32{return (ROTRIGHT(x,17u) ^ ROTRIGHT(x,19u) ^ ((x) >> 10u));}

   fn sha256_transform(ctx : ptr<function, SHA256_CTX>)
   {
@@ -55,7 +55,7 @@ struct SHA256_CTX {
     while(i < 16) {
       m[i] = ((*ctx).data[j] << 24) | ((*ctx).data[j + 1] << 16) | ((*ctx).data[j + 2] << 8) | ((*ctx).data[j + 3]);
       i++;
-      j += 4;
+      j += 4u;
     }            

     while(i < 64) {
@@ -72,7 +72,7 @@ struct SHA256_CTX {
     g = (*ctx).state[6];
     h = (*ctx).state[7];

-    i = 0;
+    i = 0u;
     for (; i < 64; i++) {
       t1 = h + EP1(e) + CH(e,f,g) + k[i] + m[i];
       t2 = EP0(a) + MAJ(a,b,c);
@@ -109,10 +109,10 @@ struct SHA256_CTX {
         if ((*ctx).bitlen[0] > 0xffffffff - (512)){
           (*ctx).bitlen[1]++;
         }
-        (*ctx).bitlen[0] += 512;
+        (*ctx).bitlen[0] += 512u;

-        (*ctx).datalen = 0;
+        (*ctx).datalen = 0u;
       }
     }
   }
@@ -122,23 +122,23 @@ struct SHA256_CTX {
     var i : u32 = (*ctx).datalen;

     if ((*ctx).datalen < 56) {
-      (*ctx).data[i] = 0x80;
+      (*ctx).data[i] = 0x80u;
         i++;
       while (i < 56){
-        (*ctx).data[i] = 0x00;
+        (*ctx).data[i] = 0x00u;
         i++;
       }
     }
     else {
-      (*ctx).data[i] = 0x80;
+      (*ctx).data[i] = 0x80u;
       i++;
       while (i < 64){
-        (*ctx).data[i] = 0x00;
+        (*ctx).data[i] = 0x00u;
         i++;
       }
       sha256_transform(ctx);
       for (var i = 0; i < 56 ; i++) {
-        (*ctx).data[i] = 0;
+        (*ctx).data[i] = 0u;
       }
     }

@@ -158,7 +158,7 @@ struct SHA256_CTX {
     (*ctx).data[56] = (*ctx).bitlen[1] >> 24;
     sha256_transform(ctx);

-    for (i = 0; i < 4; i++) {
+    for (i = 0u; i < 4u; i++) {
       (*hash)[i] = ((*ctx).state[0] >> (24 - i * 8)) & 0x000000ff;
       (*hash)[i + 4] = ((*ctx).state[1] >> (24 - i * 8)) & 0x000000ff;
       (*hash)[i + 8] = ((*ctx).state[2] >> (24 - i * 8)) & 0x000000ff;
@@ -176,17 +176,17 @@ struct SHA256_CTX {
     var buf : array<u32, SHA256_BLOCK_SIZE>;

     // CTX INIT
-    ctx.datalen = 0;
-    ctx.bitlen[0] = 0;
-    ctx.bitlen[1] = 0;
-    ctx.state[0] = 0x6a09e667;
-    ctx.state[1] = 0xbb67ae85;
-    ctx.state[2] = 0x3c6ef372;
-    ctx.state[3] = 0xa54ff53a;
-    ctx.state[4] = 0x510e527f;
-    ctx.state[5] = 0x9b05688c;
-    ctx.state[6] = 0x1f83d9ab;
-    ctx.state[7] = 0x5be0cd19;
+    ctx.datalen = 0u;
+    ctx.bitlen[0] = 0u;
+    ctx.bitlen[1] = 0u;
+    ctx.state[0] = 0x6a09e667u;
+    ctx.state[1] = 0xbb67ae85u;
+    ctx.state[2] = 0x3c6ef372u;
+    ctx.state[3] = 0xa54ff53au;
+    ctx.state[4] = 0x510e527fu;
+    ctx.state[5] = 0x9b05688cu;
+    ctx.state[6] = 0x1f83d9abu;
+    ctx.state[7] = 0x5be0cd19u;

     sha256_update(&ctx, inputSize[0]);
     sha256_final(&ctx, &buf);
emilfihlman commented 1 week ago

Awesome! Thank you!