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

Parse error for large constants #1989

Open
Mark1626 opened this issue Mar 28, 2024 · 2 comments
Open

Parse error for large constants #1989

Mark1626 opened this issue Mar 28, 2024 · 2 comments

Comments

@Mark1626
Copy link

I'm evaluating a Calyx binding I created for posits. I was trying to test the accuracy of 128 bit posits when I noticed that Calyx can't parse literals larger than the 64 bit limit

Error

=====STDERR=====
Error: Failed to parse buffer:   --> 20:28
   |
20 |     const0 = std_const(128,82412135738664784120036037737381363712);
   |                            ^------------------------------------^
   |
   = Expected valid bitwidth

=====STDOUT=====

The large number 82412135738664784120036037737381363712 is the 128 bit uint representation of 0.5 (as 128 bit posit)

Source

fn bitwidth(input: Node) -> ParseResult<u64> {
input
.as_str()
.parse::<u64>()
.map_err(|_| input.error("Expected valid bitwidth"))
}

@EclecticGriffin
Copy link
Collaborator

@rachitnigam @sampsyo I mentioned that this would be a problem in #1969. It seems like solving this would require using some sort of bigint library which would probably cause a cascade of code changes since the types would have to be updated everywhere. Probably using something like ibig or dashu would be the most straightforward approach, but I'm curious if people have thoughts. Pulling from this benchmark analysis of some bigint libraries (https://github.com/tczajka/bigint-benchmark-rs).

@andrewb1999
Copy link
Collaborator

Just want to add that these changes will also benefit the allo -> amc -> calyx flow significantly. The previous workaround works for now, but having true >64 bit int support will allow for a lot of interesting test cases. Certainly not an easy change though so I understand the hesitation to change this now...

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

No branches or pull requests

3 participants