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

Add support of bit shifts on signed integers #1479

Closed
Tracked by #1376
kevaundray opened this issue Jun 1, 2023 · 3 comments · Fixed by #3890
Closed
Tracked by #1376

Add support of bit shifts on signed integers #1479

kevaundray opened this issue Jun 1, 2023 · 3 comments · Fixed by #3890
Assignees
Labels
enhancement New feature or request refactor ssa
Milestone

Comments

@kevaundray
Copy link
Contributor

Problem

The previous code seems to have only done a right shift, if the rhs was a constant. Moreover, it did an unsigned division.

The current code does not handle integer arithmetic, ie overflows etc, so right shifts are doing field divisions instead of integer divisions. This is most likely not the correct route forward.

Happy Case

We should rationalize whether we want shift left and shift right to work only for unsigned integers.

This would mean that both implementation would be doing integer mul and integer division. The previous code was doing Field Mul and integer division.

Alternatives Considered

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@kevaundray
Copy link
Contributor Author

Rationale can be written here and we can put it in the code as docs after

@guipublic
Copy link
Contributor

Left and right shift are implemented for non-const unsigned integers in PR #2072
The only missing piece is to have bit shift for signed integers. However it is not worth to implement this as long as signed integers are not fully supported.

@kevaundray
Copy link
Contributor Author

we have signed integers implemented, so we can close once we have bit shift implemented for them

@Savio-Sou Savio-Sou changed the title Rationalize and explain logic around left and right shifts Add support of bit shifts on signed integers Nov 24, 2023
@Savio-Sou Savio-Sou added this to the 1.0 milestone Nov 24, 2023
github-merge-queue bot pushed a commit that referenced this issue Dec 20, 2023
# Description

## Problem\*

Resolves #1479

## Summary\*

Add support for bit shifts with signed integers.
We use Rust behaviour for overflows.

## Additional Context



## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor ssa
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants