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

Implement WGSL division by zero semantics #4385

Open
hasali19 opened this issue May 3, 2022 · 6 comments
Open

Implement WGSL division by zero semantics #4385

hasali19 opened this issue May 3, 2022 · 6 comments
Labels
area: naga back-end Outputs of naga shader conversion lang: GLSL OpenGL Shading Language lang: HLSL D3D Shading Language lang: Metal Metal Shading Language lang: SPIR-V Vulkan's Shading Language naga Shader Translator type: bug Something isn't working

Comments

@hasali19
Copy link
Contributor

hasali19 commented May 3, 2022

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 teoxoy added kind: bug area: naga back-end Outputs of naga shader conversion lang: HLSL D3D Shading Language labels May 3, 2022
@teoxoy
Copy link
Member

teoxoy commented May 3, 2022

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
Copy link
Member

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.

@cwfitzgerald cwfitzgerald transferred this issue from gfx-rs/naga Oct 25, 2023
@cwfitzgerald cwfitzgerald added naga Shader Translator type: bug Something isn't working and removed kind: bug labels Oct 25, 2023
@teoxoy teoxoy added this to the WebGPU Specification V1 milestone Nov 3, 2023
@jimblandy
Copy link
Member

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 jimblandy changed the title [hlsl-out] error X4010: Unsigned integer divide by zero Implement WGSL division by zero semantics Dec 14, 2023
@teoxoy teoxoy added lang: SPIR-V Vulkan's Shading Language lang: GLSL OpenGL Shading Language lang: Metal Metal Shading Language labels Dec 14, 2023
@jimblandy
Copy link
Member

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
Copy link
Member

teoxoy commented Jun 4, 2024

More specifically, this is what this issue is tracking:

If e2 is zero:

@teoxoy
Copy link
Member

teoxoy commented Jun 4, 2024

The remainder operator also needs something similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga back-end Outputs of naga shader conversion lang: GLSL OpenGL Shading Language lang: HLSL D3D Shading Language lang: Metal Metal Shading Language lang: SPIR-V Vulkan's Shading Language naga Shader Translator type: bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

4 participants