From 46faccefc6e60846143485d5c8320dbb4e7a937c Mon Sep 17 00:00:00 2001 From: joss-aztec <94053499+joss-aztec@users.noreply.github.com> Date: Wed, 21 Jun 2023 20:10:37 +0100 Subject: [PATCH] fix(ssa refactor): allow simplified call inserts & fix const radix arg handling (#1774) * fix(ssa refactor): allow simplified call inserts * fix(ssa refactor): bad radix arg handling * chore(ssa refactor): cp working test * Use Cow to avoid clones * Change _y to a --------- Co-authored-by: Jake Fecher --- .../to_bytes_integration/Nargo.toml | 5 ++ .../to_bytes_integration/Prover.toml | 2 + .../to_bytes_integration/src/main.nr | 25 ++++++++++ .../src/ssa_refactor/ir/dfg.rs | 13 ++--- .../src/ssa_refactor/ir/instruction.rs | 2 +- .../src/ssa_refactor/ssa_builder/mod.rs | 47 ++++++++++++++++++- 6 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/Prover.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/src/main.nr diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/Nargo.toml new file mode 100644 index 00000000000..5082c6e12ec --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/Prover.toml new file mode 100644 index 00000000000..bc4693e434a --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/Prover.toml @@ -0,0 +1,2 @@ +x = "2040124" +a = "0x2000000000000000000000000000000000000000000000000000000000000000" diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/src/main.nr new file mode 100644 index 00000000000..f76df026db7 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/to_bytes_integration/src/main.nr @@ -0,0 +1,25 @@ +use dep::std; + +fn main(x : Field, a: Field) { + let y: Field = 2040124; + let be_byte_array = y.to_be_bytes(31); + let le_byte_array = x.to_le_bytes(31); + + assert(le_byte_array[0] == 60); + assert(le_byte_array[0] == be_byte_array[30]); + assert(le_byte_array[1] == be_byte_array[29]); + assert(le_byte_array[2] == be_byte_array[28]); + + let z = 0 - 1; + let p_bytes = std::field::modulus_le_bytes(); + let z_bytes = z.to_le_bytes(32); + assert(p_bytes[10] == z_bytes[10]); + assert(p_bytes[0] == z_bytes[0] as u8 + 1 as u8); + + let p_bits = std::field::modulus_le_bits(); + let z_bits = z.to_le_bits(std::field::modulus_num_bits() as u32); + assert(z_bits[0] == 0); + assert(p_bits[100] == z_bits[100]); + + a.to_le_bits(std::field::modulus_num_bits() as u32); +} diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 6ba3372c005..aa5f7b53424 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, rc::Rc}; +use std::{borrow::Cow, collections::HashMap, rc::Rc}; use crate::ssa_refactor::ir::instruction::SimplifyResult; @@ -414,14 +414,11 @@ impl<'dfg> InsertInstructionResult<'dfg> { } /// Return all the results contained in the internal results array. - /// This is used for instructions returning multiple results that were - /// not simplified - like function calls. - pub(crate) fn results(&self) -> &'dfg [ValueId] { + /// This is used for instructions returning multiple results like function calls. + pub(crate) fn results(&self) -> Cow<'dfg, [ValueId]> { match self { - InsertInstructionResult::Results(results) => results, - InsertInstructionResult::SimplifiedTo(_) => { - panic!("InsertInstructionResult::results called on a simplified instruction") - } + InsertInstructionResult::Results(results) => Cow::Borrowed(results), + InsertInstructionResult::SimplifiedTo(result) => Cow::Owned(vec![*result]), InsertInstructionResult::InstructionRemoved => { panic!("InsertInstructionResult::results called on a removed instruction") } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index d2699059ef2..2f9a535a019 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs @@ -378,7 +378,7 @@ fn simplify_call(func: ValueId, arguments: &[ValueId], dfg: &mut DataFlowGraph) Intrinsic::ToRadix(endian) => { let field = constant_args[0]; let radix = constant_args[1].to_u128() as u32; - let limb_count = constant_args[1].to_u128() as u32; + let limb_count = constant_args[2].to_u128() as u32; SimplifiedTo(constant_to_radix(endian, field, radix, limb_count, dfg)) } Intrinsic::BlackBox(_) | Intrinsic::Println | Intrinsic::Sort => None, diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs index d68957fa67d..6609252f042 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs @@ -1,4 +1,4 @@ -use std::rc::Rc; +use std::{borrow::Cow, rc::Rc}; use acvm::FieldElement; @@ -236,7 +236,7 @@ impl FunctionBuilder { func: ValueId, arguments: Vec, result_types: Vec, - ) -> &[ValueId] { + ) -> Cow<[ValueId]> { self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types)).results() } @@ -348,3 +348,46 @@ impl std::ops::Index for FunctionBuilder { &self.current_function.dfg[id] } } + +#[cfg(test)] +mod tests { + use std::rc::Rc; + + use acvm::FieldElement; + + use crate::ssa_refactor::ir::{ + function::RuntimeType, + instruction::{Endian, Intrinsic}, + map::Id, + types::Type, + value::Value, + }; + + use super::FunctionBuilder; + + #[test] + fn insert_constant_call() { + // `bits` should be an array of constants [1, 1, 1, 0...]: + // let x = 7; + // let bits = x.to_le_bits(8); + let func_id = Id::test_new(0); + let mut builder = FunctionBuilder::new("func".into(), func_id, RuntimeType::Acir); + let one = builder.numeric_constant(FieldElement::one(), Type::bool()); + let zero = builder.numeric_constant(FieldElement::zero(), Type::bool()); + + let to_bits_id = builder.import_intrinsic_id(Intrinsic::ToBits(Endian::Little)); + let input = builder.numeric_constant(FieldElement::from(7_u128), Type::field()); + let length = builder.numeric_constant(FieldElement::from(8_u128), Type::field()); + let result_types = vec![Type::Array(Rc::new(vec![Type::bool()]), 8)]; + let call_result = builder.insert_call(to_bits_id, vec![input, length], result_types)[0]; + + let array = match &builder.current_function.dfg[call_result] { + Value::Array { array, .. } => array, + _ => panic!(), + }; + assert_eq!(array[0], one); + assert_eq!(array[1], one); + assert_eq!(array[2], one); + assert_eq!(array[3], zero); + } +}