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

thread 'main' panicked at 'assertion failed: prev.is_none()', src/front/spv/mod.rs:2903:25 #4388

Open
Jack-Clark opened this issue May 10, 2022 · 4 comments
Labels
area: naga front-end lang: SPIR-V Vulkan's Shading Language naga Shader Translator type: bug Something isn't working

Comments

@Jack-Clark
Copy link

Running:

target/debug/naga break-error.spv break-error.comp

Gives:

thread 'main' panicked at 'assertion failed: prev.is_none()', src/front/spv/mod.rs:2903:25
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Aborted (core dumped)

If I run a release build, then I get:

Function [1] '' is invalid: 
	The `break` is used outside of a `loop` or `switch` context
Generating glsl output requires validation to succeed, and it failed in a previous step

I can also produce a slightly different error in both debug and release mode using a different example:

target/release/naga continue-error.spv continue-error.comp

Outputs:

Function [1] '' is invalid: 
	The `continue` is used outside of a `loop` context
Generating glsl output requires validation to succeed, and it failed in a previous step

I've tried .comp .metal and .hlsl as translation targets and I get the same result regardless. I reproduced this on main (commit b3d5e6d). Here is an archive with the code examples

@teoxoy
Copy link
Member

teoxoy commented May 10, 2022

Thanks for the report!

This looks like some more complex (relative to what we are currently handling) control flow.

I generated the following images via spirv-cfg and dot -Tpng.

continue-error

continue-error

break-error

break-error

I'm not familiar with SPIR-V's control flow @kvark or @JCapucho might know more.

@Jack-Clark
Copy link
Author

No problem! Here is an archive containing a simpler example for the break-error case. This has the following CFG:

simpler-break-error

@JCapucho
Copy link
Collaborator

The problem is that our BranchConditional code expects to either branch to a new block or to a merge/continue block, these expectations are mostly correct with the slight problem that one block of a loop (called back-edge) can branch into the loop header, this block must post dominate the continue target so a quick fix for your problem would be to allow branches to the loop header but this would break in the following pseudocode example:

loop {
  continuing {
    if condition {
      branch_to_loop_header
    }
    
    // some code
  }
}

which would be something like the following spirv:

%11 = OpLabel
      OpLoopMerge %merge %continue None
      OpBranch %14
%14 = OpLabel
      OpBranch %13
%continue = OpLabel
      OpBranchConditional %condition %11 %12
%12 = OpLabel
      // some code

Here the back-edge is %continue and it follows the condition of being post dominate but if we used the quick fix this would generate the following wgsl:

loop {
  continuing {
    if condition { }
    
    // some code
  }
}

while we want to generate

loop {
  continuing {
    if condition { }
    else {
      // some code
    }
  }
}

@eddyb
Copy link
Contributor

eddyb commented Apr 27, 2023

I suspect this is a duplicate of:

I have a fix open for that, can you try it out on your input?

@cwfitzgerald cwfitzgerald added the naga Shader Translator label Oct 25, 2023
@cwfitzgerald cwfitzgerald transferred this issue from gfx-rs/naga Oct 25, 2023
@cwfitzgerald cwfitzgerald added type: bug Something isn't working and removed kind: bug labels Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga front-end lang: SPIR-V Vulkan's Shading Language naga Shader Translator type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants