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

ICE: ShiftLeft and ShiftRight are replaced by multiplications and divisions in optimization pass. #1265

Closed
1 task
resurgencelabs opened this issue May 1, 2023 · 6 comments · Fixed by #2072
Assignees
Labels
bug Something isn't working

Comments

@resurgencelabs
Copy link

Aim

The above mentioned error comes up during nargo prove p, but not during nargo check. I am using the bitwise >> and << operators and that is when the error comes up.

Expected behavior

This should not be the case since I am implementing a working algorithm for hashing from another language, where it works.

Bug

The complete message is:

nargo prove p
The application panicked (crashed).
Message: internal error: entered unreachable code: ICE: ShiftLeft and ShiftRight are replaced by multiplications and divisions in optimization pass.
Location: crates/noirc_evaluator/src/ssa/acir_gen/operations/binary.rs:244

To reproduce

  1. Include << or >> in the code.
  2. nargo check //no error
  3. nargo prove p //error comes up as shown above

Installation method

Binary

Nargo version

nargo 0.3.2

@noir-lang/noir_wasm version

No response

@noir-lang/barretenberg version

No response

@noir-lang/aztec_backend version

No response

Additional context

No response

Submission Checklist

  • Once I hit submit, I will assign this issue to the Project Board with the appropriate tags.
@resurgencelabs resurgencelabs added the bug Something isn't working label May 1, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir May 1, 2023
@TomAFrench
Copy link
Member

nargo 0.3.2

Hi @resurgencelabs , this version of nargo is quite old. Can you update to the latest version of nargo (preferably 0.5.0) and see if you still get this issue?

It would also be helpful to share an example program which exhibits this issue.

@resurgencelabs
Copy link
Author

nargo 0.3.2

Hi @resurgencelabs , this version of nargo is quite old. Can you update to the latest version of nargo (preferably 0.5.0) and see if you still get this issue?

It would also be helpful to share an example program which exhibits this issue.

Thanks for the promptness, Tom. We updated to 0.5.0 but it still did not work. Here is the repository that gives this error.

If you were to fork it, and run just nargo check followed by nargo prove p, you should be able to replicate the issue.

@TomAFrench
Copy link
Member

Hey @resurgencelabs, the issue you're running into is that bitshifts by offsets which are known only at runtime are not implemented yet.

The reason for this is that decomposing the field element into bits and applying a shift to them before reconstructing a field from these bits is rather expensive to do inside a snark. This is a bit different from traditional programming languages as processors are well optimised for bitwise operations.

You may want to consider swapping out the hash function you're implementing for a snark friendly hash function such as Pedersen (which we have in the Noir standard library). I can't give any guarantees about exactly when runtime bitshifts will be implemented unfortunately.

@resurgencelabs
Copy link
Author

Hey @resurgencelabs, the issue you're running into is that bitshifts by offsets which are known only at runtime are not implemented yet.

The reason for this is that decomposing the field element into bits and applying a shift to them before reconstructing a field from these bits is rather expensive to do inside a snark. This is a bit different from traditional programming languages as processors are well optimised for bitwise operations.

You may want to consider swapping out the hash function you're implementing for a snark friendly hash function such as Pedersen (which we have in the Noir standard library). I can't give any guarantees about exactly when runtime bitshifts will be implemented unfortunately.

No problem. We will take a look at that as well. But whenever this is implemented, just give us a heads up as well. Thanks :)

@TomAFrench
Copy link
Member

@resurgencelabs correction, I've been told you'll want to use the poseidon hash rather than pedersen.

@kevaundray
Copy link
Contributor

This limitation is also in the new SSA. I will describe the solution which would give the best developer UX.

Shifts are converted to either multiplication by a power of two or division by a power of two. If the compiler had a method to exponentiate a constant by a runtime value, this would then be fixed.

This method would need to:

  • Decompose the exponent x into bits
  • Component the result by using the square-and-multiply algorithm

We would essentially want to generalize this method and copy it into the compiler:

fn pow_32(self, exponent: Field) -> Field {

@guipublic assigning this issue to you

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
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants