`Int` - two's complement #695

erik-3milabs · 2024-10-16T09:44:31Z

Topic
Adding Int support (see #624)

To decide on sign-and-magnitude (#694) vs. two's complement, I whipped up a quick two's complement implementation of the basic add/sub/mul/div operations. It's not perfect yet; I'm just curious to see how it compares to #694.

erik-3milabs · 2024-10-16T11:02:18Z

FYI: local benchmark results @ b23483d

wrapping ops/split_mul, 
I128xI128:      26.042 ns
I256xI256:      26.104 ns
I512xI512:      68.643 ns
I1024xI1024:   239.23  ns
I2048xI2048:   799.26  ns
I4096xI4096: 2.5102    µs

wrapping ops/div, full size
I256/I128:     351.69 ns
I512/I256:     554.63 ns
I1024/I512:  1.0663   µs
I2048/I1024: 2.5858   µs
I4096/I2048: 7.2846   µs

wrapping ops/add, 
I128+I128:     1.1915 ns
I256+I256:     2.4108 ns
I512+I512:    10.374  ns
I1024+I1024:  20.704  ns
I2048+I2048:  65.318  ns
I4096+I4096: 103.10   ns

wrapping ops/sub,
I128-I128:     1.1967 ns
I256-I256:     2.3008 ns
I512-I512:    10.985  ns
I1024-I1024:  32.041  ns
I2048-I2048:  70.872  ns
I4096-I4096: 128.86   ns

lleoha · 2024-10-16T11:24:52Z

src/int.rs

+ ///
+ /// Warning: this operation is unsafe; when `self == Self::MIN`, the negation fails.
+ #[inline]
+ fn negate_unsafe(&self) -> (Self, Choice) {


it does what is the same as self.0.wrapping_neg() does.

Thanks for pointing this out! I adapted wrapping_neg() into wrapping_negc() to include the carry and still get the is_zero for free. In addition to having no duplicate code, this saves anywhere from 8% (I128) to 1% (I4096) in the multiplication benchmark 👌

btw. why do you need is_zero in the first place?

Well, I needed it for mul and div; the fix you pointed out there makes knowing whether something is zero unnecessary (for now).

tarcieri · 2024-10-18T14:25:10Z

signed-int has been branched from master

lleoha · 2024-10-18T15:45:01Z

signed-int has been branched from master

Thanks @tarcieri

lleoha

Let's make it into the feature branch so we have a baseline to work on.
LGTM to initial commit.

tarcieri · 2024-10-18T17:55:59Z

Will do another pass tomorrow, but I agree this is already a lot

lleoha · 2024-10-19T15:30:44Z

src/int/from.rs

+ // TODO(tarcieri): replace with `const impl From<i64>` when stable
+ #[cfg(target_pointer_width = "32")]
+ pub const fn from_i64(n: i64) -> Self {
+ Int::new_from_uint(Uint::<{ I64::LIMBS }>::from_u64(n as u64)).resize()


this should work and is a little simpler:
Int::new_from_uint(Uint::from_u64(n as u 64))

I don't think that would work. Let me explain why:

In scaling up a negative integer, we have to make sure that all new limbs are initialized with Limb::MAX instead of Limb::ZERO (as is the default for Uint). So, what I'm doing here is:

cast i64 to u64,

ingest this u64 into a Uint that is as small as possible,

wrap the Uint into an Int, and

scale up the int (which I wrote to make sure it uses the right Limb type).

IIUC, Int::new_from_uint(Uint::from_u64(n as u 64)) does

cast i64 to u64,

ingest this u64 into a Uint<LIMBS>, filling any missing limbs with Limb::ZERO, and

wrapping the Uint into an Int.

This would not work for negative values.

Would you agree?

ahh, right,
I forgot that you can call from_i64 to Int with more than two limbs :)

lleoha · 2024-10-19T15:36:28Z

@tarcieri @erik-3milabs
What do you think about introducing Positive, NonPositive, Negative, NonNegative newtype structs similarly to Zero, NonZero, Odd etc.?
Also I was thinking about introducing Signed and Unsigned traits (not sure if these would be a'la marker traits) or should contains any methods.

lleoha · 2024-10-19T16:16:57Z

src/int/div.rs

+ /// I128::from(2)
+ /// )
+ /// ```
+ pub fn checked_div_floor(&self, rhs: &Self) -> CtOption<Self> {


do we need this method?
I think one method that rounds towards zero it sufficient (so it works like regular primitive int division).

Do we need it? No, probably not. Do I need it? Yes, yes I do :)

I need this operation as well as div_mod_floor for a project I'm working on (that op is one of the reasons I'm implementing this right now 🙈). IIUC, the definition I am using here aligns with num_bigint::BigInt::div_mod_floor, which I'm trying to replace to make my algorithm constant time.

This function, of course, differs slightly from Int::checked_div in that it rounds down, instead of to zero.

@erik-3milabs out of curiosity, if this is not a secret, what do you need the floor_div for?

tarcieri · 2024-10-19T16:19:06Z

@lleoha the wrappers idea sounds possibly good (although I'm unclear on use cases), however something probably best tackled after getting this PR over the finish line

lleoha · 2024-10-19T16:21:41Z

src/int/div.rs

+ /// Base checked_div_mod operation.
+ /// Given `(a, b)` computes the quotient and remainder of the absolute
+ /// Note: this operation rounds towards zero, truncating any fractional part of the exact result.
+ fn checked_div_mod(


can we instead have a method that is consistent with primitive div/rem i.e. return quotient and remainder with quotient round towards zero and both q and r being signed integer as they both can be negative e.g. -5 / 2 = (-2, -1).
Also to be consistent with Uint can we rename it to div_rem instead of div_mod?

Sorry, I was a little chaotic,
What I'm trying to say is that returning two Uints and one bit is not enough to cover all the cases:

-5/-2 = (2, -1) -5/2 = (-2, -1) 5/-2 = (-2, 1) 5/2 = (2, 1)

Ahh, didn't think of that. Let me fix this later today!

I updated the code. Let me know what you think!

erik-3milabs · 2024-10-21T10:04:11Z

Failing CI should be fixed by erik-3milabs#1 which is slated to be merged.

Add widening_mul, group absolute value operations, reformat.

erik-3milabs · 2024-10-22T09:35:45Z

Ok, this PR has gotten out of hand... Oops...

With all checks passing, I'm proposing to freeze it here; I'll create a new PR for new features I intend to introduce.

erik-3milabs · 2024-10-22T09:38:02Z

@tarcieri, could you please re-review this and let me know what still needs to be done to merge this?

erik-3milabs added 16 commits October 15, 2024 09:10

Create Int core struct

4bc098c

Introduce Int::add

31e3577

Introduce Int::mul

cfb1ec4

Introduce Int::div

01a07f7

Implement Div for Int

000c007

Clean up

39b08bb

Implement Sub for Int

7b0fbab

Fix typo in Uint::sub

66b4639

Implement WrappingSub for Int

02d9d5c

Implement cmp for Int

e614cd5

Implement rand for Int

da13a4f

Implement expand for Int

69b7236

Update Int::encoding

20cdb5f

Add Int aliases

4ab45f5

Impl some more Int traits

57000c5

Add Int benchmarks

e29e425

erik-3milabs mentioned this pull request Oct 16, 2024

[WIP] Int - sign-and-magnitude #694

Closed

erik-3milabs added 8 commits October 16, 2024 11:52

Remove superfluous tests

869a8e7

Fix fmt

f6ef7ed

Introduce Int::new_from_uint

c610b08

Feature lock Int::rand

0669c1b

Add Int::cmp tests

d7029e2

Fix Int::expand bug

a653375

Attach Int benchmark

57f65f8

Improve Int benchmarks

b23483d

lleoha reviewed Oct 16, 2024

View reviewed changes

erik-3milabs added 3 commits October 16, 2024 14:45

Speed up Int::negate_unsafe

30b4676

Improve Int::sign_bit

580e0a5

Remove useless conversion

220d329

Attempt RustCrypto#2

3378cbb

erik-3milabs changed the base branch from master to signed-int October 18, 2024 14:28

lleoha approved these changes Oct 18, 2024

View reviewed changes

lleoha reviewed Oct 19, 2024

View reviewed changes

erik-3milabs added 2 commits October 21, 2024 11:48

Introduce lte and gte for Uint

d2e5692

Introduce lte and gte for Int

6e04109

erik-3milabs and others added 10 commits October 21, 2024 12:55

Update Int::div_rem

34290de

Add widening_mul, group absolute value operations, reformat.

cb67265

bench

3773bb7

Address review remarks, add benchmarks.

e218afa

Update src/int/sign.rs

e61537f

Refine.

2624a8a

reformat

23f1f4a

Merge pull request #1 from lleoha/widening-mul

c8edd3d

Add widening_mul, group absolute value operations, reformat.

Deprecate Uint::gte

6543016

Clean Int::div

51f269a

erik-3milabs requested a review from tarcieri October 22, 2024 09:38

erik-3milabs mentioned this pull request Oct 22, 2024

Int - Bit ops #697

Draft

erik-3milabs changed the title ~~[WIP] Int - two's complement~~ Int - two's complement Oct 22, 2024

erik-3milabs mentioned this pull request Oct 22, 2024

Int - gcd #698

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Int` - two's complement #695

`Int` - two's complement #695

erik-3milabs commented Oct 16, 2024

erik-3milabs commented Oct 16, 2024 •

edited

Loading

lleoha Oct 16, 2024

erik-3milabs Oct 16, 2024 •

edited

Loading

lleoha Oct 17, 2024

erik-3milabs Oct 17, 2024

tarcieri commented Oct 18, 2024

lleoha commented Oct 18, 2024

lleoha left a comment

tarcieri commented Oct 18, 2024

lleoha Oct 19, 2024

erik-3milabs Oct 21, 2024

lleoha Oct 23, 2024

lleoha commented Oct 19, 2024

lleoha Oct 19, 2024

erik-3milabs Oct 21, 2024

lleoha Oct 23, 2024

tarcieri commented Oct 19, 2024

lleoha Oct 19, 2024 •

edited

Loading

lleoha Oct 19, 2024

erik-3milabs Oct 21, 2024

erik-3milabs Oct 21, 2024

lleoha Oct 23, 2024

erik-3milabs commented Oct 21, 2024

erik-3milabs commented Oct 22, 2024

erik-3milabs commented Oct 22, 2024

Int - two's complement #695

Are you sure you want to change the base?

Int - two's complement #695

Conversation

erik-3milabs commented Oct 16, 2024

erik-3milabs commented Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

erik-3milabs Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarcieri commented Oct 18, 2024

lleoha commented Oct 18, 2024

lleoha left a comment

Choose a reason for hiding this comment

tarcieri commented Oct 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lleoha commented Oct 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarcieri commented Oct 19, 2024

lleoha Oct 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erik-3milabs commented Oct 21, 2024

erik-3milabs commented Oct 22, 2024

erik-3milabs commented Oct 22, 2024

`Int` - two's complement #695

`Int` - two's complement #695

erik-3milabs commented Oct 16, 2024 •

edited

Loading

erik-3milabs Oct 16, 2024 •

edited

Loading

lleoha Oct 19, 2024 •

edited

Loading