diff --git a/cranelift/frontend/src/frontend.rs b/cranelift/frontend/src/frontend.rs index dbef386f89..21320d8cb3 100644 --- a/cranelift/frontend/src/frontend.rs +++ b/cranelift/frontend/src/frontend.rs @@ -680,14 +680,14 @@ impl<'a> FunctionBuilder<'a> { /// /// **Note:** You are responsible for maintaining the coherence with the arguments of /// other jump instructions. - pub fn change_jump_destination(&mut self, inst: Inst, new_dest: Block) { + pub fn change_jump_destination(&mut self, inst: Inst, old_block: Block, new_block: Block) { let dfg = &mut self.func.dfg; - for old_dest in dfg.insts[inst].branch_destination_mut() { - self.func_ctx - .ssa - .remove_block_predecessor(old_dest.block(&dfg.value_lists), inst); - old_dest.set_block(new_dest, &mut dfg.value_lists); - self.func_ctx.ssa.declare_block_predecessor(new_dest, inst); + for block in dfg.insts[inst].branch_destination_mut() { + if block.block(&dfg.value_lists) == old_block { + self.func_ctx.ssa.remove_block_predecessor(old_block, inst); + block.set_block(new_block, &mut dfg.value_lists); + self.func_ctx.ssa.declare_block_predecessor(new_block, inst); + } } } diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs index 81f9ee886d..22032a0f29 100644 --- a/cranelift/wasm/src/code_translator.rs +++ b/cranelift/wasm/src/code_translator.rs @@ -292,6 +292,7 @@ pub fn translate_operator( Operator::If { blockty } => { let val = state.pop1(); + let next_block = builder.create_block(); let (params, results) = blocktype_params_results(validator, *blockty)?; let (destination, else_data) = if params.clone().eq(results.clone()) { // It is possible there is no `else` block, so we will only @@ -301,21 +302,38 @@ pub fn translate_operator( // up discovering an `else`, then we will allocate a block for it // and go back and patch the jump. let destination = block_with_params(builder, results.clone(), environ)?; - let branch_inst = - canonicalise_then_brz(builder, val, destination, state.peekn(params.len())); - (destination, ElseData::NoElse { branch_inst }) + let branch_inst = canonicalise_brif( + builder, + val, + next_block, + &[], + destination, + state.peekn(params.len()), + ); + ( + destination, + ElseData::NoElse { + branch_inst, + placeholder: destination, + }, + ) } else { // The `if` type signature is not valid without an `else` block, // so we eagerly allocate the `else` block here. let destination = block_with_params(builder, results.clone(), environ)?; let else_block = block_with_params(builder, params.clone(), environ)?; - canonicalise_then_brz(builder, val, else_block, state.peekn(params.len())); + canonicalise_brif( + builder, + val, + next_block, + &[], + else_block, + state.peekn(params.len()), + ); builder.seal_block(else_block); (destination, ElseData::WithElse { else_block }) }; - let next_block = builder.create_block(); - canonicalise_then_jump(builder, next_block, &[]); builder.seal_block(next_block); // Only predecessor is the current block. builder.switch_to_block(next_block); @@ -357,7 +375,10 @@ pub fn translate_operator( // Ensure we have a block for the `else` block (it may have // already been pre-allocated, see `ElseData` for details). let else_block = match *else_data { - ElseData::NoElse { branch_inst } => { + ElseData::NoElse { + branch_inst, + placeholder, + } => { let (params, _results) = blocktype_params_results(validator, blocktype)?; debug_assert_eq!(params.len(), num_return_values); @@ -370,7 +391,11 @@ pub fn translate_operator( ); state.popn(params.len()); - builder.change_jump_destination(branch_inst, else_block); + builder.change_jump_destination( + branch_inst, + placeholder, + else_block, + ); builder.seal_block(else_block); else_block } @@ -2171,6 +2196,7 @@ fn translate_unreachable_operator( ir::Block::reserved_value(), ElseData::NoElse { branch_inst: ir::Inst::reserved_value(), + placeholder: ir::Block::reserved_value(), }, 0, 0, @@ -2198,7 +2224,10 @@ fn translate_unreachable_operator( state.reachable = true; let else_block = match *else_data { - ElseData::NoElse { branch_inst } => { + ElseData::NoElse { + branch_inst, + placeholder, + } => { let (params, _results) = blocktype_params_results(validator, blocktype)?; let else_block = block_with_params(builder, params, environ)?; @@ -2206,7 +2235,11 @@ fn translate_unreachable_operator( frame.truncate_value_stack_to_else_params(&mut state.stack); // We change the target of the branch instruction. - builder.change_jump_destination(branch_inst, else_block); + builder.change_jump_destination( + branch_inst, + placeholder, + else_block, + ); builder.seal_block(else_block); else_block } @@ -2816,10 +2849,9 @@ fn translate_br_if( ) { let val = state.pop1(); let (br_destination, inputs) = translate_br_if_args(relative_depth, state); - canonicalise_then_brnz(builder, val, br_destination, inputs); - let next_block = builder.create_block(); - canonicalise_then_jump(builder, next_block, &[]); + canonicalise_brif(builder, val, br_destination, inputs, next_block, &[]); + builder.seal_block(next_block); // The only predecessor is the current block. builder.switch_to_block(next_block); } @@ -3120,28 +3152,28 @@ fn canonicalise_then_jump( builder.ins().jump(destination, canonicalised) } -/// The same but for a `brz` instruction. -fn canonicalise_then_brz( +/// The same but for a `brif` instruction. +fn canonicalise_brif( builder: &mut FunctionBuilder, cond: ir::Value, - destination: ir::Block, - params: &[Value], + block_then: ir::Block, + params_then: &[ir::Value], + block_else: ir::Block, + params_else: &[ir::Value], ) -> ir::Inst { - let mut tmp_canonicalised = SmallVec::<[ir::Value; 16]>::new(); - let canonicalised = canonicalise_v128_values(&mut tmp_canonicalised, builder, params); - builder.ins().brz(cond, destination, canonicalised) -} - -/// The same but for a `brnz` instruction. -fn canonicalise_then_brnz( - builder: &mut FunctionBuilder, - cond: ir::Value, - destination: ir::Block, - params: &[Value], -) -> ir::Inst { - let mut tmp_canonicalised = SmallVec::<[ir::Value; 16]>::new(); - let canonicalised = canonicalise_v128_values(&mut tmp_canonicalised, builder, params); - builder.ins().brnz(cond, destination, canonicalised) + let mut tmp_canonicalised_then = SmallVec::<[ir::Value; 16]>::new(); + let canonicalised_then = + canonicalise_v128_values(&mut tmp_canonicalised_then, builder, params_then); + let mut tmp_canonicalised_else = SmallVec::<[ir::Value; 16]>::new(); + let canonicalised_else = + canonicalise_v128_values(&mut tmp_canonicalised_else, builder, params_else); + builder.ins().brif( + cond, + block_then, + canonicalised_then, + block_else, + canonicalised_else, + ) } /// A helper for popping and bitcasting a single value; since SIMD values can lose their type by diff --git a/cranelift/wasm/src/state.rs b/cranelift/wasm/src/state.rs index 0517dd2fc4..3d775e05ec 100644 --- a/cranelift/wasm/src/state.rs +++ b/cranelift/wasm/src/state.rs @@ -22,6 +22,9 @@ pub enum ElseData { /// instruction that needs to be fixed up to point to the new `else` /// block rather than the destination block after the `if...end`. branch_inst: Inst, + + /// The placeholder block we're replacing. + placeholder: Block, }, /// We have already allocated an `else` block. @@ -43,9 +46,8 @@ pub enum ElseData { /// - `num_return_values`: number of values returned by the control block; /// - `original_stack_size`: size of the value stack at the beginning of the control block. /// -/// Moreover, the `if` frame has the `branch_inst` field that points to the `brz` instruction -/// separating the `true` and `false` branch. The `loop` frame has a `header` field that references -/// the `Block` that contains the beginning of the body of the loop. +/// The `loop` frame has a `header` field that references the `Block` that contains the beginning +/// of the body of the loop. #[derive(Debug)] pub enum ControlStackFrame { If {