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

[FIRRTL][CAPI] Allow constructing integers larger than 64 bits #6893

Merged
merged 1 commit into from
Apr 6, 2024

Conversation

SpriteOvO
Copy link
Member

We are building firrtl.constant IRs from Chisel through MLIR, mlirIntegerAttrGet is used to construct constants, however it only accepts a int64_t value as an argument, so constants larger than 64 bits are not possible to be constructed at the moment.

Upstream report: llvm/llvm-project#84190 (comment)

Refer to @mikeurbach's PR #6787, I propose to likewise add a string hack to workaround this problem. Using this new added C-API firrtlAttrGetIntegerFromString, users (e.g. Chisel) could construct a MlirIntegerAttr from a big int string.

Copy link
Contributor

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

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

I think this is reasonable given the current limitations. Can you please add some tests of positive and negative numbers both larger and smaller than what fits in 64 bits?

@SpriteOvO SpriteOvO force-pushed the firrtl-integer-from-string branch 5 times, most recently from 34a0e00 to 293bc1a Compare April 4, 2024 23:25
Copy link
Contributor

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests. This looks good to me to get something going.

@SpriteOvO SpriteOvO marked this pull request as ready for review April 6, 2024 21:03
@SpriteOvO
Copy link
Member Author

Tested working fine in Chisel binder, thanks to Mike's review.

@SpriteOvO SpriteOvO merged commit 8868394 into llvm:main Apr 6, 2024
4 checks passed
@SpriteOvO SpriteOvO deleted the firrtl-integer-from-string branch April 6, 2024 21:04
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.

2 participants