From d7203fb7db2bbf3c33ae52eb1160cfc7ab6e37da Mon Sep 17 00:00:00 2001 From: kata Date: Wed, 11 Sep 2024 16:07:35 +0800 Subject: [PATCH 1/7] fix: properly propagate constant value to assigned var in mast phase --- examples/generic_for_loop.no | 12 ++++++++++ src/mast/mod.rs | 46 +++++++++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/examples/generic_for_loop.no b/examples/generic_for_loop.no index 4c564fdba..00f0d99b1 100644 --- a/examples/generic_for_loop.no +++ b/examples/generic_for_loop.no @@ -11,6 +11,15 @@ fn join(const LEN: Field, arr1: [Field; LLEN], arr2: [Field; RLEN]) -> [Field; L return arr; } +fn accumulate_mut(const LEN: Field) -> Field { + // shouldn't fold mutable variables as constants + let mut zz = LEN; + for ii in 0..3 { + zz = zz + zz; + } + return zz; +} + fn main(pub xx: Field) { let arr1 = [xx + 1, xx + 2]; let arr2 = [xx + 3, xx + 4]; @@ -21,4 +30,7 @@ fn main(pub xx: Field) { assert_eq(arr[1], arr1[1]); assert_eq(arr[2], arr2[0]); assert_eq(arr[3], arr2[1]); + + let res = accumulate_mut(1); + assert_eq(res, 8); } \ No newline at end of file diff --git a/src/mast/mod.rs b/src/mast/mod.rs index c3accdfbc..19fd31295 100644 --- a/src/mast/mod.rs +++ b/src/mast/mod.rs @@ -563,15 +563,33 @@ fn monomorphize_expr( | Op2::Multiplication | Op2::Division | Op2::BoolAnd - | Op2::BoolOr => lhs_mono.typ, + | Op2::BoolOr => lhs_mono.clone().typ, }; - let cst = match (lhs_mono.constant, rhs_mono.constant) { - (Some(lhs), Some(rhs)) => match op { - Op2::Addition => Some(lhs + rhs), - Op2::Subtraction => Some(lhs - rhs), - Op2::Multiplication => Some(lhs * rhs), - Op2::Division => Some(lhs / rhs), + let ExprMonoInfo { + typ: lhs_typ, + constant: lhs_cst, + .. + } = lhs_mono; + let ExprMonoInfo { + typ: rhs_typ, + constant: rhs_cst, + .. + } = rhs_mono; + + // check if the constant values can be folded + let cst = match (lhs_typ, rhs_typ) { + ( + Some(TyKind::Field { constant: true }), + Some(TyKind::Field { constant: true }), + ) => match (lhs_cst, rhs_cst) { + (Some(lhs), Some(rhs)) => match op { + Op2::Addition => Some(lhs + rhs), + Op2::Subtraction => Some(lhs - rhs), + Op2::Multiplication => Some(lhs * rhs), + Op2::Division => Some(lhs / rhs), + _ => None, + }, _ => None, }, _ => None, @@ -895,7 +913,19 @@ pub fn monomorphize_stmt( StmtKind::Assign { mutable, lhs, rhs } => { let rhs_mono = monomorphize_expr(ctx, rhs, mono_fn_env)?; let typ = rhs_mono.typ.as_ref().expect("expected a type"); - let type_info = MTypeInfo::new(typ, lhs.span, None); + + let type_info = if *mutable { + // shouldn't propagate the constant value to the mutable variable, even the rhs is a constant. + // so that the expression node won't be folded when monomorphizing the binary operations. + // the binary operation will fold the express node if both lhs and rhs hold constant values. + let new_typ = match typ { + TyKind::Field { constant: true } => TyKind::Field { constant: false }, + _ => typ.clone(), + }; + MTypeInfo::new(&new_typ, lhs.span, None) + } else { + MTypeInfo::new(typ, lhs.span, rhs_mono.constant) + }; // store the type of lhs in the env mono_fn_env.store_type(&lhs.value, &type_info)?; From 5786f9f48ecf09c743fbc96ba954fa756c566d77 Mon Sep 17 00:00:00 2001 From: kata Date: Wed, 11 Sep 2024 17:09:00 +0800 Subject: [PATCH 2/7] simplify --- src/mast/mod.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/mast/mod.rs b/src/mast/mod.rs index 19fd31295..d9d5b0319 100644 --- a/src/mast/mod.rs +++ b/src/mast/mod.rs @@ -914,15 +914,12 @@ pub fn monomorphize_stmt( let rhs_mono = monomorphize_expr(ctx, rhs, mono_fn_env)?; let typ = rhs_mono.typ.as_ref().expect("expected a type"); - let type_info = if *mutable { + let type_info = if *mutable && matches!(typ, TyKind::Field { constant: true }) { // shouldn't propagate the constant value to the mutable variable, even the rhs is a constant. // so that the expression node won't be folded when monomorphizing the binary operations. // the binary operation will fold the express node if both lhs and rhs hold constant values. - let new_typ = match typ { - TyKind::Field { constant: true } => TyKind::Field { constant: false }, - _ => typ.clone(), - }; - MTypeInfo::new(&new_typ, lhs.span, None) + let new_typ = &TyKind::Field { constant: false }; + MTypeInfo::new(new_typ, lhs.span, None) } else { MTypeInfo::new(typ, lhs.span, rhs_mono.constant) }; From 06fee99a6f693f97dda7e04f523910e092e4ca46 Mon Sep 17 00:00:00 2001 From: kata Date: Wed, 11 Sep 2024 17:11:27 +0800 Subject: [PATCH 3/7] update comment --- src/mast/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/mast/mod.rs b/src/mast/mod.rs index d9d5b0319..ff24e0b32 100644 --- a/src/mast/mod.rs +++ b/src/mast/mod.rs @@ -915,9 +915,7 @@ pub fn monomorphize_stmt( let typ = rhs_mono.typ.as_ref().expect("expected a type"); let type_info = if *mutable && matches!(typ, TyKind::Field { constant: true }) { - // shouldn't propagate the constant value to the mutable variable, even the rhs is a constant. - // so that the expression node won't be folded when monomorphizing the binary operations. - // the binary operation will fold the express node if both lhs and rhs hold constant values. + // mutable variable shouldn't be constant let new_typ = &TyKind::Field { constant: false }; MTypeInfo::new(new_typ, lhs.span, None) } else { From 3b20903f5fd64c29ce65710fca2bbd83dcb921c7 Mon Sep 17 00:00:00 2001 From: kata Date: Mon, 23 Sep 2024 10:11:04 +0800 Subject: [PATCH 4/7] fix asm fixtures --- examples/fixture/asm/kimchi/generic_nested_func.asm | 1 + examples/fixture/asm/kimchi/generic_nested_method.asm | 1 + examples/fixture/asm/r1cs/generic_nested_func.asm | 1 + examples/fixture/asm/r1cs/generic_nested_method.asm | 1 + 4 files changed, 4 insertions(+) diff --git a/examples/fixture/asm/kimchi/generic_nested_func.asm b/examples/fixture/asm/kimchi/generic_nested_func.asm index ea83d5c55..c2c1cbf62 100644 --- a/examples/fixture/asm/kimchi/generic_nested_func.asm +++ b/examples/fixture/asm/kimchi/generic_nested_func.asm @@ -1,4 +1,5 @@ @ noname.0.7.0 +@ public inputs: 4 DoubleGeneric<1> DoubleGeneric<1> diff --git a/examples/fixture/asm/kimchi/generic_nested_method.asm b/examples/fixture/asm/kimchi/generic_nested_method.asm index ea83d5c55..c2c1cbf62 100644 --- a/examples/fixture/asm/kimchi/generic_nested_method.asm +++ b/examples/fixture/asm/kimchi/generic_nested_method.asm @@ -1,4 +1,5 @@ @ noname.0.7.0 +@ public inputs: 4 DoubleGeneric<1> DoubleGeneric<1> diff --git a/examples/fixture/asm/r1cs/generic_nested_func.asm b/examples/fixture/asm/r1cs/generic_nested_func.asm index 4d0e4621a..b7ff82ca4 100644 --- a/examples/fixture/asm/r1cs/generic_nested_func.asm +++ b/examples/fixture/asm/r1cs/generic_nested_func.asm @@ -1,4 +1,5 @@ @ noname.0.7.0 +@ public inputs: 4 v_4 == (v_1) * (1) v_4 == (v_2) * (1) diff --git a/examples/fixture/asm/r1cs/generic_nested_method.asm b/examples/fixture/asm/r1cs/generic_nested_method.asm index 4d0e4621a..b7ff82ca4 100644 --- a/examples/fixture/asm/r1cs/generic_nested_method.asm +++ b/examples/fixture/asm/r1cs/generic_nested_method.asm @@ -1,4 +1,5 @@ @ noname.0.7.0 +@ public inputs: 4 v_4 == (v_1) * (1) v_4 == (v_2) * (1) From e13df0c878090db07e2a91b095658a2c2a84cac1 Mon Sep 17 00:00:00 2001 From: kata Date: Mon, 23 Sep 2024 10:32:31 +0800 Subject: [PATCH 5/7] update test to cover propagating cst value for assign stmt --- examples/generic_for_loop.no | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/examples/generic_for_loop.no b/examples/generic_for_loop.no index 00f0d99b1..8b6a910aa 100644 --- a/examples/generic_for_loop.no +++ b/examples/generic_for_loop.no @@ -11,9 +11,9 @@ fn join(const LEN: Field, arr1: [Field; LLEN], arr2: [Field; RLEN]) -> [Field; L return arr; } -fn accumulate_mut(const LEN: Field) -> Field { +fn accumulate_mut(const INIT: Field) -> Field { // shouldn't fold mutable variables as constants - let mut zz = LEN; + let mut zz = INIT; for ii in 0..3 { zz = zz + zz; } @@ -31,6 +31,7 @@ fn main(pub xx: Field) { assert_eq(arr[2], arr2[0]); assert_eq(arr[3], arr2[1]); - let res = accumulate_mut(1); + let init_val = 1; + let res = accumulate_mut(init_val); assert_eq(res, 8); } \ No newline at end of file From f9e1c70dd93ad5dae3835f0961f766ef9af104f1 Mon Sep 17 00:00:00 2001 From: kata Date: Mon, 23 Sep 2024 10:32:44 +0800 Subject: [PATCH 6/7] remove unnecessary changes --- src/mast/mod.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/mast/mod.rs b/src/mast/mod.rs index 9889db84c..60039c45d 100644 --- a/src/mast/mod.rs +++ b/src/mast/mod.rs @@ -556,7 +556,7 @@ fn monomorphize_expr( | Op2::Multiplication | Op2::Division | Op2::BoolAnd - | Op2::BoolOr => lhs_mono.clone().typ, + | Op2::BoolOr => lhs_mono.typ, }; let ExprMonoInfo { expr: lhs_expr, .. } = lhs_mono; @@ -892,14 +892,7 @@ pub fn monomorphize_stmt( StmtKind::Assign { mutable, lhs, rhs } => { let rhs_mono = monomorphize_expr(ctx, rhs, mono_fn_env)?; let typ = rhs_mono.typ.as_ref().expect("expected a type"); - - let type_info = if *mutable && matches!(typ, TyKind::Field { constant: true }) { - // mutable variable shouldn't be constant - let new_typ = &TyKind::Field { constant: false }; - MTypeInfo::new(new_typ, lhs.span, None) - } else { - MTypeInfo::new(typ, lhs.span, rhs_mono.constant) - }; + let type_info = MTypeInfo::new(typ, lhs.span, rhs_mono.constant); // store the type of lhs in the env mono_fn_env.store_type(&lhs.value, &type_info)?; From 5620a7f6b0d06b86643ea59bbc794bf58f4758b7 Mon Sep 17 00:00:00 2001 From: kata Date: Mon, 23 Sep 2024 10:39:58 +0800 Subject: [PATCH 7/7] update comment --- examples/generic_for_loop.no | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/generic_for_loop.no b/examples/generic_for_loop.no index 8b6a910aa..c0caddc27 100644 --- a/examples/generic_for_loop.no +++ b/examples/generic_for_loop.no @@ -12,7 +12,8 @@ fn join(const LEN: Field, arr1: [Field; LLEN], arr2: [Field; RLEN]) -> [Field; L } fn accumulate_mut(const INIT: Field) -> Field { - // shouldn't fold mutable variables as constants + // it shouldn't fold these variables, even they hold constant values + // it should only fold the generic vars let mut zz = INIT; for ii in 0..3 { zz = zz + zz;