From e83af22a2ece15a99c3af18f06ea19f1fd049890 Mon Sep 17 00:00:00 2001 From: Chris Fallin Date: Tue, 15 Apr 2025 09:38:36 -0700 Subject: [PATCH] Checker: process defs/clobbers on branches to allow checking of try-calls. The regalloc checker previously handled branches specially: it skipped the normal operand-procesing logic because it handles block-call arguments explicitly with parallel moves. However, this is no longer fully correct in the presence of try-calls: these are branches but also have normal uses, defs, and clobbers, so we need to process those normally. The comment noted also that we needed to skip operand processing on branches because edge-moves may come before branches, but I'm not sure that exemption ever made sense: if moves interfere with operands to the branch (e.g. conditional branch inputs) we should catch that too. This simplifies the checker's instruction handling to no longer skip branches' operands, with no other changes (and no changes to the allocator itself). Addresses bytecodealliance/wasmtime#10585 (and will fix once we release with this and pull into Cranelift). --- src/checker.rs | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/checker.rs b/src/checker.rs index 0f180e26..91943a5e 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -758,25 +758,22 @@ impl<'a, F: Function> Checker<'a, F> { /// For each original instruction, create an `Op`. fn handle_inst(&mut self, block: Block, inst: Inst, out: &Output) { - // Skip normal checks if this is a branch: the blockparams do - // not exist in post-regalloc code, and the edge-moves have to - // be inserted before the branch rather than after. - if !self.f.is_branch(inst) { - let operands: Vec<_> = self.f.inst_operands(inst).iter().cloned().collect(); - let allocs: Vec<_> = out.inst_allocs(inst).iter().cloned().collect(); - let clobbers: Vec<_> = self.f.inst_clobbers(inst).into_iter().collect(); - let checkinst = CheckerInst::Op { - inst, - operands, - allocs, - clobbers, - }; - trace!("checker: adding inst {:?}", checkinst); - self.bb_insts.get_mut(&block).unwrap().push(checkinst); - } - // Instead, if this is a branch, emit a ParallelMove on each - // outgoing edge as necessary to handle blockparams. - else { + // Process uses, defs, and clobbers. + let operands: Vec<_> = self.f.inst_operands(inst).iter().cloned().collect(); + let allocs: Vec<_> = out.inst_allocs(inst).iter().cloned().collect(); + let clobbers: Vec<_> = self.f.inst_clobbers(inst).into_iter().collect(); + let checkinst = CheckerInst::Op { + inst, + operands, + allocs, + clobbers, + }; + trace!("checker: adding inst {:?}", checkinst); + self.bb_insts.get_mut(&block).unwrap().push(checkinst); + + // If this is a branch, emit a ParallelMove on each outgoing + // edge as necessary to handle blockparams. + if self.f.is_branch(inst) { for (i, &succ) in self.f.block_succs(block).iter().enumerate() { let args = self.f.branch_blockparams(block, inst, i); let params = self.f.block_params(succ);