From b901ba66d8871aa31d07dad779e5c641586e6ff2 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 23 Sep 2025 11:33:50 +0200 Subject: [PATCH 1/2] audit review --- .../src/ssa/opt/checked_to_unchecked.rs | 35 +++++++------------ 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/checked_to_unchecked.rs b/compiler/noirc_evaluator/src/ssa/opt/checked_to_unchecked.rs index b31a0bf0d1c..3f7c760a267 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/checked_to_unchecked.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/checked_to_unchecked.rs @@ -47,7 +47,7 @@ impl Function { let dfg = &context.dfg; - match binary.operator { + let unchecked = match binary.operator { BinaryOp::Add { unchecked: false } => { let bit_size = dfg.type_of_value(lhs).bit_size(); let max_lhs_bits = get_max_num_bits(dfg, lhs, &mut value_max_num_bits); @@ -57,19 +57,14 @@ impl Function { // value is at most `2^(n-1) - 1`, assuming `n = bit_size`. Adding those // we get `2^(n-1) - 1 + 2^(n-1) - 1`, so `2*(2^(n-1)) - 2`, // so `2^n - 2` which fits in `0..2^n`. - if max_lhs_bits < bit_size && max_rhs_bits < bit_size { - // `lhs` and `rhs` have both been casted up from smaller types and so cannot overflow. - let operator = BinaryOp::Add { unchecked: true }; - let binary = Binary { operator, ..*binary }; - context.replace_current_instruction_with(Instruction::Binary(binary)); - } + // In that case, `lhs` and `rhs` have both been casted up from smaller types and so cannot overflow. + max_lhs_bits < bit_size && max_rhs_bits < bit_size } BinaryOp::Sub { unchecked: false } => { let Some(lhs_const) = dfg.get_numeric_constant(lhs) else { return; }; - let lhs_bits = lhs_const.num_bits(); let max_rhs_bits = get_max_num_bits(dfg, rhs, &mut value_max_num_bits); let max_rhs = if max_rhs_bits == 128 { u128::MAX } else { (1 << max_rhs_bits) - 1 }; @@ -79,11 +74,7 @@ impl Function { // 2. `lhs` is the maximum value for the maximum bitsize of `rhs`. // For example: `lhs` is 1 and `rhs` max bitsize is 1, so at most it's `1 - 1` which cannot overflow. // Another example: `lhs` is 255 and `rhs` max bitsize is 8, so at most it's `255 - 255` which cannot overflow, etc. - if lhs_bits > max_rhs_bits || (lhs_const == max_rhs.into()) { - let operator = BinaryOp::Sub { unchecked: true }; - let binary = Binary { operator, ..*binary }; - context.replace_current_instruction_with(Instruction::Binary(binary)); - } + lhs_const >= max_rhs.into() } BinaryOp::Mul { unchecked: false } => { let bit_size = dfg.type_of_value(lhs).bit_size(); @@ -95,19 +86,17 @@ impl Function { // less than or equal to the bit size of the result then it cannot overflow. // 3. lhs was upcasted from a boolean // 4. rhs was upcasted from a boolean - if bit_size == 1 - || max_lhs_bits + max_rhs_bits <= bit_size + // So either performing boolean multiplication (which cannot overflow), + // or `lhs` and `rhs` have both been casted up from smaller types and cannot overflow. + max_lhs_bits + max_rhs_bits <= bit_size || max_lhs_bits == 1 || max_rhs_bits == 1 - { - // Either performing boolean multiplication (which cannot overflow), - // or `lhs` and `rhs` have both been casted up from smaller types and so cannot overflow. - let operator = BinaryOp::Mul { unchecked: true }; - let binary = Binary { operator, ..*binary }; - context.replace_current_instruction_with(Instruction::Binary(binary)); - } } - _ => (), + _ => false, + }; + if unchecked { + let operator = binary.operator.into_unchecked(); + context.replace_current_instruction_with(Instruction::Binary(Binary { lhs: binary.lhs, rhs: binary.rhs, operator})); } }); } From 09e27c92cf0141e228dfdec1e4f74d58a00b26e2 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 23 Sep 2025 13:00:42 +0000 Subject: [PATCH 2/2] fmt --- .../noirc_evaluator/src/ssa/opt/checked_to_unchecked.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/checked_to_unchecked.rs b/compiler/noirc_evaluator/src/ssa/opt/checked_to_unchecked.rs index 3f7c760a267..42d0daa2f56 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/checked_to_unchecked.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/checked_to_unchecked.rs @@ -96,7 +96,11 @@ impl Function { }; if unchecked { let operator = binary.operator.into_unchecked(); - context.replace_current_instruction_with(Instruction::Binary(Binary { lhs: binary.lhs, rhs: binary.rhs, operator})); + context.replace_current_instruction_with(Instruction::Binary(Binary { + lhs: binary.lhs, + rhs: binary.rhs, + operator, + })); } }); }