Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 46 additions & 12 deletions src/react_compiler/optimization/prune_maybe_throws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//! Analogous to TS `Optimization/PruneMaybeThrows.ts`.

use crate::collections::IdMap;
use crate::diagnostics::{CompilerDiagnostic, cold_invariant};
use crate::diagnostics::{CompilerDiagnostic, CompilerError, cold_invariant};
use crate::hir::cfg_utils::{
get_reverse_postordered_blocks, mark_instruction_ids, remove_dead_do_while_statements,
remove_unnecessary_try_catch, remove_unreachable_for_updates,
Expand All @@ -34,6 +34,19 @@ pub(crate) fn prune_maybe_throws(
remove_dead_do_while_statements(&mut func.body);
remove_unnecessary_try_catch(&mut func.body);
mark_instruction_ids(&mut func.body, &mut func.instructions);
// Not in upstream, which finds the phi operand of a removed catch block
// in the rewrite below. That is too late here: the merge asserts on
// such a phi, and the rewrite can move the operand onto another removed
// block, which can make InferReactivePlaces panic.
for block in func.body.blocks.values() {
for phi in &block.phis {
for predecessor in phi.operands.keys() {
if !func.body.blocks.contains_key(predecessor) {
return Err(unmapped_predecessor(*predecessor, block.id).into());
}
}
}
}
merge_consecutive_blocks(func, functions);

// Rewrite phi operands to reference the updated predecessor blocks
Expand All @@ -45,17 +58,10 @@ pub(crate) fn prune_maybe_throws(
let mut updates = Vec::new();
for (predecessor, _) in &phi.operands {
if !preds.contains(predecessor) {
let mapped_terminal =
terminal_mapping.get(*predecessor).copied().ok_or_else(|| {
cold_invariant(
"Expected non-existing phi operand's predecessor to have been mapped to a new terminal",
Some(format!(
"Could not find mapping for predecessor bb{} in block bb{}",
predecessor.0, block.id.0,
)),
None,
)
})?;
let mapped_terminal = terminal_mapping
.get(*predecessor)
.copied()
.ok_or_else(|| unmapped_predecessor(*predecessor, block.id))?;
updates.push((*predecessor, mapped_terminal));
}
}
Expand All @@ -78,8 +84,21 @@ pub(crate) fn prune_maybe_throws(
Ok(())
}

#[cold]
fn unmapped_predecessor(predecessor: BlockId, block: BlockId) -> CompilerError {
cold_invariant(
"Expected non-existing phi operand's predecessor to have been mapped to a new terminal",
Some(format!(
"Could not find mapping for predecessor bb{} in block bb{}",
predecessor.0, block.0,
)),
None,
)
}

fn prune_maybe_throws_impl(func: &mut HirFunction) -> Option<IdMap<BlockId, BlockId>> {
let mut terminal_mapping: IdMap<BlockId, BlockId> = IdMap::new();
let mut pruned_edges: Vec<(BlockId, BlockId)> = Vec::new();
let instructions = &func.instructions;

for block in func.body.blocks.values_mut() {
Expand All @@ -101,11 +120,26 @@ fn prune_maybe_throws_impl(func: &mut HirFunction) -> Option<IdMap<BlockId, Bloc
// BuildReactiveFunction, while nulling out the handler tells us
// that control cannot flow to the handler.
if let Terminal::MaybeThrow { handler, .. } = &mut block.terminal {
if let Some(handler) = *handler {
pruned_edges.push((block.id, handler));
}
*handler = None;
}
}
}

// Not in upstream: a pruned block is no longer a predecessor of its handler,
// so the phis of the handler drop its operand. Upstream keeps the operand,
// and the rewrite in `prune_maybe_throws` raises its invariant for it:
// `terminal_mapping` has no predecessor of the handler to map it to.
for (predecessor, handler) in pruned_edges {
if let Some(handler_block) = func.body.blocks.get_mut(&handler) {
for phi in &mut handler_block.phis {
phi.operands.shift_remove(&predecessor);
}
}
}

if terminal_mapping.is_empty() {
None
} else {
Expand Down
13 changes: 13 additions & 0 deletions src/react_compiler/reactive_scopes/build_reactive_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,19 @@ impl<'a, 'b> Driver<'a, 'b> {
}
GotoVariant::Try => {
// noop
//
// Not in upstream: the check. This is a no-op only
// because the `try` terminal visits the target. If
// a pass removed that terminal and did not merge the
// target into this block, its code would be lost.
if !self.cx.is_scheduled(*goto_block) {
return Err(cold_invariant(
"Expected the fallthrough of a try to be scheduled",
Some(format!("bb{} is not scheduled", goto_block.0)),
None,
)
.into());
}
Comment thread
robobun marked this conversation as resolved.
}
}
}
Expand Down
Loading
Loading