From 4c594bc3d7ef23378839ef5897811de93fa3e5ea Mon Sep 17 00:00:00 2001 From: Imbris Date: Thu, 2 May 2024 01:28:27 -0400 Subject: [PATCH] [glsl-out] Implement degenerate switch to loop transform for glsl-out --- naga/src/back/continue_forward.rs | 17 +- naga/src/back/glsl/mod.rs | 157 +++++++++++++----- naga/src/back/hlsl/writer.rs | 9 +- naga/tests/in/control-flow.wgsl | 18 ++ .../out/glsl/control-flow.main.Compute.glsl | 40 +++-- .../degenerate-switch.fs_main.Fragment.glsl | 10 +- naga/tests/out/hlsl/control-flow.hlsl | 21 +++ naga/tests/out/msl/control-flow.msl | 16 ++ naga/tests/out/spv/control-flow.spvasm | 139 +++++++++------- naga/tests/out/wgsl/control-flow.wgsl | 14 ++ 10 files changed, 301 insertions(+), 140 deletions(-) diff --git a/naga/src/back/continue_forward.rs b/naga/src/back/continue_forward.rs index 134f069d7ae..a60477b1d5a 100644 --- a/naga/src/back/continue_forward.rs +++ b/naga/src/back/continue_forward.rs @@ -18,9 +18,10 @@ enum Nesting { /// * When entering an inner loop, increment the depth. /// * When exiting the loop, decrement the depth (and pop if it reaches 0). Loop { depth: u32 }, - /// Currently nested in at least one switch. + /// Currently nested in at least one switch that needs to forward continues. /// - /// (including ones transformed into `do {} while(false)` loops). + /// This includes switches transformed into `do {} while(false)` loops, but doesn't need to + /// include regular switches in backends that can support `continue` within switches. /// /// `continue` should be forwarded to surrounding loop. /// @@ -30,7 +31,7 @@ enum Nesting { Switch { depth: u32, variable_id: u32 }, } -pub(crate) enum ExitSwitchOp { +pub(crate) enum ExitControlFlow { None, /// Emit `if (continue_variable) { continue; }` Continue { @@ -124,12 +125,12 @@ impl ContinueCtx { /// Updates internal state and returns whether this switch needs to be followed by a statement /// to forward continues. - pub fn exit_switch(&mut self) -> ExitSwitchOp { + pub fn exit_switch(&mut self) -> ExitControlFlow { match self.stack.last_mut() { - None => ExitSwitchOp::None, + None => ExitControlFlow::None, Some(&mut Nesting::Loop { .. }) => { log::error!("Unexpected loop state when exiting switch"); - ExitSwitchOp::None + ExitControlFlow::None } Some(&mut Nesting::Switch { ref mut depth, @@ -138,9 +139,9 @@ impl ContinueCtx { *depth -= 1; if *depth == 0 { self.stack.pop(); - ExitSwitchOp::Continue { variable_id } + ExitControlFlow::Continue { variable_id } } else { - ExitSwitchOp::Break { variable_id } + ExitControlFlow::Break { variable_id } } } } diff --git a/naga/src/back/glsl/mod.rs b/naga/src/back/glsl/mod.rs index 98f9209e700..7aacb2b5b48 100644 --- a/naga/src/back/glsl/mod.rs +++ b/naga/src/back/glsl/mod.rs @@ -546,6 +546,11 @@ pub struct Writer<'a, W> { named_expressions: crate::NamedExpressions, /// Set of expressions that need to be baked to avoid unnecessary repetition in output need_bake_expressions: back::NeedBakeExpressions, + /// Information about nesting of loops and switches. + /// + /// Used for forwarding continue statements in switches that have been + /// transformed to `do {} while(false);` loops. + continue_ctx: back::continue_forward::ContinueCtx, /// How many views to render to, if doing multiview rendering. multiview: Option, /// Mapping of varying variables to their location. Needed for reflections. @@ -620,6 +625,7 @@ impl<'a, W: Write> Writer<'a, W> { block_id: IdGenerator::default(), named_expressions: Default::default(), need_bake_expressions: Default::default(), + continue_ctx: back::continue_forward::ContinueCtx::default(), varying: Default::default(), }; @@ -2077,61 +2083,110 @@ impl<'a, W: Write> Writer<'a, W> { selector, ref cases, } => { - // TODO: Do this + let l2 = level.next(); // Some glsl consumers may not handle switches with a single body correctly. // So write this as a `do {} while(false);` loop instead. - // This is the same fix as for FXC in hlsl, although here it wasn't specifically - // tested against any problematic consumers. - - // Start the switch - write!(self.out, "{level}")?; - write!(self.out, "switch(")?; - self.write_expr(selector, ctx)?; - writeln!(self.out, ") {{")?; + // This is the same fix as for FXC in hlsl, although here it + // wasn't specifically tested against any problematic consumers. + // See https://github.com/gfx-rs/wgpu/issues/4514 + let one_body = cases + .iter() + .rev() + .skip(1) + .all(|case| case.fall_through && case.body.is_empty()); + if one_body { + // See docs of `back::continue_forward` module. For glsl + // we only need to handle switches transformed into loops. + if let Some(variable_id) = self.continue_ctx.enter_switch() { + writeln!( + self.out, + "{level}bool {}{variable_id} = false;", + back::CONTINUE_PREFIX, + )?; + }; + writeln!(self.out, "{level}do {{")?; + // Note: Expressions have no side-effects so we don't need to emit selector expression. - // Write all cases - let l2 = level.next(); - for case in cases { - match case.value { - crate::SwitchValue::I32(value) => write!(self.out, "{l2}case {value}:")?, - crate::SwitchValue::U32(value) => write!(self.out, "{l2}case {value}u:")?, - crate::SwitchValue::Default => { - if cases.len() == 1 { - let uint = matches!( - ctx.resolve_type(selector, &self.module.types).scalar_kind(), - Some(crate::ScalarKind::Uint) - ); - if uint { - writeln!(self.out, "{l2}case 0u:")?; - } else { - writeln!(self.out, "{l2}case 0:")?; + // Body + if let Some(case) = cases.last() { + for sta in case.body.iter() { + self.write_stmt(sta, ctx, l2)?; + } + } + // End do-while + writeln!(self.out, "{level}}} while(false);")?; + + // Handle any forwarded continue statements. + use back::continue_forward::ExitControlFlow; + let op = match self.continue_ctx.exit_switch() { + ExitControlFlow::None => None, + ExitControlFlow::Continue { variable_id } => { + Some(("continue", variable_id)) + } + ExitControlFlow::Break { variable_id } => Some(("break", variable_id)), + }; + if let Some((control_flow, id)) = op { + writeln!(self.out, "{level}if ({}{id}) {{", back::CONTINUE_PREFIX)?; + writeln!(self.out, "{l2}{control_flow};")?; + writeln!(self.out, "{level}}}")?; + } + } else { + // Start the switch + write!(self.out, "{level}")?; + write!(self.out, "switch(")?; + self.write_expr(selector, ctx)?; + writeln!(self.out, ") {{")?; + + // Write all cases + for case in cases { + match case.value { + crate::SwitchValue::I32(value) => { + write!(self.out, "{l2}case {value}:")? + } + crate::SwitchValue::U32(value) => { + write!(self.out, "{l2}case {value}u:")? + } + crate::SwitchValue::Default => { + if cases.len() == 1 { + let uint = matches!( + ctx.resolve_type(selector, &self.module.types) + .scalar_kind(), + Some(crate::ScalarKind::Uint) + ); + if uint { + writeln!(self.out, "{l2}case 0u:")?; + } else { + writeln!(self.out, "{l2}case 0:")?; + } } + write!(self.out, "{l2}default:")?; } - write!(self.out, "{l2}default:")?; } - } - let write_block_braces = !(case.fall_through && case.body.is_empty()); - if write_block_braces { - writeln!(self.out, " {{")?; - } else { - writeln!(self.out)?; - } + let write_block_braces = !(case.fall_through && case.body.is_empty()); + if write_block_braces { + writeln!(self.out, " {{")?; + } else { + writeln!(self.out)?; + } - for sta in case.body.iter() { - self.write_stmt(sta, ctx, l2.next())?; - } + for sta in case.body.iter() { + self.write_stmt(sta, ctx, l2.next())?; + } - if !case.fall_through && case.body.last().map_or(true, |s| !s.is_terminator()) { - writeln!(self.out, "{}break;", l2.next())?; - } + if !case.fall_through + && case.body.last().map_or(true, |s| !s.is_terminator()) + { + writeln!(self.out, "{}break;", l2.next())?; + } - if write_block_braces { - writeln!(self.out, "{l2}}}")?; + if write_block_braces { + writeln!(self.out, "{l2}}}")?; + } } - } - writeln!(self.out, "{level}}}")? + writeln!(self.out, "{level}}}")? + } } // Loops in naga IR are based on wgsl loops, glsl can emulate the behaviour by using a // while true loop and appending the continuing block to the body resulting on: @@ -2148,6 +2203,7 @@ impl<'a, W: Write> Writer<'a, W> { ref continuing, break_if, } => { + self.continue_ctx.enter_loop(); if !continuing.is_empty() || break_if.is_some() { let gate_name = self.namer.call("loop_init"); writeln!(self.out, "{level}bool {gate_name} = true;")?; @@ -2173,7 +2229,8 @@ impl<'a, W: Write> Writer<'a, W> { for sta in body { self.write_stmt(sta, ctx, level.next())?; } - writeln!(self.out, "{level}}}")? + writeln!(self.out, "{level}}}")?; + self.continue_ctx.exit_loop(); } // Break, continue and return as written as in C // `break;` @@ -2183,8 +2240,16 @@ impl<'a, W: Write> Writer<'a, W> { } // `continue;` Statement::Continue => { - write!(self.out, "{level}")?; - writeln!(self.out, "continue;")? + if let Some(variable_id) = self.continue_ctx.needs_forwarding() { + writeln!( + self.out, + "{level}{}{variable_id} = true;", + back::CONTINUE_PREFIX + )?; + writeln!(self.out, "{level}break;")? + } else { + writeln!(self.out, "{level}continue;")? + } } // `return expr;`, `expr` is optional Statement::Return { value } => { diff --git a/naga/src/back/hlsl/writer.rs b/naga/src/back/hlsl/writer.rs index eddbc4991de..7222dc850d7 100644 --- a/naga/src/back/hlsl/writer.rs +++ b/naga/src/back/hlsl/writer.rs @@ -1510,11 +1510,12 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> { writeln!(self.out, "{level}}}")?; } - use back::continue_forward::ExitSwitchOp; + // Handle any forwarded continue statements. + use back::continue_forward::ExitControlFlow; let op = match self.continue_ctx.exit_switch() { - ExitSwitchOp::None => None, - ExitSwitchOp::Continue { variable_id } => Some(("continue", variable_id)), - ExitSwitchOp::Break { variable_id } => Some(("break", variable_id)), + ExitControlFlow::None => None, + ExitControlFlow::Continue { variable_id } => Some(("continue", variable_id)), + ExitControlFlow::Break { variable_id } => Some(("break", variable_id)), }; if let Some((control_flow, id)) = op { writeln!(self.out, "{level}if ({}{id}) {{", back::CONTINUE_PREFIX)?; diff --git a/naga/tests/in/control-flow.wgsl b/naga/tests/in/control-flow.wgsl index 1076977290f..2e654e7e5fd 100644 --- a/naga/tests/in/control-flow.wgsl +++ b/naga/tests/in/control-flow.wgsl @@ -114,5 +114,23 @@ fn loop_switch_continue_nesting(x: i32, y: i32, z: i32) { } default: {} } + + + // Degenerate switch with continue + switch y { + default: { + continue; + } + } + // Nested degenerate switch with continue + switch y { + case 1, default: { + switch z { + default: { + continue; + } + } + } + } } } diff --git a/naga/tests/out/glsl/control-flow.main.Compute.glsl b/naga/tests/out/glsl/control-flow.main.Compute.glsl index 0f70a433fbd..6da677c0999 100644 --- a/naga/tests/out/glsl/control-flow.main.Compute.glsl +++ b/naga/tests/out/glsl/control-flow.main.Compute.glsl @@ -7,12 +7,9 @@ layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in; void switch_default_break(int i) { - switch(i) { - case 0: - default: { - break; - } - } + do { + break; + } while(false); } void switch_case_break() { @@ -72,6 +69,27 @@ void loop_switch_continue_nesting(int x_1, int y, int z) { break; } } + bool _continue0 = false; + do { + _continue0 = true; + break; + } while(false); + if (_continue0) { + continue; + } + bool _continue1 = false; + do { + do { + _continue1 = true; + break; + } while(false); + if (_continue1) { + break; + } + } while(false); + if (_continue1) { + continue; + } } return; } @@ -83,13 +101,9 @@ void main() { barrier(); memoryBarrierShared(); barrier(); - switch(1) { - case 0: - default: { - pos = 1; - break; - } - } + do { + pos = 1; + } while(false); int _e4 = pos; switch(_e4) { case 1: { diff --git a/naga/tests/out/glsl/degenerate-switch.fs_main.Fragment.glsl b/naga/tests/out/glsl/degenerate-switch.fs_main.Fragment.glsl index 88fd81eb527..ee13e790789 100644 --- a/naga/tests/out/glsl/degenerate-switch.fs_main.Fragment.glsl +++ b/naga/tests/out/glsl/degenerate-switch.fs_main.Fragment.glsl @@ -7,13 +7,9 @@ layout(location = 0) out vec4 _fs2p_location0; void main() { float x = 0.0; - switch(0) { - case 0: - default: { - x = 1.0; - break; - } - } + do { + x = 1.0; + } while(false); float _e4 = x; _fs2p_location0 = vec4(_e4); return; diff --git a/naga/tests/out/hlsl/control-flow.hlsl b/naga/tests/out/hlsl/control-flow.hlsl index bc64d1d8239..61e5556a315 100644 --- a/naga/tests/out/hlsl/control-flow.hlsl +++ b/naga/tests/out/hlsl/control-flow.hlsl @@ -84,6 +84,27 @@ void loop_switch_continue_nesting(int x_1, int y, int z) if (_continue1) { continue; } + bool _continue3 = false; + do { + _continue3 = true; + break; + } while(false); + if (_continue3) { + continue; + } + bool _continue4 = false; + do { + do { + _continue4 = true; + break; + } while(false); + if (_continue4) { + break; + } + } while(false); + if (_continue4) { + continue; + } } return; } diff --git a/naga/tests/out/msl/control-flow.msl b/naga/tests/out/msl/control-flow.msl index d5e53fb75f8..75bf99c7a8d 100644 --- a/naga/tests/out/msl/control-flow.msl +++ b/naga/tests/out/msl/control-flow.msl @@ -79,6 +79,22 @@ void loop_switch_continue_nesting( break; } } + switch(y) { + default: { + continue; + } + } + switch(y) { + case 1: + default: { + switch(z) { + default: { + continue; + } + } + break; + } + } } return; } diff --git a/naga/tests/out/spv/control-flow.spvasm b/naga/tests/out/spv/control-flow.spvasm index 432722a880f..b722d646afc 100644 --- a/naga/tests/out/spv/control-flow.spvasm +++ b/naga/tests/out/spv/control-flow.spvasm @@ -1,13 +1,13 @@ ; SPIR-V ; Version: 1.1 ; Generator: rspirv -; Bound: 94 +; Bound: 100 OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 -OpEntryPoint GLCompute %61 "main" %58 -OpExecutionMode %61 LocalSize 1 1 1 -OpDecorate %58 BuiltIn GlobalInvocationId +OpEntryPoint GLCompute %67 "main" %64 +OpExecutionMode %67 LocalSize 1 1 1 +OpDecorate %64 BuiltIn GlobalInvocationId %2 = OpTypeVoid %4 = OpTypeInt 32 0 %3 = OpTypeVector %4 3 @@ -16,19 +16,19 @@ OpDecorate %58 BuiltIn GlobalInvocationId %15 = OpTypeFunction %2 %16 = OpConstant %5 0 %37 = OpTypeFunction %2 %5 %5 %5 -%59 = OpTypePointer Input %3 -%58 = OpVariable %59 Input -%62 = OpConstant %5 1 -%63 = OpConstant %5 2 -%64 = OpConstant %5 3 -%65 = OpConstant %5 4 -%66 = OpConstant %4 0 -%68 = OpTypePointer Function %5 -%69 = OpConstantNull %5 -%71 = OpConstant %4 2 -%72 = OpConstant %4 1 -%73 = OpConstant %4 72 -%74 = OpConstant %4 264 +%65 = OpTypePointer Input %3 +%64 = OpVariable %65 Input +%68 = OpConstant %5 1 +%69 = OpConstant %5 2 +%70 = OpConstant %5 3 +%71 = OpConstant %5 4 +%72 = OpConstant %4 0 +%74 = OpTypePointer Function %5 +%75 = OpConstantNull %5 +%77 = OpConstant %4 2 +%78 = OpConstant %4 1 +%79 = OpConstant %4 72 +%80 = OpConstant %4 264 %8 = OpFunction %2 None %9 %7 = OpFunctionParameter %5 %6 = OpLabel @@ -121,69 +121,84 @@ OpBranch %43 %46 = OpLabel OpBranch %43 %43 = OpLabel +OpSelectionMerge %57 None +OpSwitch %34 %58 +%58 = OpLabel +OpBranch %42 +%57 = OpLabel +OpSelectionMerge %59 None +OpSwitch %34 %60 1 %60 +%60 = OpLabel +OpSelectionMerge %61 None +OpSwitch %35 %62 +%62 = OpLabel +OpBranch %42 +%61 = OpLabel +OpBranch %59 +%59 = OpLabel OpBranch %42 %42 = OpLabel OpBranch %39 %40 = OpLabel OpReturn OpFunctionEnd -%61 = OpFunction %2 None %15 -%57 = OpLabel -%67 = OpVariable %68 Function %69 -%60 = OpLoad %3 %58 -OpBranch %70 -%70 = OpLabel -OpControlBarrier %71 %72 %73 -OpControlBarrier %71 %71 %74 -OpSelectionMerge %75 None -OpSwitch %62 %76 +%67 = OpFunction %2 None %15 +%63 = OpLabel +%73 = OpVariable %74 Function %75 +%66 = OpLoad %3 %64 +OpBranch %76 %76 = OpLabel -OpStore %67 %62 -OpBranch %75 -%75 = OpLabel -%77 = OpLoad %5 %67 -OpSelectionMerge %78 None -OpSwitch %77 %83 1 %79 2 %80 3 %81 4 %81 5 %82 6 %83 -%79 = OpLabel -OpStore %67 %16 -OpBranch %78 -%80 = OpLabel -OpStore %67 %62 -OpBranch %78 -%81 = OpLabel -OpStore %67 %63 -OpBranch %78 +OpControlBarrier %77 %78 %79 +OpControlBarrier %77 %77 %80 +OpSelectionMerge %81 None +OpSwitch %68 %82 %82 = OpLabel -OpStore %67 %64 -OpBranch %78 -%83 = OpLabel -OpStore %67 %65 -OpBranch %78 -%78 = OpLabel +OpStore %73 %68 +OpBranch %81 +%81 = OpLabel +%83 = OpLoad %5 %73 OpSelectionMerge %84 None -OpSwitch %66 %86 0 %85 +OpSwitch %83 %89 1 %85 2 %86 3 %87 4 %87 5 %88 6 %89 %85 = OpLabel +OpStore %73 %16 OpBranch %84 %86 = OpLabel +OpStore %73 %68 +OpBranch %84 +%87 = OpLabel +OpStore %73 %69 +OpBranch %84 +%88 = OpLabel +OpStore %73 %70 OpBranch %84 -%84 = OpLabel -%87 = OpLoad %5 %67 -OpSelectionMerge %88 None -OpSwitch %87 %93 1 %89 2 %90 3 %91 4 %92 %89 = OpLabel -OpStore %67 %16 -OpBranch %88 +OpStore %73 %71 +OpBranch %84 +%84 = OpLabel +OpSelectionMerge %90 None +OpSwitch %72 %92 0 %91 +%91 = OpLabel +OpBranch %90 +%92 = OpLabel +OpBranch %90 %90 = OpLabel -OpStore %67 %62 +%93 = OpLoad %5 %73 +OpSelectionMerge %94 None +OpSwitch %93 %99 1 %95 2 %96 3 %97 4 %98 +%95 = OpLabel +OpStore %73 %16 +OpBranch %94 +%96 = OpLabel +OpStore %73 %68 OpReturn -%91 = OpLabel -OpStore %67 %63 +%97 = OpLabel +OpStore %73 %69 OpReturn -%92 = OpLabel +%98 = OpLabel OpReturn -%93 = OpLabel -OpStore %67 %64 +%99 = OpLabel +OpStore %73 %70 OpReturn -%88 = OpLabel +%94 = OpLabel OpReturn OpFunctionEnd \ No newline at end of file diff --git a/naga/tests/out/wgsl/control-flow.wgsl b/naga/tests/out/wgsl/control-flow.wgsl index b22df6b0ab1..8d0365c9534 100644 --- a/naga/tests/out/wgsl/control-flow.wgsl +++ b/naga/tests/out/wgsl/control-flow.wgsl @@ -57,6 +57,20 @@ fn loop_switch_continue_nesting(x_1: i32, y: i32, z: i32) { default: { } } + switch y { + default: { + continue; + } + } + switch y { + case 1, default: { + switch z { + default: { + continue; + } + } + } + } } return; }