gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.4k stars 908 forks source link

Implement WGSL division by zero semantics #4385

Open hasali19 opened 2 years ago

hasali19 commented 2 years ago

I've found what seems to be another hlsl compilation failure caused by FXC. This one incorrectly reports a divide by zero error. Interestingly it's only rejected when the divide call is in a nested loop.

fn divide(a: i32, b: i32) -> i32 {
    if (b == 0) {
        return a / 2;
    } else {
        return a / b;
    }
}

struct Out {
    value: u32,
}

@group(0)
@binding(0)
var<storage, read_write> s_out: Out;

@compute
@workgroup_size(1)
fn main() {
    loop {
        loop {
            var x = divide(0, 0);
            break;
        }
        break;
    }
    s_out.value = 0u;
}

Output:

The application panicked (crashed).
Message:  wgpu error: Validation Error

Caused by:
    In Device::create_compute_pipeline
    Internal error: D3DCompile error (0x80004005): \\wsl.localhost\Ubuntu\home\hasan\dev\wgslsmith\Shader@0x0000025A33C8DBA0(19,17-21): error X4010: Unsigned integer divide by zero
\\wsl.localhost\Ubuntu\home\hasan\dev\wgslsmith\Shader@0x0000025A33C8DBA0(28,5-15): warning X3557: loop only executes for 0 iteration(s), forcing loop to unroll

Tint seems to be able to satisfy FXC by adding an extra check on b in the divide function: https://shader-playground.timjones.io/ac86c91d9f0f9b09c7668fa99534fe3f.

teoxoy commented 2 years ago

The WGSL spec requires us to add some checks (akin to tint's) to handle undefined behavior of some operators and built-in functions which we aren't currently doing.

jimblandy commented 2 years ago

If implementing WGSL's division semantics happens to work around this FXC bug, that'd be great. The Naga HLSL back end code implementing the workaround should mention this FXC bug, so future developers will know we have unexpected constraints we're trying to meet.

jimblandy commented 10 months ago

Even once we fully implement WGSL's semantics for division by zero, there will still be times where wgpu simply can't anticipate what FXC will reject, and we will need to handle those cases as unexpected platform errors like "out of memory".

jimblandy commented 10 months ago

WGSL requires that division by zero in a runtime expression returns the left operand (surprising!). Implementing that check should clear this up on FXC, so we're re-purposing this issue to represent that project.

teoxoy commented 4 months ago

More specifically, this is what this issue is tracking:

If e2 is zero:

teoxoy commented 4 months ago

The remainder operator also needs something similar.