Skip to content

chore: add another test for try_merge_only_changed_indices#8143

Closed
asterite wants to merge 2 commits intomasterfrom
ab/remove_if_else_try_merge_only_changed_indices
Closed

chore: add another test for try_merge_only_changed_indices#8143
asterite wants to merge 2 commits intomasterfrom
ab/remove_if_else_try_merge_only_changed_indices

Conversation

@asterite
Copy link
Collaborator

@asterite asterite commented Apr 21, 2025 •
edited
Loading

Description

Problem

Debugging AztecProtocol/aztec-packages#13677

Summary

Adds an SSA test where try_merge_only_changed_indices currently happens, but didn't happen before #8070 was merged.

The program that produces this SSA right before "Remove IfElse" is this one: 
global MAX_COMMITMENTS_PER_CALL: u32 = 2;
global MAX_COMMITMENT_READ_REQUESTS_PER_CALL: u32 = 2;
global MAX_COMMITMENTS_PER_TX: u32 = 4;
global MAX_COMMITMENT_READ_REQUESTS_PER_TX: u32 = 4;

struct PrivateKernelPublicInputs {
    commitments: [Field; MAX_COMMITMENTS_PER_TX],
    read_requests: [Field; MAX_COMMITMENT_READ_REQUESTS_PER_TX],
}

struct PrivateKernelPublicInputsBuilder {
    commitments: BoundedVec<Field, MAX_COMMITMENTS_PER_TX>,
    read_requests: BoundedVec<Field, MAX_COMMITMENT_READ_REQUESTS_PER_TX>,
}

impl PrivateKernelPublicInputsBuilder {
    fn ingest_app_inputs(&mut self, app_inputs: AppPublicInputs) {
        for i in 0..MAX_COMMITMENTS_PER_CALL {
            if app_inputs.commitments[i] != 0 {
                self.commitments.push(app_inputs.commitments[i]);
            }
        }

        for i in 0..MAX_COMMITMENT_READ_REQUESTS_PER_CALL {
            if app_inputs.read_requests[i] != 0 {
                self.read_requests.push(app_inputs.read_requests[i]);
            }
        }
    }

    fn finish(self) -> PrivateKernelPublicInputs {
        PrivateKernelPublicInputs {
            commitments: self.commitments.storage(),
            read_requests: self.read_requests.storage(),
        }
    }
}

struct AppPublicInputs {
    commitments: [Field; MAX_COMMITMENTS_PER_CALL],
    read_requests: [Field; MAX_COMMITMENT_READ_REQUESTS_PER_CALL],
}

fn main(app_inputs: call_data(1) AppPublicInputs) -> return_data PrivateKernelPublicInputs {
    let mut private_kernel_inputs = PrivateKernelPublicInputsBuilder {
        commitments: BoundedVec::new(),
        read_requests: BoundedVec::new(),
    };
    private_kernel_inputs.ingest_app_inputs(app_inputs);
    private_kernel_inputs.finish()
}

The above program is exactly mock-private-kernel-init.

The SSA before the "Remove IfElse" pass: g0 = u32 2

acir(inline) predicate_pure fn main f0 {
b0(v1: [Field; 2], v2: [Field; 2]):
v4 = array_get v1, index u32 0 -> Field
v6 = array_get v1, index u32 1 -> Field
v7 = array_get v2, index u32 0 -> Field
v8 = array_get v2, index u32 1 -> Field
v9 = make_array [v4, v6, v7, v8] : [Field; 4]
v11 = make_array [Field 0, Field 0, Field 0, Field 0] : [Field; 4]
v12 = make_array [Field 0, Field 0, Field 0, Field 0] : [Field; 4]
v13 = allocate -> &mut [Field; 4]
v14 = allocate -> &mut u32
v15 = allocate -> &mut [Field; 4]
v16 = allocate -> &mut u32
v17 = array_get v1, index u32 0 -> Field
v18 = eq v17, Field 0
v19 = not v18
enable_side_effects v19
v20 = array_get v1, index u32 0 -> Field
v21 = make_array [v20, Field 0, Field 0, Field 0] : [Field; 4]
v22 = if v19 then v21 else (if v18) v11
v23 = cast v19 as u32
v24 = cast v18 as u32
enable_side_effects u1 1
v26 = array_get v1, index u32 1 -> Field
v27 = eq v26, Field 0
v28 = not v27
enable_side_effects v28
v29 = array_get v1, index u32 1 -> Field
v31 = lt v23, u32 4
v32 = mul v31, v28
constrain v32 == v28, "push out of bounds"
v33 = lt v23, u32 4
v34 = mul v33, v28
constrain v34 == v28, "Index out of bounds"
v35 = array_set v22, index v23, value v29
v36 = add v23, u32 1
v37 = if v28 then v35 else (if v27) v22
v38 = cast v28 as u32
v39 = cast v27 as u32
v40 = unchecked_mul v38, v36
v41 = unchecked_mul v39, v23
v42 = unchecked_add v40, v41
enable_side_effects u1 1
v43 = array_get v2, index u32 0 -> Field
v44 = eq v43, Field 0
v45 = not v44
enable_side_effects v45
v46 = array_get v2, index u32 0 -> Field
v47 = make_array [v46, Field 0, Field 0, Field 0] : [Field; 4]
v48 = if v45 then v47 else (if v44) v12
v49 = cast v45 as u32
v50 = cast v44 as u32
enable_side_effects u1 1
v51 = array_get v2, index u32 1 -> Field
v52 = eq v51, Field 0
v53 = not v52
enable_side_effects v53
v54 = array_get v2, index u32 1 -> Field
v55 = lt v49, u32 4
v56 = mul v55, v53
constrain v56 == v53, "push out of bounds"
v57 = lt v49, u32 4
v58 = mul v57, v53
constrain v58 == v53, "Index out of bounds"
v59 = array_set v48, index v49, value v54
v60 = add v49, u32 1
v61 = if v53 then v59 else (if v52) v48
v62 = cast v53 as u32
v63 = cast v52 as u32
v64 = unchecked_mul v62, v60
v65 = unchecked_mul v63, v49
v66 = unchecked_add v64, v65
enable_side_effects u1 1
v67 = array_get v37, index u32 0 -> Field
v68 = array_get v37, index u32 1 -> Field
v69 = array_get v37, index u32 2 -> Field
v71 = array_get v37, index u32 3 -> Field
v72 = array_get v61, index u32 0 -> Field
v73 = array_get v61, index u32 1 -> Field
v74 = array_get v61, index u32 2 -> Field
v75 = array_get v61, index u32 3 -> Field
v76 = make_array [v67, v68, v69, v71, v72, v73, v74, v75] : [Field; 8]
return v76
}

The SSA after "Remove IfElse" in this PR is this: 
g0 = u32 2

acir(inline) predicate_pure fn main f0 {
  b0(v1: [Field; 2], v2: [Field; 2]):
    v4 = array_get v1, index u32 0 -> Field
    v6 = array_get v1, index u32 1 -> Field
    v7 = array_get v2, index u32 0 -> Field
    v8 = array_get v2, index u32 1 -> Field
    v9 = make_array [v4, v6, v7, v8] : [Field; 4]
    v11 = make_array [Field 0, Field 0, Field 0, Field 0] : [Field; 4]
    v12 = make_array [Field 0, Field 0, Field 0, Field 0] : [Field; 4]
    v13 = allocate -> &mut [Field; 4]
    v14 = allocate -> &mut u32
    v15 = allocate -> &mut [Field; 4]
    v16 = allocate -> &mut u32
    v17 = array_get v1, index u32 0 -> Field
    v18 = eq v17, Field 0
    v19 = not v18
    enable_side_effects v19
    v20 = array_get v1, index u32 0 -> Field
    v21 = make_array [v20, Field 0, Field 0, Field 0] : [Field; 4]
    v22 = cast v19 as Field
    v23 = cast v18 as Field
    v24 = mul v22, v20
    v25 = make_array [v24, Field 0, Field 0, Field 0] : [Field; 4]
    v26 = cast v19 as u32
    v27 = cast v18 as u32
    enable_side_effects u1 1
    v29 = array_get v1, index u32 1 -> Field
    v30 = eq v29, Field 0
    v31 = not v30
    enable_side_effects v31
    v32 = array_get v1, index u32 1 -> Field
    v34 = lt v26, u32 4
    v35 = mul v34, v31
    constrain v35 == v31
    v36 = lt v26, u32 4
    v37 = mul v36, v31
    constrain v37 == v31
    v38 = array_set v25, index v26, value v32
    v39 = add v26, u32 1
    enable_side_effects v31
    v40 = array_get v38, index v26 -> Field
    v41 = array_get v25, index v26 -> Field
    v42 = cast v31 as Field
    v43 = cast v30 as Field
    v44 = mul v42, v40
    v45 = mul v43, v41
    v46 = add v44, v45
    v47 = array_set v38, index v26, value v46
    enable_side_effects v31
    v48 = cast v31 as u32
    v49 = cast v30 as u32
    v50 = unchecked_mul v48, v39
    v51 = unchecked_mul v49, v26
    v52 = unchecked_add v50, v51
    enable_side_effects u1 1
    v53 = array_get v2, index u32 0 -> Field
    v54 = eq v53, Field 0
    v55 = not v54
    enable_side_effects v55
    v56 = array_get v2, index u32 0 -> Field
    v57 = make_array [v56, Field 0, Field 0, Field 0] : [Field; 4]
    v58 = cast v55 as Field
    v59 = cast v54 as Field
    v60 = mul v58, v56
    v61 = make_array [v60, Field 0, Field 0, Field 0] : [Field; 4]
    v62 = cast v55 as u32
    v63 = cast v54 as u32
    enable_side_effects u1 1
    v64 = array_get v2, index u32 1 -> Field
    v65 = eq v64, Field 0
    v66 = not v65
    enable_side_effects v66
    v67 = array_get v2, index u32 1 -> Field
    v68 = lt v62, u32 4
    v69 = mul v68, v66
    constrain v69 == v66
    v70 = lt v62, u32 4
    v71 = mul v70, v66
    constrain v71 == v66
    v72 = array_set v61, index v62, value v67
    v73 = add v62, u32 1
    enable_side_effects v66
    v74 = array_get v72, index v62 -> Field
    v75 = array_get v61, index v62 -> Field
    v76 = cast v66 as Field
    v77 = cast v65 as Field
    v78 = mul v76, v74
    v79 = mul v77, v75
    v80 = add v78, v79
    v81 = array_set v72, index v62, value v80
    enable_side_effects v66
    v82 = cast v66 as u32
    v83 = cast v65 as u32
    v84 = unchecked_mul v82, v73
    v85 = unchecked_mul v83, v62
    v86 = unchecked_add v84, v85
    enable_side_effects u1 1
    v87 = array_get v47, index u32 0 -> Field
    v88 = array_get v47, index u32 1 -> Field
    v89 = array_get v47, index u32 2 -> Field
    v91 = array_get v47, index u32 3 -> Field
    v92 = array_get v81, index u32 0 -> Field
    v93 = array_get v81, index u32 1 -> Field
    v94 = array_get v81, index u32 2 -> Field
    v95 = array_get v81, index u32 3 -> Field
    v96 = make_array [v87, v88, v89, v91, v92, v93, v94, v95] : [Field; 8]
    return v96
}
The SSA after "Remove IfElse" right before #8070 was merged is instead this one: 
g0 = u32 2

acir(inline) predicate_pure fn main f0 {
  b0(v1: [Field; 2], v2: [Field; 2]):
    v4 = array_get v1, index u32 0 -> Field
    v6 = array_get v1, index u32 1 -> Field
    v7 = array_get v2, index u32 0 -> Field
    v8 = array_get v2, index u32 1 -> Field
    v9 = make_array [v4, v6, v7, v8] : [Field; 4]
    v11 = make_array [Field 0, Field 0, Field 0, Field 0] : [Field; 4]
    v12 = make_array [Field 0, Field 0, Field 0, Field 0] : [Field; 4]
    v13 = allocate -> &mut [Field; 4]
    v14 = allocate -> &mut u32
    v15 = allocate -> &mut [Field; 4]
    v16 = allocate -> &mut u32
    v17 = array_get v1, index u32 0 -> Field
    v18 = eq v17, Field 0
    v19 = not v18
    enable_side_effects v19
    v20 = array_get v1, index u32 0 -> Field
    v21 = make_array [v20, Field 0, Field 0, Field 0] : [Field; 4]
    v22 = cast v19 as Field
    v23 = cast v18 as Field
    v24 = mul v22, v20
    v25 = make_array [v24, Field 0, Field 0, Field 0] : [Field; 4]
    v26 = cast v19 as u32
    v27 = cast v18 as u32
    enable_side_effects u1 1
    v29 = array_get v1, index u32 1 -> Field
    v30 = eq v29, Field 0
    v31 = not v30
    enable_side_effects v31
    v32 = array_get v1, index u32 1 -> Field
    v34 = lt v26, u32 4
    v35 = mul v34, v31
    constrain v35 == v31
    v36 = lt v26, u32 4
    v37 = mul v36, v31
    constrain v37 == v31
    v38 = array_set v25, index v26, value v32
    v39 = add v26, u32 1
    v40 = array_get v38, index Field 0 -> Field
    v41 = cast v31 as Field
    v42 = cast v30 as Field
    v43 = mul v41, v40
    v44 = mul v42, v24
    v45 = add v43, v44
    v47 = array_get v38, index Field 1 -> Field
    v48 = cast v31 as Field
    v49 = cast v30 as Field
    v50 = mul v48, v47
    v52 = array_get v38, index Field 2 -> Field
    v53 = cast v31 as Field
    v54 = cast v30 as Field
    v55 = mul v53, v52
    v57 = array_get v38, index Field 3 -> Field
    v58 = cast v31 as Field
    v59 = cast v30 as Field
    v60 = mul v58, v57
    v61 = make_array [v45, v50, v55, v60] : [Field; 4]
    v62 = cast v31 as u32
    v63 = cast v30 as u32
    v64 = unchecked_mul v62, v39
    v65 = unchecked_mul v63, v26
    v66 = unchecked_add v64, v65
    enable_side_effects u1 1
    v67 = array_get v2, index u32 0 -> Field
    v68 = eq v67, Field 0
    v69 = not v68
    enable_side_effects v69
    v70 = array_get v2, index u32 0 -> Field
    v71 = make_array [v70, Field 0, Field 0, Field 0] : [Field; 4]
    v72 = cast v69 as Field
    v73 = cast v68 as Field
    v74 = mul v72, v70
    v75 = make_array [v74, Field 0, Field 0, Field 0] : [Field; 4]
    v76 = cast v69 as u32
    v77 = cast v68 as u32
    enable_side_effects u1 1
    v78 = array_get v2, index u32 1 -> Field
    v79 = eq v78, Field 0
    v80 = not v79
    enable_side_effects v80
    v81 = array_get v2, index u32 1 -> Field
    v82 = lt v76, u32 4
    v83 = mul v82, v80
    constrain v83 == v80
    v84 = lt v76, u32 4
    v85 = mul v84, v80
    constrain v85 == v80
    v86 = array_set v75, index v76, value v81
    v87 = add v76, u32 1
    v88 = array_get v86, index Field 0 -> Field
    v89 = cast v80 as Field
    v90 = cast v79 as Field
    v91 = mul v89, v88
    v92 = mul v90, v74
    v93 = add v91, v92
    v94 = array_get v86, index Field 1 -> Field
    v95 = cast v80 as Field
    v96 = cast v79 as Field
    v97 = mul v95, v94
    v98 = array_get v86, index Field 2 -> Field
    v99 = cast v80 as Field
    v100 = cast v79 as Field
    v101 = mul v99, v98
    v102 = array_get v86, index Field 3 -> Field
    v103 = cast v80 as Field
    v104 = cast v79 as Field
    v105 = mul v103, v102
    v106 = make_array [v93, v97, v101, v105] : [Field; 4]
    v107 = cast v80 as u32
    v108 = cast v79 as u32
    v109 = unchecked_mul v107, v87
    v110 = unchecked_mul v108, v76
    v111 = unchecked_add v109, v110
    enable_side_effects u1 1
    v112 = array_get v61, index u32 0 -> Field
    v113 = array_get v61, index u32 1 -> Field
    v114 = array_get v61, index u32 2 -> Field
    v116 = array_get v61, index u32 3 -> Field
    v117 = array_get v106, index u32 0 -> Field
    v118 = array_get v106, index u32 1 -> Field
    v119 = array_get v106, index u32 2 -> Field
    v120 = array_get v106, index u32 3 -> Field
    v121 = make_array [v112, v113, v114, v116, v117, v118, v119, v120] : [Field; 8]
    return v121
}

What I'm trying to understand is whether it's okay for this optimization to kick in here... because when it does, it causes a failure when bumping Noir in Aztec-Packages.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

github-actions[bot]
github-actions bot reviewed Apr 21, 2025
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 431dd16 Previous: 58f17b1 Ratio
test_report_noir-lang_noir_bigcurve_ 283 s 234 s 1.21

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@asterite asterite closed this Apr 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@asterite