Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: flattening pass no longer overwrites previously mapped condition values #2117

Merged
merged 2 commits into from
Aug 2, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
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