From eec5970658157704dac5c41e6d61b2aa652ce996 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 29 Nov 2024 10:04:58 -0300 Subject: [PATCH 1/2] fix: correct types returned by constant EC operations simplified within SSA (#6652) Co-authored-by: TomAFrench Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- .../src/ssa/ir/instruction/call.rs | 34 ++++++++++++------- .../src/ssa/ir/instruction/call/blackbox.rs | 4 +-- .../Nargo.toml | 7 ++++ .../src/main.nr | 15 ++++++++ 4 files changed, 46 insertions(+), 14 deletions(-) create mode 100644 test_programs/execution_success/inline_decompose_hint_brillig_call/Nargo.toml create mode 100644 test_programs/execution_success/inline_decompose_hint_brillig_call/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 4be37b3c626..67222d06ea8 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -45,17 +45,17 @@ pub(super) fn simplify_call( _ => return SimplifyResult::None, }; + let return_type = ctrl_typevars.and_then(|return_types| return_types.first().cloned()); + let constant_args: Option> = arguments.iter().map(|value_id| dfg.get_numeric_constant(*value_id)).collect(); - match intrinsic { + let simplified_result = match intrinsic { Intrinsic::ToBits(endian) => { // TODO: simplify to a range constraint if `limb_count == 1` - if let (Some(constant_args), Some(return_type)) = - (constant_args, ctrl_typevars.map(|return_types| return_types.first().cloned())) - { + if let (Some(constant_args), Some(return_type)) = (constant_args, return_type.clone()) { let field = constant_args[0]; - let limb_count = if let Some(Type::Array(_, array_len)) = return_type { + let limb_count = if let Type::Array(_, array_len) = return_type { array_len as u32 } else { unreachable!("ICE: Intrinsic::ToRadix return type must be array") @@ -67,12 +67,10 @@ pub(super) fn simplify_call( } Intrinsic::ToRadix(endian) => { // TODO: simplify to a range constraint if `limb_count == 1` - if let (Some(constant_args), Some(return_type)) = - (constant_args, ctrl_typevars.map(|return_types| return_types.first().cloned())) - { + if let (Some(constant_args), Some(return_type)) = (constant_args, return_type.clone()) { let field = constant_args[0]; let radix = constant_args[1].to_u128() as u32; - let limb_count = if let Some(Type::Array(_, array_len)) = return_type { + let limb_count = if let Type::Array(_, array_len) = return_type { array_len as u32 } else { unreachable!("ICE: Intrinsic::ToRadix return type must be array") @@ -330,7 +328,7 @@ pub(super) fn simplify_call( } Intrinsic::FromField => { let incoming_type = Type::field(); - let target_type = ctrl_typevars.unwrap().remove(0); + let target_type = return_type.clone().unwrap(); let truncate = Instruction::Truncate { value: arguments[0], @@ -352,8 +350,8 @@ pub(super) fn simplify_call( Intrinsic::AsWitness => SimplifyResult::None, Intrinsic::IsUnconstrained => SimplifyResult::None, Intrinsic::DerivePedersenGenerators => { - if let Some(Type::Array(_, len)) = ctrl_typevars.unwrap().first() { - simplify_derive_generators(dfg, arguments, *len as u32, block, call_stack) + if let Some(Type::Array(_, len)) = return_type.clone() { + simplify_derive_generators(dfg, arguments, len as u32, block, call_stack) } else { unreachable!("Derive Pedersen Generators must return an array"); } @@ -370,7 +368,19 @@ pub(super) fn simplify_call( } Intrinsic::ArrayRefCount => SimplifyResult::None, Intrinsic::SliceRefCount => SimplifyResult::None, + }; + + if let (Some(expected_types), SimplifyResult::SimplifiedTo(result)) = + (return_type, &simplified_result) + { + assert_eq!( + dfg.type_of_value(*result), + expected_types, + "Simplification should not alter return type" + ); } + + simplified_result } /// Slices have a tuple structure (slice length, slice contents) to enable logic diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs index 4f2a31e2fb0..301b75e0bd4 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs @@ -48,7 +48,7 @@ pub(super) fn simplify_ec_add( let result_x = dfg.make_constant(result_x, Type::field()); let result_y = dfg.make_constant(result_y, Type::field()); - let result_is_infinity = dfg.make_constant(result_is_infinity, Type::bool()); + let result_is_infinity = dfg.make_constant(result_is_infinity, Type::field()); let typ = Type::Array(Arc::new(vec![Type::field()]), 3); @@ -107,7 +107,7 @@ pub(super) fn simplify_msm( let result_x = dfg.make_constant(result_x, Type::field()); let result_y = dfg.make_constant(result_y, Type::field()); - let result_is_infinity = dfg.make_constant(result_is_infinity, Type::bool()); + let result_is_infinity = dfg.make_constant(result_is_infinity, Type::field()); let elements = im::vector![result_x, result_y, result_is_infinity]; let typ = Type::Array(Arc::new(vec![Type::field()]), 3); diff --git a/test_programs/execution_success/inline_decompose_hint_brillig_call/Nargo.toml b/test_programs/execution_success/inline_decompose_hint_brillig_call/Nargo.toml new file mode 100644 index 00000000000..ecac2dfb197 --- /dev/null +++ b/test_programs/execution_success/inline_decompose_hint_brillig_call/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "inline_decompose_hint_brillig_call" +version = "0.1.0" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/inline_decompose_hint_brillig_call/src/main.nr b/test_programs/execution_success/inline_decompose_hint_brillig_call/src/main.nr new file mode 100644 index 00000000000..e500f0f976d --- /dev/null +++ b/test_programs/execution_success/inline_decompose_hint_brillig_call/src/main.nr @@ -0,0 +1,15 @@ +use std::embedded_curve_ops::{EmbeddedCurvePoint, EmbeddedCurveScalar, fixed_base_scalar_mul}; + +fn main() -> pub Field { + let pre_address = 0x23d95e303879a5d0bbef78ecbc335e559da37431f6dcd11da54ed375c2846813; + let (a, b) = std::field::bn254::decompose(pre_address); + let curve = EmbeddedCurveScalar { lo: a, hi: b }; + let key = fixed_base_scalar_mul(curve); + let point = EmbeddedCurvePoint { + x: 0x111223493147f6785514b1c195bb37a2589f22a6596d30bb2bb145fdc9ca8f1e, + y: 0x273bbffd678edce8fe30e0deafc4f66d58357c06fd4a820285294b9746c3be95, + is_infinite: false, + }; + let address_point = key.add(point); + address_point.x +} From c49d7209cf6b22fcfe2996cceaf76fe99075091b Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Fri, 29 Nov 2024 14:33:43 +0000 Subject: [PATCH 2/2] chore: refactor poseidon2 (#6655) --- noir_stdlib/src/hash/poseidon2.nr | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/noir_stdlib/src/hash/poseidon2.nr b/noir_stdlib/src/hash/poseidon2.nr index f2167c43c2c..419f07a2aca 100644 --- a/noir_stdlib/src/hash/poseidon2.nr +++ b/noir_stdlib/src/hash/poseidon2.nr @@ -13,11 +13,7 @@ pub struct Poseidon2 { impl Poseidon2 { #[no_predicates] pub fn hash(input: [Field; N], message_size: u32) -> Field { - if message_size == N { - Poseidon2::hash_internal(input, N, false) - } else { - Poseidon2::hash_internal(input, message_size, true) - } + Poseidon2::hash_internal(input, message_size, message_size != N) } pub(crate) fn new(iv: Field) -> Poseidon2 {