From 1a0a5f61231c3b65fa6397ec0769d3e6c5c238a3 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 19 Dec 2024 18:16:01 +0000 Subject: [PATCH] feat: flatten nested if-else statements with equivalent conditions (#6875) --- .../noirc_evaluator/src/ssa/ir/instruction.rs | 51 ++++++++++- .../src/ssa/opt/flatten_cfg.rs | 84 +++++++++++++++++++ 2 files changed, 131 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index fccd3b87d3a..5d9bfc89f61 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -958,9 +958,11 @@ impl Instruction { } } Instruction::IfElse { then_condition, then_value, else_condition, else_value } => { + let then_condition = dfg.resolve(*then_condition); + let else_condition = dfg.resolve(*else_condition); let typ = dfg.type_of_value(*then_value); - if let Some(constant) = dfg.get_numeric_constant(*then_condition) { + if let Some(constant) = dfg.get_numeric_constant(then_condition) { if constant.is_one() { return SimplifiedTo(*then_value); } else if constant.is_zero() { @@ -974,10 +976,51 @@ impl Instruction { return SimplifiedTo(then_value); } - if matches!(&typ, Type::Numeric(_)) { - let then_condition = *then_condition; - let else_condition = *else_condition; + if let Value::Instruction { instruction, .. } = &dfg[then_value] { + if let Instruction::IfElse { + then_condition: inner_then_condition, + then_value: inner_then_value, + else_condition: inner_else_condition, + .. + } = dfg[*instruction] + { + if then_condition == inner_then_condition { + let instruction = Instruction::IfElse { + then_condition, + then_value: inner_then_value, + else_condition: inner_else_condition, + else_value, + }; + return SimplifiedToInstruction(instruction); + } + // TODO: We could check to see if `then_condition == inner_else_condition` + // but we run into issues with duplicate NOT instructions having distinct ValueIds. + } + }; + + if let Value::Instruction { instruction, .. } = &dfg[else_value] { + if let Instruction::IfElse { + then_condition: inner_then_condition, + else_condition: inner_else_condition, + else_value: inner_else_value, + .. + } = dfg[*instruction] + { + if then_condition == inner_then_condition { + let instruction = Instruction::IfElse { + then_condition, + then_value, + else_condition: inner_else_condition, + else_value: inner_else_value, + }; + return SimplifiedToInstruction(instruction); + } + // TODO: We could check to see if `then_condition == inner_else_condition` + // but we run into issues with duplicate NOT instructions having distinct ValueIds. + } + }; + if matches!(&typ, Type::Numeric(_)) { let result = ValueMerger::merge_numeric_values( dfg, block, diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index afc14eed15e..6e9f6f62423 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -1459,4 +1459,88 @@ mod test { let merged_ssa = Ssa::from_str(src).unwrap(); let _ = merged_ssa.flatten_cfg(); } + + #[test] + fn eliminates_unnecessary_if_else_instructions_on_numeric_types() { + let src = " + acir(inline) fn main f0 { + b0(v0: bool): + v1 = allocate -> &mut [Field; 1] + store Field 0 at v1 + jmpif v0 then: b1, else: b2 + b1(): + store Field 1 at v1 + store Field 2 at v1 + jmp b2() + b2(): + v3 = load v1 -> Field + return v3 + }"; + + let ssa = Ssa::from_str(src).unwrap(); + + let ssa = ssa.flatten_cfg().mem2reg().fold_constants(); + + let expected = " + acir(inline) fn main f0 { + b0(v0: u1): + v1 = allocate -> &mut [Field; 1] + enable_side_effects v0 + v2 = not v0 + v3 = cast v0 as Field + v4 = cast v2 as Field + v6 = mul v3, Field 2 + v7 = mul v4, v3 + v8 = add v6, v7 + enable_side_effects u1 1 + return v8 + } + "; + + assert_normalized_ssa_equals(ssa, expected); + } + + #[test] + fn eliminates_unnecessary_if_else_instructions_on_array_types() { + let src = " + acir(inline) fn main f0 { + b0(v0: bool, v1: bool): + v2 = make_array [Field 0] : [Field; 1] + v3 = allocate -> &mut [Field; 1] + store v2 at v3 + jmpif v0 then: b1, else: b2 + b1(): + v4 = make_array [Field 1] : [Field; 1] + store v4 at v3 + v5 = make_array [Field 2] : [Field; 1] + store v5 at v3 + jmp b2() + b2(): + v24 = load v3 -> Field + return v24 + }"; + + let ssa = Ssa::from_str(src).unwrap(); + + let ssa = ssa + .flatten_cfg() + .mem2reg() + .remove_if_else() + .fold_constants() + .dead_instruction_elimination(); + + let expected = " + acir(inline) fn main f0 { + b0(v0: u1, v1: u1): + enable_side_effects v0 + v2 = cast v0 as Field + v4 = mul v2, Field 2 + v5 = make_array [v4] : [Field; 1] + enable_side_effects u1 1 + return v5 + } + "; + + assert_normalized_ssa_equals(ssa, expected); + } }