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

u256 add operator #4896

Merged
merged 10 commits into from
Aug 2, 2023
Merged

u256 add operator #4896

merged 10 commits into from
Aug 2, 2023

Conversation

xunilrj
Copy link
Contributor

@xunilrj xunilrj commented Aug 1, 2023

Description

This PR is part of #4794. It implements support for u256 add operators.

The IR generated is a vanilla add operator as any other integer.

script {
    entry fn main() -> u256, !1 {
        entry():
        v0 = const u256 0x0000000000000000000000000000000000000000000000000000000000000001, !2
        v1 = const u256 0x0000000000000000000000000000000000000000000000000000000000000002, !3
        v2 = call add_0(v0, v1), !4
        ret u256 v2
    }
}

This will then be transformed by miscdemotion pass to something like:

script {
    entry fn main() -> u256 {
         local u256 __wide_lhs
         local mut u256 __wide_result
         local u256 __wide_rhs
 
        entry():       
         v0 = get_local ptr u256, __wide_lhs, !0
         v1 = const u256 0x0000000000000000000000000000000000000000000000000000000000000001, !0
         store v1 to v0, !0
         v2 = get_local ptr u256, __wide_rhs, !0
         v3 = const u256 0x0000000000000000000000000000000000000000000000000000000000000002, !0
         store v3 to v2, !0
         v4 = get_local ptr u256, __wide_result, !0
         wide add v0, v2 to v4, !0
         v5 = load v4, !0
         ret u256 v5
    }
}

Mind the wide add here. It is a Fuel specific operation, giving space to other targets to implement this differently.

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@xunilrj xunilrj mentioned this pull request Aug 1, 2023
19 tasks
@xunilrj xunilrj requested review from anton-trunov and a team August 1, 2023 15:04
IGI-111
IGI-111 previously approved these changes Aug 1, 2023
@IGI-111 IGI-111 requested review from a team and vaivaswatha August 1, 2023 17:05
@vaivaswatha
Copy link
Contributor

A unit test framework already exists to test this, in sway-ir/tests/demote_misc. Any reason you aren't using that?

We're beginning to have testing frameworks that pretty much do the same thing in multiple places. That's going to be confusing to new developers and hard to maintain.

@xunilrj
Copy link
Contributor Author

xunilrj commented Aug 2, 2023

A unit test framework already exists to test this, in sway-ir/tests/demote_misc. Any reason you aren't using that?

Never realised those tests existed. Moved everything there.

@IGI-111 IGI-111 requested a review from a team August 2, 2023 16:40
@xunilrj xunilrj enabled auto-merge (squash) August 2, 2023 20:15
@xunilrj xunilrj merged commit 390b945 into master Aug 2, 2023
@xunilrj xunilrj deleted the xunilrj/u256-add branch August 2, 2023 21:47
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.

4 participants