-
Notifications
You must be signed in to change notification settings - Fork 157
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 unsigned int types #1764
Add unsigned int types #1764
Conversation
@certik Do you think we should move forward with these changes? I wanted your approval before making changes as it will include changes backend as well as some ASR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this is the minimal design that will get us there.
This should allow a relatively clean implementation that is separate from the integer implementation and should be easy to maintain. It does add some code everywhere, but it is maintainable for now, so I think we should start with this.
Thanks @Smit-create!
This now prints 5: def main0():
x: u32
x = (u32(2)+u32(3))*u32(5)
print(x)
main0()
# Not implemented yet in LPython:
#if __name__ == "__main__":
# main() The ASR is missing the arithmetic expression, it just has 5. But otherwise it seems to work. There are still many bugs and corner cases to get right. |
Now it works, just constants not generated. |
Constants work now. Thorough tests must be added. |
The added test does not work yet. |
The test works now. We need to add many more tests, but the basics are working. @czgdp1807 can you please review this? I'll polish the history tomorrow and merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving it but you might have noticed the type generation in LLVM. We need to clean it up and decouple it from ASRToLLVMVisitor. :-).
Yes, it's all over the place, it's a mess. Ideally this is done at just once place, and all decisions shifted to physical types to ASR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good enough now to merge, I polished the commits.
More PRs will be needed with more tests, and fixing corner cases and Debug time wraparound tests.
Thanks, @certik for completing it! |
#1588