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

Compiler panic during SSA unrolling #6734

Closed
nventuro opened this issue Dec 7, 2024 · 3 comments · Fixed by #6736
Closed

Compiler panic during SSA unrolling #6734

nventuro opened this issue Dec 7, 2024 · 3 comments · Fixed by #6736
Labels
bug Something isn't working

Comments

@nventuro
Copy link
Contributor

nventuro commented Dec 7, 2024

The following noir program crashes:

pub fn subarray_extended<let SRC_LEN: u32, let DST_LEN: u32>(
    src: [Field; SRC_LEN],
    offset: u32,
) -> [Field; DST_LEN] {
    let available_elements_to_copy = SRC_LEN - offset;
    let elements_to_copy = if DST_LEN > available_elements_to_copy {
        available_elements_to_copy
    } else {
        DST_LEN
    };

    let mut dst: [Field; DST_LEN] = std::mem::zeroed();
    for i in 0..elements_to_copy {
        dst[i] = src[i + offset];
    }

    dst
}

#[test]
unconstrained fn subarray_extended_into_empty() {
    assert_eq(subarray_extended([], 0), []);
}

nargo test yields:

[example] Running 1 test function
The application panicked (crashed).
Message:  assertion `left == right` failed: The header should just compare the induction variable and jump
  left: 0
 right: 1
Location: compiler/noirc_evaluator/src/ssa/opt/unrolling.rs:305

This seems to be related to the for loop iterating over elements_to_copy. An empty loop body also causes the crash, and elements_to_copy can be simplified a bit and still cause a crash, for example:

let elements_to_copy = if DST_LEN > available_elements_to_copy {
    1
} else {
    DST_LEN
};
@nventuro nventuro added the bug Something isn't working label Dec 7, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Dec 7, 2024
@jfecher
Copy link
Contributor

jfecher commented Dec 9, 2024

@vezenovm I've seen cases where loop invariant code motion can hoist instructions into a loop's header block instead of its pre-header. Perhaps this is an instance of that where it causes an issue.

@TomAFrench
Copy link
Member

I think this program just manages to have enough indirection so that we get halfway through optimizing out the loop before we start trying to unroll.

After Mem2Reg (1st):
brillig(inline) fn main f0 {
  b0():
    v3 = make_array [] : [Field; 0]
    jmp b1()
  b1():
    jmp b2(u32 0)
  b2(v0: u32):
    v5 = make_array [] : [Field; 0]
    v6 = allocate -> &mut [Field; 0]
    store v5 at v6
    jmp b3(u32 0)
  b3(v1: u32):
    v7 = lt v1, v0
    jmpif v7 then: b7, else: b4
  b4():
    v8 = load v6 -> [Field; 0]
    v9 = make_array [] : [Field; 0]
    v10 = allocate -> &mut u1
    jmp b5(u32 0)
  b5(v2: u32):
    jmp b6()
  b6():
    return
  b7():
    v11 = load v6 -> [Field; 0]
    v12 = array_get v3, index v1 -> Field
    v13 = array_set v11, index v1, value v12
    v15 = add v1, u32 1
    store v13 at v6
    v16 = add v1, u32 1
    jmp b3(v16)
}

After Simplifying (1st):
brillig(inline) fn main f0 {
  b0():
    v1 = make_array [] : [Field; 0]
    v2 = make_array [] : [Field; 0]
    v3 = allocate -> &mut [Field; 0]
    store v2 at v3
    jmp b1(u32 0)
  b1(v0: u32):
    jmpif u1 0 then: b3, else: b2 // uh oh
  b2():
    v6 = load v3 -> [Field; 0]
    v7 = make_array [] : [Field; 0]
    v8 = allocate -> &mut u1
    return
  b3():
    v9 = load v3 -> [Field; 0]
    v10 = array_get v1, index v0 -> Field
    v11 = array_set v9, index v0, value v10
    v13 = add v0, u32 1
    store v11 at v3
    v14 = add v0, u32 1
    jmp b1(v14)
}

@jfecher
Copy link
Contributor

jfecher commented Dec 9, 2024

Maybe we just need a simplify_cfg pass before unrolling then. Either that or check for the constant case during unrolling.
Edit: I see @guipublic has already fixed this

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants