Skip to content

Commit

Permalink
fix: flattening pass no longer overwrites previously mapped condition…
Browse files Browse the repository at this point in the history
… 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>
  • Loading branch information
jfecher and TomAFrench authored Aug 2, 2023
1 parent 39610af commit f7742ab
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 2 deletions.
6 changes: 6 additions & 0 deletions crates/nargo_cli/tests/test_data/regression_2099/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "regression_2099"
authors = [""]
compiler_version = "0.9.0"

[dependencies]
37 changes: 37 additions & 0 deletions crates/nargo_cli/tests/test_data/regression_2099/src/main.nr
Original file line number Diff line number Diff line change
@@ -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)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
5 changes: 4 additions & 1 deletion crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f7742ab

Please sign in to comment.