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

Added implementation for i128 mul. #3625

Merged
merged 1 commit into from
Sep 18, 2023
Merged

Conversation

orizi
Copy link
Collaborator

@orizi orizi commented Jul 6, 2023

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.


This change is Reviewable

@orizi orizi requested a review from spapinistarkware July 6, 2023 14:41
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e1d9fd58 branch from c62248d to 98fdd32 Compare July 6, 2023 16:51
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch 3 times, most recently from 7938e3e to 8342c29 Compare July 6, 2023 18:02
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e1d9fd58 branch from 36ff4ae to 845a654 Compare July 6, 2023 18:02
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch from 8342c29 to d5f14d2 Compare July 6, 2023 18:36
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e1d9fd58 branch from 11f24fb to f32e14b Compare July 9, 2023 04:35
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch 3 times, most recently from 569b388 to 9cab5e9 Compare July 10, 2023 03:27
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch from 9cab5e9 to a7a3bec Compare July 24, 2023 10:19
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e1d9fd58 branch 2 times, most recently from 73bc500 to d18e11a Compare July 24, 2023 10:48
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch 3 times, most recently from c9791f3 to 789e8a2 Compare July 24, 2023 11:39
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e1d9fd58 branch from 242b294 to 4dd7982 Compare July 24, 2023 11:39
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch from 3597c00 to 62dab2e Compare July 25, 2023 15:29
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e1d9fd58 branch 2 times, most recently from 40c8bb2 to cbef09f Compare July 26, 2023 13:32
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch 2 times, most recently from 6439e21 to d55c716 Compare July 26, 2023 13:53
@orizi orizi changed the base branch from pr/orizi/orizi/signed-ints/e1d9fd58 to main August 1, 2023 09:15
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch from d55c716 to 66417d5 Compare August 1, 2023 09:15
@orizi orizi changed the base branch from main to pr/orizi/orizi/signed-ints/c56d4b74 August 1, 2023 09:16
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/c56d4b74 branch from 2473110 to 764ce61 Compare August 2, 2023 17:30
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch from 66417d5 to 1f0229b Compare August 2, 2023 17:30
@orizi orizi changed the base branch from pr/orizi/orizi/signed-ints/c56d4b74 to main August 8, 2023 07:49
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch 2 times, most recently from f4b7e64 to c8ea4a3 Compare August 8, 2023 09:46
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch from c8ea4a3 to 66e6d1a Compare August 24, 2023 13:37
@orizi orizi changed the base branch from main to pr/orizi/orizi/signed-ints/c56d4b74 August 24, 2023 14:10
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch from 66e6d1a to 1646e59 Compare August 24, 2023 14:10
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch from 1646e59 to b552afc Compare September 9, 2023 17:58
Copy link
Contributor

@gilbens-starkware gilbens-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @orizi and @spapinistarkware)


corelib/src/test/integer_test.cairo line 1745 at r1 (raw file):

    assert_eq(@(-3_i128 - -1_i128), @-2_i128, '-3 - -1 == -2');
    assert_eq(@(-231_i128 - -131_i128), @-100_i128, '-231--131=-100');
    assert_eq(@(1_i128 * 3_i128), @3_i128, '1 * 3 == 3');

Consider adding a 1*0 test case.


corelib/src/test/integer_test.cairo line 1748 at r1 (raw file):

    assert_eq(@(2_i128 * 4_i128), @8_i128, '2 * 4 == 8');
    assert_eq(@(-1_i128 * 3_i128), @-3_i128, '-1 * 3 == 3');
    assert_eq(@(-2_i128 * 4_i128), @-8_i128, '-2 * 4 == 8');

Suggestion:

    assert_eq(@(-1_i128 * 3_i128), @-3_i128, '-1 * 3 == -3');
    assert_eq(@(-2_i128 * 4_i128), @-8_i128, '-2 * 4 == -8');

Copy link
Contributor

@gilbens-starkware gilbens-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @orizi and @spapinistarkware)

@orizi orizi changed the base branch from pr/orizi/orizi/signed-ints/c56d4b74 to main September 18, 2023 14:11
@orizi orizi force-pushed the pr/orizi/orizi/signed-ints/e5ea59f5 branch from b552afc to 1581e78 Compare September 18, 2023 14:12
Copy link
Contributor

@gilbens-starkware gilbens-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @orizi and @spapinistarkware)

Copy link
Collaborator Author

@orizi orizi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @spapinistarkware)

@orizi orizi enabled auto-merge September 18, 2023 14:16
@orizi orizi added this pull request to the merge queue Sep 18, 2023
Merged via the queue into main with commit 8f0397b Sep 18, 2023
@orizi orizi deleted the pr/orizi/orizi/signed-ints/e5ea59f5 branch September 27, 2023 12:59
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