Skip to content

Commit

Permalink
[WIP] [spv-in] Convert conditional backedges to break if.
Browse files Browse the repository at this point in the history
  • Loading branch information
eddyb committed Mar 25, 2023
1 parent 53d62b9 commit 2c825d9
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 13 deletions.
10 changes: 6 additions & 4 deletions src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = (&Statement, &Span)> {
pub fn span_iter(&self) -> impl DoubleEndedIterator<Item = (&Statement, &Span)> {
#[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<Item = (&mut Statement, Option<&mut Span>)> {
pub fn span_iter_mut(
&mut self,
) -> impl DoubleEndedIterator<Item = (&mut Statement, Option<&mut Span>)> {
#[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)
}
Expand Down
87 changes: 79 additions & 8 deletions src/front/spv/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<crate::Expression>,
blocks: &mut crate::FastHashMap<spirv::Word, crate::Block>,
bodies: &[super::Body],
body_idx: BodyIndex,
Expand All @@ -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 {
Expand All @@ -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(),
)
Expand All @@ -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());
Expand All @@ -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,
});

Expand All @@ -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)
}
}
7 changes: 6 additions & 1 deletion src/front/spv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3148,7 +3148,12 @@ impl<I: Iterator<Item = u32>> Frontend<I> {
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 {
Expand Down

0 comments on commit 2c825d9

Please sign in to comment.