From e68319e6486d5940e9fa6e69f1ec23f3785f2d23 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 10 Sep 2025 13:49:01 -0300 Subject: [PATCH 1/3] Make it clearer that this is for ACIR functions --- compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index 35a0c408c59..d1462905558 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -93,7 +93,7 @@ impl Function { /// If this is an ACIR function, go through every instruction, replacing bit shifts with /// more primitive arithmetic operations, fn remove_bit_shifts(&mut self) { - if self.runtime().is_brillig() { + if !self.runtime().is_acir() { return; } From 295df2960f244f57b9c5e2dd8e03db306dd40928 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 10 Sep 2025 13:59:06 -0300 Subject: [PATCH 2/3] Reduce number of lines --- .../src/ssa/opt/remove_bit_shifts.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index d1462905558..d7c26194d50 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -233,11 +233,8 @@ impl Context<'_, '_, '_> { let lhs_as_field = self.insert_cast(lhs, NumericType::NativeField); // For negative numbers, convert to 1-complement using wrapping addition of a + 1 // Unchecked add as these are fields - let one_complement = self.insert_binary( - lhs_sign_as_field, - BinaryOp::Add { unchecked: true }, - lhs_as_field, - ); + let add = BinaryOp::Add { unchecked: true }; + let one_complement = self.insert_binary(lhs_sign_as_field, add, lhs_as_field); let one_complement = self.insert_truncate(one_complement, bit_size, bit_size + 1); let one_complement = self.insert_cast(one_complement, NumericType::signed(bit_size)); @@ -251,11 +248,8 @@ impl Context<'_, '_, '_> { // - ones_complement(lhs) / (2^rhs) == 0 // As the upper bit is set for the ones complement of negative numbers we'd need 2^rhs // to be larger than the lhs bitsize for this to overflow. - let shifted = self.insert_binary( - shifted_complement, - BinaryOp::Sub { unchecked: true }, - lhs_sign_as_int, - ); + let sub = BinaryOp::Sub { unchecked: true }; + let shifted = self.insert_binary(shifted_complement, sub, lhs_sign_as_int); self.insert_truncate(shifted, bit_size, bit_size + 1) } From fda546d23efe4a99b6fc0483b7f21bb44bcd5046 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 10 Sep 2025 14:01:27 -0300 Subject: [PATCH 3/3] Better visibilities --- .../src/ssa/opt/remove_bit_shifts.rs | 39 +++++-------------- 1 file changed, 10 insertions(+), 29 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index d7c26194d50..8ec6d9e267c 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -141,7 +141,7 @@ struct Context<'m, 'dfg, 'mapping> { impl Context<'_, '_, '_> { /// Insert ssa instructions which computes lhs << rhs by doing lhs*2^rhs /// and truncate the result to bit_size - pub(crate) fn insert_wrapping_shift_left(&mut self, lhs: ValueId, rhs: ValueId) -> ValueId { + fn insert_wrapping_shift_left(&mut self, lhs: ValueId, rhs: ValueId) -> ValueId { let typ = self.context.dfg.type_of_value(lhs).unwrap_numeric(); let max_lhs_bits = self.context.dfg.get_value_max_num_bits(lhs); let max_bit_shift_size = self.context.dfg.get_numeric_constant(rhs).map_or_else( @@ -213,7 +213,7 @@ impl Context<'_, '_, '_> { /// Insert ssa instructions which computes lhs >> rhs by doing lhs/2^rhs /// For negative signed integers, we do the division on the 1-complement representation of lhs, /// before converting back the result to the 2-complement representation. - pub(crate) fn insert_shift_right(&mut self, lhs: ValueId, rhs: ValueId) -> ValueId { + fn insert_shift_right(&mut self, lhs: ValueId, rhs: ValueId) -> ValueId { let lhs_typ = self.context.dfg.type_of_value(lhs).unwrap_numeric(); let pow = self.two_pow(rhs); @@ -352,34 +352,25 @@ impl Context<'_, '_, '_> { self.insert_constrain(overflow, one, assert_message.map(Into::into)); } - pub(crate) fn field_constant(&mut self, constant: FieldElement) -> ValueId { + fn field_constant(&mut self, constant: FieldElement) -> ValueId { self.context.dfg.make_constant(constant, NumericType::NativeField) } /// Insert a numeric constant into the current function - pub(crate) fn numeric_constant( - &mut self, - value: impl Into, - typ: NumericType, - ) -> ValueId { + fn numeric_constant(&mut self, value: impl Into, typ: NumericType) -> ValueId { self.context.dfg.make_constant(value.into(), typ) } /// Insert a binary instruction at the end of the current block. /// Returns the result of the binary instruction. - pub(crate) fn insert_binary( - &mut self, - lhs: ValueId, - operator: BinaryOp, - rhs: ValueId, - ) -> ValueId { + fn insert_binary(&mut self, lhs: ValueId, operator: BinaryOp, rhs: ValueId) -> ValueId { let instruction = Instruction::Binary(Binary { lhs, rhs, operator }); self.context.insert_instruction(instruction, None).first() } /// Insert a not instruction at the end of the current block. /// Returns the result of the instruction. - pub(crate) fn insert_not(&mut self, rhs: ValueId) -> ValueId { + fn insert_not(&mut self, rhs: ValueId) -> ValueId { self.context.insert_instruction(Instruction::Not(rhs), None).first() } @@ -395,12 +386,7 @@ impl Context<'_, '_, '_> { /// Insert a truncate instruction at the end of the current block. /// Returns the result of the truncate instruction. - pub(crate) fn insert_truncate( - &mut self, - value: ValueId, - bit_size: u32, - max_bit_size: u32, - ) -> ValueId { + fn insert_truncate(&mut self, value: ValueId, bit_size: u32, max_bit_size: u32) -> ValueId { self.context .insert_instruction(Instruction::Truncate { value, bit_size, max_bit_size }, None) .first() @@ -408,13 +394,13 @@ impl Context<'_, '_, '_> { /// Insert a cast instruction at the end of the current block. /// Returns the result of the cast instruction. - pub(crate) fn insert_cast(&mut self, value: ValueId, typ: NumericType) -> ValueId { + fn insert_cast(&mut self, value: ValueId, typ: NumericType) -> ValueId { self.context.insert_instruction(Instruction::Cast(value, typ), None).first() } /// Insert a call instruction at the end of the current block and return /// the results of the call. - pub(crate) fn insert_call( + fn insert_call( &mut self, func: ValueId, arguments: Vec, @@ -426,12 +412,7 @@ impl Context<'_, '_, '_> { } /// Insert an instruction to extract an element from an array - pub(crate) fn insert_array_get( - &mut self, - array: ValueId, - index: ValueId, - element_type: Type, - ) -> ValueId { + fn insert_array_get(&mut self, array: ValueId, index: ValueId, element_type: Type) -> ValueId { let element_type = Some(vec![element_type]); let offset = ArrayOffset::None; let instruction = Instruction::ArrayGet { array, index, offset };