Conversation
| } | ||
|
|
||
| impl Ord for i256 { | ||
| fn cmp(&self, other: &Self) -> Ordering { |
There was a problem hiding this comment.
This is related to #2360, performing the comparison this way is significantly faster than using memcmp, which is itself significantly faster than using BigInt.
FYI @liukun4515
| /// Performs wrapping multiplication | ||
| #[inline] | ||
| pub fn wrapping_mul(self, other: Self) -> Self { | ||
| let l = BigInt::from_signed_bytes_le(&self.0); | ||
| let r = BigInt::from_signed_bytes_le(&other.0); | ||
| Self::from_bigint_with_overflow(l * r).0 | ||
| } | ||
|
|
||
| /// Performs checked multiplication | ||
| #[inline] | ||
| pub fn checked_mul(self, other: Self) -> Option<Self> { | ||
| let l = BigInt::from_signed_bytes_le(&self.0); | ||
| let r = BigInt::from_signed_bytes_le(&other.0); | ||
| let (val, overflow) = Self::from_bigint_with_overflow(l * r); | ||
| (!overflow).then(|| val) | ||
| } | ||
|
|
||
| /// Performs wrapping division | ||
| #[inline] | ||
| pub fn wrapping_div(self, other: Self) -> Self { | ||
| let l = BigInt::from_signed_bytes_le(&self.0); | ||
| let r = BigInt::from_signed_bytes_le(&other.0); | ||
| Self::from_bigint_with_overflow(l / r).0 | ||
| } | ||
|
|
||
| /// Performs checked division | ||
| #[inline] | ||
| pub fn checked_div(self, other: Self) -> Option<Self> { | ||
| let l = BigInt::from_signed_bytes_le(&self.0); | ||
| let r = BigInt::from_signed_bytes_le(&other.0); | ||
| let (val, overflow) = Self::from_bigint_with_overflow(l / r); | ||
| (!overflow).then(|| val) | ||
| } | ||
|
|
||
| /// Performs wrapping remainder | ||
| #[inline] | ||
| pub fn wrapping_rem(self, other: Self) -> Self { | ||
| let l = BigInt::from_signed_bytes_le(&self.0); | ||
| let r = BigInt::from_signed_bytes_le(&other.0); | ||
| Self::from_bigint_with_overflow(l % r).0 | ||
| } | ||
|
|
||
| /// Performs checked division | ||
| #[inline] | ||
| pub fn checked_rem(self, other: Self) -> Option<Self> { | ||
| if other.0 == [0; 32] { | ||
| return None; | ||
| } | ||
|
|
||
| let l = BigInt::from_signed_bytes_le(&self.0); | ||
| let r = BigInt::from_signed_bytes_le(&other.0); | ||
| let (val, overflow) = Self::from_bigint_with_overflow(l % r); | ||
| (!overflow).then(|| val) | ||
| } | ||
| } |
There was a problem hiding this comment.
Theoretically these would all benefit from an implementation not using BigInt, as this elides allocations and branches, but unlike the addition and subtraction kernels these are significantly more complicated to implement. I leave this as a potential future optimisation for someone who really cares about decimal256 performance
|
I need more time to review it tomorrow. Thanks @tustvold |
| match overflow { | ||
| true => assert!(checked.is_none()), | ||
| false => assert_eq!(checked.unwrap(), actual), | ||
| } |
There was a problem hiding this comment.
This doesn't include tests for div, rem because they are implemented using BigInt?
There was a problem hiding this comment.
Yeah I figured that would be a bit circular
| } | ||
|
|
||
| #[inline] | ||
| fn mulx(a: u128, b: u128) -> (u128, u128) { |
|
Benchmark runs are scheduled for baseline = 9b59081 and contender = 4df1f3c. 4df1f3c is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
|
There appears to be a mistake somewhere in this 😢 will look into https://github.com/apache/arrow-rs/actions/runs/3174304664/jobs/5170929134 |
4 participants
Which issue does this PR close?
Part of #2637
Rationale for this change
This adds an i256 that can be used as the basis for a Decimal256Array, and implements common arithmetic for it.
What changes are included in this PR?
Are there any user-facing changes?