Skip to content
This repository has been archived by the owner on Jan 29, 2025. It is now read-only.

[spv-in] Naga rejects example with break from switch #1028

Closed
afd opened this issue Jun 25, 2021 · 5 comments
Closed

[spv-in] Naga rejects example with break from switch #1028

afd opened this issue Jun 25, 2021 · 5 comments
Labels
area: front-end Input formats for conversion kind: bug Something isn't working lang: SPIR-V Binary SPIR-V input and output

Comments

@afd
Copy link

afd commented Jun 25, 2021

Naga (1f42d4f) rejects this SPIR-V:

               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint Fragment %4 "main"
               OpExecutionMode %4 OriginUpperLeft
          %2 = OpTypeVoid
          %3 = OpTypeFunction %2
         %72 = OpTypeInt 32 0
         %73 = OpConstant %72 0
        %114 = OpTypeBool
       %2317 = OpUndef %114
          %4 = OpFunction %2 None %3
       %1538 = OpLabel
               OpSelectionMerge %1729 None
               OpSwitch %73 %1608
       %1608 = OpLabel
               OpSelectionMerge %1621 None
               OpBranchConditional %2317 %1619 %1621
       %1619 = OpLabel
               OpBranch %1729
       %1621 = OpLabel
               OpBranch %1729
       %1729 = OpLabel
               OpReturn
               OpFunctionEnd

I get:

$ RUST_BACKTRACE=1 cargo run ./reductiontemp/reduced.spv temp.wgsl
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/naga ./reductiontemp/reduced.spv temp.wgsl`
Function [1] '' is invalid: 
	The `break` is used outside of a `loop` or `switch` context
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', cli/src/main.rs:317:70
stack backtrace:
   0: rust_begin_unwind
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099/library/std/src/panicking.rs:515:5
   1: core::panicking::panic_fmt
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099/library/core/src/panicking.rs:92:14
   2: core::panicking::panic
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099/library/core/src/panicking.rs:50:5
   3: core::option::Option<T>::unwrap
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099/library/core/src/option.rs:388:21
   4: naga::main
             at ./cli/src/main.rs:317:56
   5: core::ops::function::FnOnce::call_once
             at /rustc/24bdc6d73a75dce9a7013ebc7c037013ff4ea099/library/core/src/ops/function.rs:227:5

I believe the SPIR-V is valid, and spirv-val accepts it. The "break" edge, from 1619 to 1729, is inside a switch construct - namely the construct headed at 1538. It's also inside a selection construct - the construct at 1608 - but that's allowed.

Here is the CFG:

image

Tint accepts this and gives this WGSL:

[[stage(fragment)]]
fn main() {
  switch(0u) {
    default: {
      if (false) {
        break;
      }
    }
  }
  return;
}
@kvark kvark added area: front-end Input formats for conversion kind: bug Something isn't working lang: SPIR-V Binary SPIR-V input and output labels Jun 26, 2021
@kvark
Copy link
Member

kvark commented Jun 26, 2021

Thank you for filing! cc @MatusT

@jimblandy
Copy link
Member

#940 is probably relevant to this.

@jimblandy
Copy link
Member

With #1041, Naga accepts this and produces the WGSL:

fn function() {
    switch(i32(0u)) {
        default: {
            if (false) {
                break;
            } else {
                break;
            }
        }
    }
    return;
}

[[stage(fragment)]]
fn main() {
    function();
}

@jimblandy
Copy link
Member

We've abandoned #1041 as insufficient. The underlying bug here is the rearrangement carried out by this code in the SPIR-V front end:

// uplift "default" into the parent
let extracted = mem::take(default);
statements.splice(i + 1..i + 1, extracted);

This is not a semantics-preserving transformation, when the statements include a Break.

#1294 might remove the broken transformation.

@JCapucho
Copy link
Collaborator

Fixed by #1294

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: front-end Input formats for conversion kind: bug Something isn't working lang: SPIR-V Binary SPIR-V input and output
Projects
None yet
Development

No branches or pull requests

4 participants