Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion acvm-repo/acir_field/src/generic_ark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ pub trait AcirField:

fn is_zero(&self) -> bool;
fn is_one(&self) -> bool;

fn pow(&self, exponent: &Self) -> Self;

/// Maximum number of bits needed to represent a field element
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ petgraph.workspace = true

[dev-dependencies]
proptest.workspace = true
proptest-derive.workspace = true
similar-asserts.workspace = true
tracing-test = "0.2.5"
num-traits.workspace = true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Seeds for failure cases proptest has generated in the past. It is
# automatically read and these particular cases re-run before any
# novel cases are generated.
#
# It is recommended to check this file in to source control so that
# everyone who runs the test benefits from these saved cases.
cc ec5f15cf200d34abc8e9b16b520e929f0e6b792c2f523eeadd17e4719d0209ff # shrinks to input = Unsigned { bit_size: 129 }
31 changes: 31 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,4 +465,35 @@ mod tests {
";
assert_normalized_ssa_equals(ssa, expected);
}

#[test]
fn simplifies_or_when_one_side_is_all_1s() {
let test_cases = vec![
("u128", "340282366920938463463374607431768211455"),
("u64", "18446744073709551615"),
("u32", "4294967295"),
("u16", "65535"),
("u8", "255"),
];
const SRC_TEMPLATE: &str = "
acir(inline) pure fn main f0 {
b0(v1: {typ}):
v2 = or {typ} {max}, v1
return v2
}
";

const EXPECTED_TEMPLATE: &str = "
acir(inline) pure fn main f0 {
b0(v1: {typ}):
return {typ} {max}
}
";
for (typ, max) in test_cases {
let src = SRC_TEMPLATE.replace("{typ}", typ).replace("{max}", max);
let expected = EXPECTED_TEMPLATE.replace("{typ}", typ).replace("{max}", max);
let ssa: Ssa = Ssa::from_str_simplifying(&src).unwrap();
assert_normalized_ssa_equals(ssa, &expected);
}
}
}
9 changes: 9 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
}

let operator = if lhs_type == NumericType::NativeField {
// Unchecked operations between fields or bools don't make sense, so we convert those to non-unchecked

Check warning on line 28 in compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/binary.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bools)
// to reduce noise and confusion in the generated SSA.
match operator {
BinaryOp::Add { unchecked: true } => BinaryOp::Add { unchecked: false },
Expand All @@ -34,7 +34,7 @@
_ => operator,
}
} else if lhs_type == NumericType::bool() {
// Unchecked mul between bools doesn't make sense, so we convert that to non-unchecked

Check warning on line 37 in compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/binary.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bools)
if let BinaryOp::Mul { unchecked: true } = operator {
BinaryOp::Mul { unchecked: false }
} else {
Expand Down Expand Up @@ -62,6 +62,10 @@

let lhs_is_one = lhs_value.is_some_and(|lhs| lhs.is_one());
let rhs_is_one = rhs_value.is_some_and(|rhs| rhs.is_one());
let lhs_is_max =
lhs_value.is_some_and(|lhs| lhs_type.max_value().is_ok_and(|max_value| lhs == max_value));
let rhs_is_max =
rhs_value.is_some_and(|rhs| rhs_type.max_value().is_ok_and(|max_value| rhs == max_value));

match binary.operator {
BinaryOp::Add { .. } => {
Expand Down Expand Up @@ -205,7 +209,7 @@
}
if lhs_type == NumericType::bool() {
// Boolean AND is equivalent to multiplication, which is a cheaper operation.
// (mul unchecked because these are bools so it doesn't matter really)

Check warning on line 212 in compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/binary.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (bools)
let instruction = Instruction::binary(BinaryOp::Mul { unchecked: true }, lhs, rhs);
return SimplifyResult::SimplifiedToInstruction(instruction);
}
Expand Down Expand Up @@ -254,6 +258,11 @@
if lhs == rhs {
return SimplifyResult::SimplifiedTo(lhs);
}

if lhs_is_max || rhs_is_max {
let max = dfg.make_constant(lhs_type.max_value().unwrap(), lhs_type);
return SimplifyResult::SimplifiedTo(max);
}
}
BinaryOp::Xor => {
if lhs_is_zero {
Expand Down
31 changes: 28 additions & 3 deletions compiler/noirc_evaluator/src/ssa/ir/types.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use acvm::{FieldElement, acir::AcirField};
use iter_extended::vecmap;
use noirc_frontend::signed_field::SignedField;
use serde::{Deserialize, Serialize};
use std::sync::Arc;

use acvm::{FieldElement, acir::AcirField};
use iter_extended::vecmap;

use crate::ssa::ssa_gen::SSA_WORD_SIZE;

/// A numeric type in the Intermediate representation
Expand All @@ -16,6 +15,7 @@ use crate::ssa::ssa_gen::SSA_WORD_SIZE;
/// Fields do not have a notion of ordering, so this distinction
/// is reasonable.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Ord, PartialOrd, Serialize, Deserialize)]
#[cfg_attr(test, derive(proptest_derive::Arbitrary))]
pub enum NumericType {
Signed { bit_size: u32 },
Unsigned { bit_size: u32 },
Expand Down Expand Up @@ -85,6 +85,20 @@ impl NumericType {
pub(crate) fn is_unsigned(&self) -> bool {
matches!(self, NumericType::Unsigned { .. })
}

pub(crate) fn max_value(&self) -> Result<FieldElement, String> {
match self {
NumericType::Unsigned { bit_size } => match bit_size {
bit_size if *bit_size > 128 => {
Err("Cannot get max value for unsigned type: bit size is greater than 128"
.to_string())
}
Comment thread
TomAFrench marked this conversation as resolved.
128 => Ok(FieldElement::from(u128::MAX)),
_ => Ok(FieldElement::from(2u128.pow(*bit_size) - 1)),
},
other => Err(format!("Cannot get max value for type: {other}")),
}
}
}

/// All types representable in the IR.
Expand Down Expand Up @@ -296,6 +310,7 @@ impl std::fmt::Display for NumericType {
#[cfg(test)]
mod tests {
use super::*;
use proptest::prelude::*;

#[test]
fn test_u8_value_is_outside_limits() {
Expand All @@ -315,4 +330,14 @@ mod tests {
assert!(i8.value_is_outside_limits(SignedField::positive(127_i128)).is_none());
assert!(i8.value_is_outside_limits(SignedField::positive(128_i128)).is_some());
}

proptest! {
#[test]
fn test_max_value_is_in_limits(input: NumericType) {
let max_value = input.max_value();
if let Ok(max_value) = max_value {
prop_assert!(input.value_is_outside_limits(SignedField::from(max_value)).is_none());
}
}
}
}
Loading