From c0ef309d72ef657b6d0632428705980d9df5f9c4 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 19 Dec 2024 16:58:33 +0000 Subject: [PATCH 1/4] feat: flatten nested if-else statements with equivalent conditions --- .../noirc_evaluator/src/ssa/ir/instruction.rs | 51 ++++++++++- .../src/ssa/opt/flatten_cfg.rs | 85 +++++++++++++++++++ 2 files changed, 132 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..aefe34a6e3e 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. + } + }; + + 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. + } + }; + 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..333bd3cd7e7 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -1459,4 +1459,89 @@ mod test { let merged_ssa = Ssa::from_str(src).unwrap(); let _ = merged_ssa.flatten_cfg(); } + + #[test] + fn stores_if_else_numeric() { + 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 stores_if_else() { + 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(); + + print!("{ssa}"); + 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); + } } From efb38c686a215f48710eab85f8dc2606d6c85892 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 19 Dec 2024 17:41:57 +0000 Subject: [PATCH 2/4] chore: rename tests --- compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 333bd3cd7e7..e502cca9dc2 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -1461,7 +1461,7 @@ mod test { } #[test] - fn stores_if_else_numeric() { + fn eliminates_unnecessary_stores_of_numeric_types() { let src = " acir(inline) fn main f0 { b0(v0: bool): @@ -1501,7 +1501,7 @@ mod test { } #[test] - fn stores_if_else() { + fn eliminates_unnecessary_stores_of_array_types() { let src = " acir(inline) fn main f0 { b0(v0: bool, v1: bool): @@ -1529,7 +1529,6 @@ mod test { .fold_constants() .dead_instruction_elimination(); - print!("{ssa}"); let expected = " acir(inline) fn main f0 { b0(v0: u1, v1: u1): From f86df0e26b4e79f0aacabf7bf0e39fcc4039bdb4 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 19 Dec 2024 17:57:58 +0000 Subject: [PATCH 3/4] Update compiler/noirc_evaluator/src/ssa/ir/instruction.rs --- compiler/noirc_evaluator/src/ssa/ir/instruction.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index aefe34a6e3e..5d9bfc89f61 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -994,7 +994,7 @@ impl Instruction { return SimplifiedToInstruction(instruction); } // TODO: We could check to see if `then_condition == inner_else_condition` - // but we run into issues with duplicate NOT instructions. + // but we run into issues with duplicate NOT instructions having distinct ValueIds. } }; @@ -1016,7 +1016,7 @@ impl Instruction { return SimplifiedToInstruction(instruction); } // TODO: We could check to see if `then_condition == inner_else_condition` - // but we run into issues with duplicate NOT instructions. + // but we run into issues with duplicate NOT instructions having distinct ValueIds. } }; From 866d07eacbe24e1d366675592eaf5b1abb836848 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 19 Dec 2024 18:00:39 +0000 Subject: [PATCH 4/4] . --- compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index e502cca9dc2..6e9f6f62423 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -1461,7 +1461,7 @@ mod test { } #[test] - fn eliminates_unnecessary_stores_of_numeric_types() { + fn eliminates_unnecessary_if_else_instructions_on_numeric_types() { let src = " acir(inline) fn main f0 { b0(v0: bool): @@ -1501,7 +1501,7 @@ mod test { } #[test] - fn eliminates_unnecessary_stores_of_array_types() { + fn eliminates_unnecessary_if_else_instructions_on_array_types() { let src = " acir(inline) fn main f0 { b0(v0: bool, v1: bool):