From 2c825d90374ff0766e0215193dbb03ed919020ae Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Thu, 23 Mar 2023 11:15:44 +0200 Subject: [PATCH] [WIP] [spv-in] Convert conditional backedges to `break if`. --- src/block.rs | 10 +++-- src/front/spv/function.rs | 87 +++++++++++++++++++++++++++++++++++---- src/front/spv/mod.rs | 7 +++- 3 files changed, 91 insertions(+), 13 deletions(-) diff --git a/src/block.rs b/src/block.rs index ba4c95ab3a..d7e1bc5a61 100644 --- a/src/block.rs +++ b/src/block.rs @@ -79,20 +79,22 @@ impl Block { .splice(range.clone(), other.span_info.into_iter()); self.body.splice(range, other.body.into_iter()); } - pub fn span_iter(&self) -> impl Iterator { + pub fn span_iter(&self) -> impl DoubleEndedIterator { #[cfg(feature = "span")] let span_iter = self.span_info.iter(); #[cfg(not(feature = "span"))] - let span_iter = std::iter::repeat_with(|| &Span::UNDEFINED); + let span_iter = (0..self.len()).map(|_| &Span::UNDEFINED); self.body.iter().zip(span_iter) } - pub fn span_iter_mut(&mut self) -> impl Iterator)> { + pub fn span_iter_mut( + &mut self, + ) -> impl DoubleEndedIterator)> { #[cfg(feature = "span")] let span_iter = self.span_info.iter_mut().map(Some); #[cfg(not(feature = "span"))] - let span_iter = std::iter::repeat_with(|| None); + let span_iter = (0..self.len()).map(|_| None); self.body.iter_mut().zip(span_iter) } diff --git a/src/front/spv/function.rs b/src/front/spv/function.rs index 5dc781504e..d50ccbfcf5 100644 --- a/src/front/spv/function.rs +++ b/src/front/spv/function.rs @@ -571,6 +571,7 @@ impl<'function> BlockContext<'function> { /// Consumes the `BlockContext` producing a Ir [`Block`](crate::Block) fn lower(mut self) -> crate::Block { fn lower_impl( + expressions: &mut Arena, blocks: &mut crate::FastHashMap, bodies: &[super::Body], body_idx: BodyIndex, @@ -585,8 +586,8 @@ impl<'function> BlockContext<'function> { accept, reject, } => { - let accept = lower_impl(blocks, bodies, accept); - let reject = lower_impl(blocks, bodies, reject); + let accept = lower_impl(expressions, blocks, bodies, accept); + let reject = lower_impl(expressions, blocks, bodies, reject); block.push( crate::Statement::If { @@ -598,14 +599,84 @@ impl<'function> BlockContext<'function> { ) } super::BodyFragment::Loop { body, continuing } => { - let body = lower_impl(blocks, bodies, body); - let continuing = lower_impl(blocks, bodies, continuing); + let body = lower_impl(expressions, blocks, bodies, body); + let mut continuing = lower_impl(expressions, blocks, bodies, continuing); + + // HACK(eddyb) pattern-match on `if _ { break; }` or + // `if _ {} else { break; }` to extract the `break if`. + let mut break_if = None; + let continuing_trailing_statement_with_span = + continuing.span_iter_mut().rev().next(); + if let Some(( + crate::Statement::If { + condition, + accept, + reject, + }, + continuing_trailing_if_span, + )) = continuing_trailing_statement_with_span + { + let accept_breaks = matches!(accept[..], [.., crate::Statement::Break]); + let reject_breaks = matches!(reject[..], [.., crate::Statement::Break]); + + // FIXME(eddyb) change this so be one of: + // * silently accepting (failing validation later) + // * generating into an unconditional `break` + // * erroring nicely + assert!( + !(accept_breaks && reject_breaks), + "unsupported (both sides break): `continuing {{ ... \ + if _ {{ ... break; }} else {{ ... break; }} \ + }}`" + ); + + // Just in case additional expressions are needed. + let mut emitter = Emitter::default(); + emitter.start(expressions); + + if accept_breaks { + accept.cull(accept.len() - 1..); + break_if = Some(*condition); + } + + if reject_breaks { + reject.cull(reject.len() - 1..); + break_if = Some(expressions.append( + crate::Expression::Unary { + op: crate::UnaryOperator::Not, + expr: *condition, + }, + continuing_trailing_if_span.copied().unwrap_or_default(), + )); + } + + let extra_emit_just_before_break_if = emitter.finish(expressions); + + if break_if.is_some() { + // FIXME(eddyb) silently accept, or error nicely. + assert!( + accept.is_empty() && reject.is_empty(), + "unsupported extra statements in `break if`-shaped \ + `continuing {{ ... \ + if _ {{ ...{accept_break} }} \ + else {{ ...{reject_break} }} \ + }}`", + accept_break = if accept_breaks { " break" } else { "" }, + reject_break = if reject_breaks { " break" } else { "" }, + ); + + continuing.cull(continuing.len() - 1..); + continuing.extend(extra_emit_just_before_break_if); + } else { + assert!(extra_emit_just_before_break_if.is_none()); + } + } block.push( crate::Statement::Loop { body, continuing, - break_if: None, + break_if, }, crate::Span::default(), ) @@ -618,7 +689,7 @@ impl<'function> BlockContext<'function> { let mut ir_cases: Vec<_> = cases .iter() .map(|&(value, body_idx)| { - let body = lower_impl(blocks, bodies, body_idx); + let body = lower_impl(expressions, blocks, bodies, body_idx); // Handle simple cases that would make a fallthrough statement unreachable code let fall_through = body.last().map_or(true, |s| !s.is_terminator()); @@ -632,7 +703,7 @@ impl<'function> BlockContext<'function> { .collect(); ir_cases.push(crate::SwitchCase { value: crate::SwitchValue::Default, - body: lower_impl(blocks, bodies, default), + body: lower_impl(expressions, blocks, bodies, default), fall_through: false, }); @@ -656,6 +727,6 @@ impl<'function> BlockContext<'function> { block } - lower_impl(&mut self.blocks, &self.bodies, 0) + lower_impl(&mut self.expressions, &mut self.blocks, &self.bodies, 0) } } diff --git a/src/front/spv/mod.rs b/src/front/spv/mod.rs index c69a230cb0..0a5bcdcb12 100644 --- a/src/front/spv/mod.rs +++ b/src/front/spv/mod.rs @@ -3148,7 +3148,12 @@ impl> Frontend { false => accept, }, ); - debug_assert!(prev.is_none()); + // FIXME(eddyb) somehow this trips when compiling this: + // https://github.com/gfx-rs/naga/issues/1977#issuecomment-1318334723 + // (only in debug mode, as `--release` silences this) + if false { + debug_assert!(prev.is_none()); + } } if same_target {