Skip to content

Commit

Permalink
Merge branch 'master' into michaeljklein/pedantic-flag
Browse files Browse the repository at this point in the history
  • Loading branch information
michaeljklein authored Jan 6, 2025
2 parents 6186750 + da94c2b commit 55de7d6
Show file tree
Hide file tree
Showing 22 changed files with 238 additions and 154 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/reports.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ jobs:
- name: Compare gates reports
id: gates_diff
uses: noir-lang/noir-gates-diff@60c57a6e3b67bf1553a5774ded82920150f2210e
uses: noir-lang/noir-gates-diff@7e4ddaa91c69380f15ccba514eac17bc7432a8cc
with:
report: gates_report.json
summaryQuantile: 0.9 # only display the 10% most significant circuit size diffs in the summary (defaults to 20%)
Expand Down Expand Up @@ -121,7 +121,7 @@ jobs:
- name: Compare Brillig bytecode size reports
id: brillig_bytecode_diff
uses: noir-lang/noir-gates-diff@60c57a6e3b67bf1553a5774ded82920150f2210e
uses: noir-lang/noir-gates-diff@7e4ddaa91c69380f15ccba514eac17bc7432a8cc
with:
report: gates_report_brillig.json
header: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-js-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ jobs:
- name: Run nargo test
working-directory: ./test-repo/${{ matrix.project.path }}
run: |
out=$(nargo test --silence-warnings --pedantic-solving --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }}) && echo "$out" > ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
nargo test --silence-warnings --pedantic-solving --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }} | tee ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl
env:
NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true

Expand Down
34 changes: 8 additions & 26 deletions Cargo.lock

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

8 changes: 6 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,12 @@ jsonrpsee = { version = "0.24.7", features = ["client-core"] }
flate2 = "1.0.24"
color-eyre = "0.6.2"
rand = "0.8.5"
proptest = "1.2.0"
proptest-derive = "0.4.0"
# The `fork` and `timeout` feature doesn't compile with Wasm (wait-timeout doesn't find the `imp` module).
proptest = { version = "1.6.0", default-features = false, features = [
"std",
"bit-set",
] }
proptest-derive = "0.5.0"
rayon = "1.8.0"
sha2 = { version = "0.10.6", features = ["compress"] }
sha3 = "0.10.6"
Expand Down
26 changes: 20 additions & 6 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ pub(crate) struct FunctionBuilder {
finished_functions: Vec<Function>,
call_stack: CallStackId,
error_types: BTreeMap<ErrorSelector, HirType>,

/// Whether instructions are simplified as soon as they are inserted into this builder.
/// This is true by default unless changed to false after constructing a builder.
pub(crate) simplify: bool,
}

impl FunctionBuilder {
Expand All @@ -56,6 +60,7 @@ impl FunctionBuilder {
finished_functions: Vec::new(),
call_stack: CallStackId::root(),
error_types: BTreeMap::default(),
simplify: true,
}
}

Expand Down Expand Up @@ -172,12 +177,21 @@ impl FunctionBuilder {
ctrl_typevars: Option<Vec<Type>>,
) -> InsertInstructionResult {
let block = self.current_block();
self.current_function.dfg.insert_instruction_and_results(
instruction,
block,
ctrl_typevars,
self.call_stack,
)
if self.simplify {
self.current_function.dfg.insert_instruction_and_results(
instruction,
block,
ctrl_typevars,
self.call_stack,
)
} else {
self.current_function.dfg.insert_instruction_and_results_without_simplification(
instruction,
block,
ctrl_typevars,
self.call_stack,
)
}
}

/// Switch to inserting instructions in the given block.
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ mod tests {
return v2
}
"#;
let ssa = Ssa::from_str(src).unwrap();
let ssa = Ssa::from_str_simplifying(src).unwrap();

let expected = r#"
brillig(inline) fn main f0 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ mod test {
v2 = call multi_scalar_mul (v1, v0) -> [Field; 3]
return v2
}"#;
let ssa = Ssa::from_str(src).unwrap();
let ssa = Ssa::from_str_simplifying(src).unwrap();

let expected_src = r#"
acir(inline) fn main f0 {
Expand Down Expand Up @@ -351,7 +351,7 @@ mod test {
return v4
}"#;
let ssa = Ssa::from_str(src).unwrap();
let ssa = Ssa::from_str_simplifying(src).unwrap();
//First point is zero, second scalar is zero, so we should be left with the scalar mul of the last point.
let expected_src = r#"
acir(inline) fn main f0 {
Expand Down Expand Up @@ -379,7 +379,7 @@ mod test {
v4 = call multi_scalar_mul (v3, v2) -> [Field; 3]
return v4
}"#;
let ssa = Ssa::from_str(src).unwrap();
let ssa = Ssa::from_str_simplifying(src).unwrap();
//First and last scalar/point are constant, so we should be left with the msm of the middle point and the folded constant point
let expected_src = r#"
acir(inline) fn main f0 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ mod test {
return
}
";
let ssa = Ssa::from_str(src).unwrap();
let ssa = Ssa::from_str_simplifying(src).unwrap();

let expected = "
acir(inline) fn main f0 {
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,7 @@ mod test {
v23 = mul v20, Field 6
v24 = mul v21, v16
v25 = add v23, v24
enable_side_effects v0
enable_side_effects v3
v26 = cast v3 as Field
v27 = cast v0 as Field
Expand Down
13 changes: 7 additions & 6 deletions compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,13 @@ impl Context {
.requires_ctrl_typevars()
.then(|| vecmap(old_results, |result| old_function.dfg.type_of_value(*result)));

let new_results = new_function.dfg.insert_instruction_and_results(
instruction,
new_block_id,
ctrl_typevars,
new_call_stack,
);
let new_results =
new_function.dfg.insert_instruction_and_results_without_simplification(
instruction,
new_block_id,
ctrl_typevars,
new_call_stack,
);

assert_eq!(old_results.len(), new_results.len());
for (old_result, new_result) in old_results.iter().zip(new_results.results().iter())
Expand Down
11 changes: 6 additions & 5 deletions compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use super::{
};

impl ParsedSsa {
pub(crate) fn into_ssa(self) -> Result<Ssa, SsaError> {
Translator::translate(self)
pub(crate) fn into_ssa(self, simplify: bool) -> Result<Ssa, SsaError> {
Translator::translate(self, simplify)
}
}

Expand All @@ -41,8 +41,8 @@ struct Translator {
}

impl Translator {
fn translate(mut parsed_ssa: ParsedSsa) -> Result<Ssa, SsaError> {
let mut translator = Self::new(&mut parsed_ssa)?;
fn translate(mut parsed_ssa: ParsedSsa, simplify: bool) -> Result<Ssa, SsaError> {
let mut translator = Self::new(&mut parsed_ssa, simplify)?;

// Note that the `new` call above removed the main function,
// so all we are left with are non-main functions.
Expand All @@ -53,12 +53,13 @@ impl Translator {
Ok(translator.finish())
}

fn new(parsed_ssa: &mut ParsedSsa) -> Result<Self, SsaError> {
fn new(parsed_ssa: &mut ParsedSsa, simplify: bool) -> Result<Self, SsaError> {
// A FunctionBuilder must be created with a main Function, so here wer remove it
// from the parsed SSA to avoid adding it twice later on.
let main_function = parsed_ssa.functions.remove(0);
let main_id = FunctionId::test_new(0);
let mut builder = FunctionBuilder::new(main_function.external_name.clone(), main_id);
builder.simplify = simplify;
builder.set_runtime(main_function.runtime_type);

// Map function names to their IDs so calls can be resolved
Expand Down
18 changes: 13 additions & 5 deletions compiler/noirc_evaluator/src/ssa/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,24 @@ mod token;

impl Ssa {
/// Creates an Ssa object from the given string.
///
/// Note that the resulting Ssa might not be exactly the same as the given string.
/// This is because, internally, the Ssa is built using a `FunctionBuilder`, so
/// some instructions might be simplified while they are inserted.
pub(crate) fn from_str(src: &str) -> Result<Ssa, SsaErrorWithSource> {
Self::from_str_impl(src, false)
}

/// Creates an Ssa object from the given string but trying to simplify
/// each parsed instruction as it's inserted into the final SSA.
pub(crate) fn from_str_simplifying(src: &str) -> Result<Ssa, SsaErrorWithSource> {
Self::from_str_impl(src, true)
}

fn from_str_impl(src: &str, simplify: bool) -> Result<Ssa, SsaErrorWithSource> {
let mut parser =
Parser::new(src).map_err(|err| SsaErrorWithSource::parse_error(err, src))?;
let parsed_ssa =
parser.parse_ssa().map_err(|err| SsaErrorWithSource::parse_error(err, src))?;
parsed_ssa.into_ssa().map_err(|error| SsaErrorWithSource { src: src.to_string(), error })
parsed_ssa
.into_ssa(simplify)
.map_err(|error| SsaErrorWithSource { src: src.to_string(), error })
}
}

Expand Down
12 changes: 12 additions & 0 deletions compiler/noirc_evaluator/src/ssa/parser/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,3 +502,15 @@ fn test_function_type() {
";
assert_ssa_roundtrip(src);
}

#[test]
fn test_does_not_simplify() {
let src = "
acir(inline) fn main f0 {
b0():
v2 = add Field 1, Field 2
return v2
}
";
assert_ssa_roundtrip(src);
}
7 changes: 7 additions & 0 deletions test_programs/execution_success/regression_bignum/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_bignum"
version = "0.1.0"
type = "bin"
authors = [""]

[dependencies]
51 changes: 51 additions & 0 deletions test_programs/execution_success/regression_bignum/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
fn main() {
let numerator =
[790096867046896348, 1063071665130103641, 602707730209562162, 996751591622961462, 28650, 0];
unsafe { __udiv_mod(numerator) };

let denominator = [12, 0, 0, 0, 0, 0];
let result = unsafe { __validate_gt_remainder(denominator) };
assert(result[4] == 0);
assert(result[5] == 0);
}

unconstrained fn __udiv_mod(remainder_u60: [u64; 6]) {
let bit_difference = get_msb(remainder_u60);
let accumulator_u60: [u64; 6] = shl(bit_difference);
}

unconstrained fn __validate_gt_remainder(a_u60: [u64; 6]) -> [u64; 6] {
let mut addend_u60: [u64; 6] = [0; 6];
let mut result_u60: [u64; 6] = [0; 6];

for i in 0..6 {
result_u60[i] = a_u60[i] + addend_u60[i];
}

result_u60
}

unconstrained fn get_msb(val: [u64; 6]) -> u32 {
let mut count = 0;
for i in 0..6 {
let v = val[(6 - 1) - i];
if (v > 0) {
count = 60 * ((6 - 1) - i);
break;
}
}
count
}

unconstrained fn shl(shift: u32) -> [u64; 6] {
let num_shifted_limbs = shift / 60;
let limb_shift = (shift % 60) as u8;

let mut result = [0; 6];
result[num_shifted_limbs] = (1 << limb_shift);

for i in 1..(6 - num_shifted_limbs) {
result[i + num_shifted_limbs] = 0;
}
result
}
Loading

0 comments on commit 55de7d6

Please sign in to comment.