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

For loop not supported by Naga #1032

Closed
Kethku opened this issue Apr 14, 2023 · 3 comments
Closed

For loop not supported by Naga #1032

Kethku opened this issue Apr 14, 2023 · 3 comments
Labels
t: external Issues not about rust-gpu itself, but related enough to be tracked.

Comments

@Kethku
Copy link

Kethku commented Apr 14, 2023

Expected Behaviour

Disabling spirt should not fix runtime errors.

Example & Steps To Reproduce

Compiling the following:

#[spirv(fragment)]
pub fn fragment(
    #[spirv(storage_buffer, descriptor_set = 0, binding = 0)] quads: &[InstancedQuad],
    #[spirv(descriptor_set = 1, binding = 0)] surface: &Image2d,
    #[spirv(descriptor_set = 1, binding = 1)] sampler: &Sampler,
    #[spirv(push_constant)] constants: &ShaderConstants,
    #[spirv(flat)] instance_index: i32,
    #[spirv(frag_coord)] surface_position: Vec4,
    out_color: &mut Vec4,
) {
    let quad = quads[instance_index as usize];

    if quad.blur != 0 {
        for y in -quad.blur..(quad.blur + 1) {
            for x in -quad.blur..(quad.blur + 1) {
                let offset = vec2(x as f32, y as f32);
                let sample_pos = (surface_position.xy() + offset) / constants.surface_size;
                let sample = surface.sample_by_lod(*sampler, sample_pos, 0.);
                *out_color += sample / (quad.blur * quad.blur) as f32;
            }
        }

        *out_color = *out_color * quad.color;
    } else {
        *out_color = quad.color;
    }
}

Causes a panic at runtime in the naga crate:

thread 'main' panicked at 'assertion failed: prev.is_none()', C:\Users\KaySimmons\.cargo\git\checkouts\naga-dbb2b19faed4
9210\f59668c\src\front\spv\mod.rs:3151:25

Adding the --no-spirt compile arg fixes this issue.

This issue upstream seems relevant: gfx-rs/wgpu#4388

System Info

  • Rust: rustc 1.68.0-nightly (5ce39f42b 2023-01-20)
  • OS: Windows 11
  • GPU: Intel(R) UHD Graphics
  • SPIR-V: SPIRV-Tools v2023.2 v2022.4-135-g44d72a9b
@Kethku Kethku added the t: bug Something isn't working label Apr 14, 2023
@Kethku Kethku changed the title (my bug report) While loop causes runtime panic Apr 14, 2023
@Kethku Kethku changed the title While loop causes runtime panic For loop causes runtime panic Apr 14, 2023
@Kethku
Copy link
Author

Kethku commented Apr 14, 2023

Rewriting the for loop as a loop with break, or while loop doesn't fix the issue. Similarly doing a single iteration without a loop works fine, so the loop itself seems to be the issue when using spirt

@eddyb eddyb changed the title For loop causes runtime panic For loop not supported by Naga Apr 15, 2023
@eddyb
Copy link
Contributor

eddyb commented Apr 15, 2023

See these links for more details:

Assuming you're using Naga 0.11, this may work:

[patch.crates-io]
naga = { git = "https://github.com/EmbarkStudios/naga", branch = "spv-in-break-if-v0.11.0" }

This is 100% a Naga bug (they're not implementing a legal form of SPIR-V control-flow) and will be fixed independently of Rust-GPU.

@eddyb eddyb added t: external Issues not about rust-gpu itself, but related enough to be tracked. and removed t: bug Something isn't working labels Apr 15, 2023
@Kethku
Copy link
Author

Kethku commented Apr 18, 2023

Sweet! Thank you for the details.

@Kethku Kethku closed this as completed Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: external Issues not about rust-gpu itself, but related enough to be tracked.
Projects
None yet
Development

No branches or pull requests

2 participants