feat(integer): add unsigned_oveflowing_add #674
Conversation
|
@slab-ci cpu_fast_test |
tmontaigu
force-pushed
the
unsigned-overflowing-add
branch
from
635d9f0 to
c168f51
Compare
|
@slab-ci cpu_fast_test |
| fn overflowing_add_under_modulus(lhs: u64, rhs: u64, modulus: u64) -> (u64, bool) { | ||
| let result = lhs.wrapping_add(rhs); | ||
| (result % modulus, result >= modulus) | ||
| } |
There was a problem hiding this comment.
let's say your modulus is 2^63 + 1
I take 2^63 + 2^63 result is 2^64 or 0 in a u64, the computation overflowed but we would not see it here, am I correct ?
Maybe the sub under modulus is wrong as well 🤔
There was a problem hiding this comment.
fn overflowing_sub_under_modulus(lhs: u64, rhs: u64, modulus: u64) -> (u64, bool) {
let overflowed = rhs > lhs;
let result = if overflowed {
todo!() // maybe use wrapping_sub_custom mod from the unsigned integer trait
} else {
lhs - rhs
};
(result, overflowed)
}There was a problem hiding this comment.
Yeah, but these functions are written only for the tests where we don't go beyond a modulus of 2^16
There was a problem hiding this comment.
understood, could you add the assert in the two functions to avoid issues on the value ranges ? or update them so that we don't mistakenly use them in contexts where they are not meant to be used ?
There was a problem hiding this comment.
and sorry for being annoying with this but I just battled against moduli 😅 I'd rather we don't have weird surprises
There was a problem hiding this comment.
fn overflowing_add_under_modulus(lhs: u64, rhs: u64, modulus: u64) -> (u64, bool) {
let (result, overflowed) = lhs.overflowing_add(rhs);
(result % modulus, overfowed || result >= modulus)
}should work for all moduluses
There was a problem hiding this comment.
Well maybe not for non-power of two modulus that have their sum overflow u64, but we don't have that
There was a problem hiding this comment.
could you take a look for the sub as well ?
tmontaigu
force-pushed
the
unsigned-overflowing-add
branch
from
c168f51 to
34f38e8
Compare
|
@slab-ci cpu_fast_test |
tfhe/src/integer/server_key/radix_parallel/tests_cases_unsigned.rs
Outdated
Show resolved
Hide resolved
tmontaigu
force-pushed
the
unsigned-overflowing-add
branch
from
34f38e8 to
72f1aee
Compare
|
@slab-ci cpu_fast_test |
|
Pull Request has been approved 🎉 |
No description provided.