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

chore(ssa refactor): array eq ACIR gen #1701

Closed
wants to merge 2 commits into from
Closed

Conversation

joss-aztec
Copy link
Contributor

Description

Problem*

Resolves GH-1625

Summary*

WIP

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

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

Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

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

Is there a reason this was implemented in acir-gen? I think it'd be better if this were implemented in SSA-gen. That way we may not need to implement it separately for brillig.

@joss-aztec
Copy link
Contributor Author

joss-aztec commented Jun 14, 2023

That way we may not need to implement it separately for brillig.

I think the brillig solution is intended to take advantage of control flow for this

@jfecher
Copy link
Contributor

jfecher commented Jun 14, 2023

I think we can leverage the same in SSA by inserting a loop:
For arrays v0 and v1 of length v2:

  v0 = [ ... ]
  v1 = [ ... ]
  v2 = length of v0 and v1
  jmp b1(Field 0)
b1(v3: Field):  // v3 will be the index
  v4 = lt v3, v2
  jmpif v4, then: b2, else: end_true
b2(): // loop body
  v5 = array_get v0, index v3
  v6 = array_get v1, index v3
  v7 = eq v5, v6
  jmpif v7, then: b4, else: end_false
b4():
  v8 = add v3, Field 1
  jmp b1(v8)
end_false():
  jmp end(u1 0)
end_true():
  jmp end(u1 1)
end(v9: u1) // result of the eq in v9
  ...

A bit long but there's a few ways you can encode it.

Another advantage of having it in SSA (versus ACIR) is that it can be optimized out for any array elements that are known.

@joss-aztec
Copy link
Contributor Author

Closed in favour of #1704

@joss-aztec joss-aztec closed this Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Array equality
2 participants