From f7742ab026092f129bd4ec4f122bcd3249100529 Mon Sep 17 00:00:00 2001 From: jfecher Date: Wed, 2 Aug 2023 03:59:08 -0500 Subject: [PATCH] fix: flattening pass no longer overwrites previously mapped condition values (#2117) * Fix flattening pass overwriting previously mapped values * chore: add backticks to variable names in comment --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- .../test_data/regression_2099/Nargo.toml | 6 +++ .../test_data/regression_2099/src/main.nr | 37 +++++++++++++++++++ .../src/ssa_refactor/ir/function_inserter.rs | 1 - .../src/ssa_refactor/opt/flatten_cfg.rs | 5 ++- 4 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data/regression_2099/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data/regression_2099/src/main.nr diff --git a/crates/nargo_cli/tests/test_data/regression_2099/Nargo.toml b/crates/nargo_cli/tests/test_data/regression_2099/Nargo.toml new file mode 100644 index 00000000000..ca96e7164a5 --- /dev/null +++ b/crates/nargo_cli/tests/test_data/regression_2099/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_2099" +authors = [""] +compiler_version = "0.9.0" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data/regression_2099/src/main.nr b/crates/nargo_cli/tests/test_data/regression_2099/src/main.nr new file mode 100644 index 00000000000..b96e664dedf --- /dev/null +++ b/crates/nargo_cli/tests/test_data/regression_2099/src/main.nr @@ -0,0 +1,37 @@ +use dep::std::ec::tecurve::affine::Curve as AffineCurve; +use dep::std::ec::tecurve::affine::Point as Gaffine; +use dep::std::ec::tecurve::curvegroup::Curve; +use dep::std::ec::tecurve::curvegroup::Point as G; + +use dep::std::ec::swcurve::affine::Point as SWGaffine; +use dep::std::ec::swcurve::curvegroup::Point as SWG; + +use dep::std::ec::montcurve::affine::Point as MGaffine; +use dep::std::ec::montcurve::curvegroup::Point as MG; + +fn main() { + // Define Baby Jubjub (ERC-2494) parameters in affine representation + let bjj_affine = AffineCurve::new(168700, 168696, Gaffine::new(995203441582195749578291179787384436505546430278305826713579947235728471134,5472060717959818805561601436314318772137091100104008585924551046643952123905)); + + // Test addition + let p1_affine = Gaffine::new(17777552123799933955779906779655732241715742912184938656739573121738514868268, 2626589144620713026669568689430873010625803728049924121243784502389097019475); + let p2_affine = Gaffine::new(16540640123574156134436876038791482806971768689494387082833631921987005038935, 20819045374670962167435360035096875258406992893633759881276124905556507972311); + let _p3_affine = bjj_affine.add(p1_affine, p2_affine); + + // Test SWCurve equivalents of the above + // First the affine representation + let bjj_swcurve_affine = bjj_affine.into_swcurve(); + + let p1_swcurve_affine = bjj_affine.map_into_swcurve(p1_affine); + let p2_swcurve_affine = bjj_affine.map_into_swcurve(p2_affine); + + let _p3_swcurve_affine_from_add = bjj_swcurve_affine.add( + p1_swcurve_affine, + p2_swcurve_affine + ); + + // Check that these points are on the curve + assert( + bjj_swcurve_affine.contains(p1_swcurve_affine) + ); +} diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs index 38dcfbbb168..15c755f40c2 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/function_inserter.rs @@ -124,7 +124,6 @@ impl<'f> FunctionInserter<'f> { let old_parameters = self.function.dfg.block_parameters(block); for (param, new_param) in old_parameters.iter().zip(new_values) { - // Don't overwrite any existing entries to avoid overwriting the induction variable self.values.entry(*param).or_insert(*new_param); } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs index 4ff857f942f..fdc4be085d7 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs @@ -274,7 +274,10 @@ impl<'f> Context<'f> { // end, in addition to resetting the value of old_condition since it is set to // known to be true/false within the then/else branch respectively. self.insert_current_side_effects_enabled(); - self.inserter.map_value(old_condition, old_condition); + + // We must map back to `then_condition` here. Mapping `old_condition` to itself would + // lose any previous mappings. + self.inserter.map_value(old_condition, then_condition); // While there is a condition on the stack we don't compile outside the condition // until it is popped. This ensures we inline the full then and else branches