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.

Sign up for GitHub

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

Simplification of BigNum::bit_length #92747

Merged
merged 2 commits into from
Jan 15, 2022
Merged

Simplification of BigNum::bit_length #92747

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
424f38f
Simplification of BigNum::bit_length
swenson Jan 10, 2022
0589cac
Simplify BigNum::bit_length() with log2()
swenson Jan 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions library/core/src/num/bignum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,20 +162,13 @@ macro_rules! define_bignum {
let digits = self.digits();
let zeros = digits.iter().rev().take_while(|&&x| x == 0).count();
let end = digits.len() - zeros;
Copy link
Member

Choose a reason for hiding this comment

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

(I'm new to this code, so let me know if I'm misunderstanding things. It took me a bit to figure out what was going on.)

I think this could be simplified further?

If I'm reading it right, I think this is something like

let digits = self.digits();
// Find the most significant non-zero digit
let msd = digits.iter().rposition(|&x| x != 0);
if let Some(msd) = msd {
    let digitbits = <$ty>::BITS as usize;
    msd * digitbits + digits[msd].log2() as usize
} else {
    // There are no non-zero digits, i.e., the number is zero.
    return 0;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the log2() would be slower (more complex instruction that does a lot we don't need) than a CLZ, but the rest of the changes I can certainly incorporate. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

log2 on integers is just BITS - CLZ, though:

rust/library/core/src/num/uint_macros.rs

Lines 841 to 849 in 23ce5fc

pub const fn checked_log2(self) -> Option<u32> {
if self <= 0 {
None
} else {
// SAFETY: We just checked that this number is positive
let log = (Self::BITS - 1) - unsafe { intrinsics::ctlz_nonzero(self) as u32 };
Some(log)
}
}

(Oh, I might be off-by-one in the code. Up to you if you like ... + log2() + 1 or (msd + 1) * digitbits - leading_zeros() or whatever best.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, my bad, I thought log2() would return the actual logarithm base 2, not just the integer part. I'll totally use that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

log2 on integers is just BITS - CLZ, though:

rust/library/core/src/num/uint_macros.rs

Lines 841 to 849 in 23ce5fc

pub const fn checked_log2(self) -> Option<u32> {
if self <= 0 {
None
} else {
// SAFETY: We just checked that this number is positive
let log = (Self::BITS - 1) - unsafe { intrinsics::ctlz_nonzero(self) as u32 };
Some(log)
}
}

(Oh, I might be off-by-one in the code. Up to you if you like ... + log2() + 1 or (msd + 1) * digitbits - leading_zeros() or whatever best.)

Yep! We're always off-by-one the first time or two. :) I worked it out.

I went with a match statement instead of if let since it is one line shorter.

let nonzero = &digits[..end];

if nonzero.is_empty() {
if end == 0 {
// There are no non-zero digits, i.e., the number is zero.
return 0;
}
// This could be optimized with leading_zeros() and bit shifts, but that's
// probably not worth the hassle.
let digitbits = <$ty>::BITS as usize;
let mut i = nonzero.len() * digitbits - 1;
while self.get_bit(i) == 0 {
i -= 1;
}
i + 1
let end_leading_zeros = digits[end - 1].leading_zeros() as usize;
end * digitbits - end_leading_zeros
}

/// Adds `other` to itself and returns its own mutable reference.
Expand Down
35 changes: 35 additions & 0 deletions library/core/tests/num/bignum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use core::num::bignum::tests::Big8x3 as Big;
use core::num::bignum::Big32x40;

#[test]
#[should_panic]
Expand Down Expand Up @@ -215,6 +216,16 @@ fn test_get_bit_out_of_range() {

#[test]
fn test_bit_length() {
for i in 0..8 * 3 {
// 010000...000
assert_eq!(Big::from_small(1).mul_pow2(i).bit_length(), i + 1);
}
for i in 1..8 * 3 - 1 {
// 010000...001
assert_eq!(Big::from_small(1).mul_pow2(i).add(&Big::from_small(1)).bit_length(), i + 1);
// 110000...000
assert_eq!(Big::from_small(3).mul_pow2(i).bit_length(), i + 2);
}
assert_eq!(Big::from_small(0).bit_length(), 0);
assert_eq!(Big::from_small(1).bit_length(), 1);
assert_eq!(Big::from_small(5).bit_length(), 3);
Expand All @@ -223,6 +234,30 @@ fn test_bit_length() {
assert_eq!(Big::from_u64(0xffffff).bit_length(), 24);
}

#[test]
fn test_bit_length_32x40() {
for i in 0..32 * 40 {
// 010000...000
assert_eq!(Big32x40::from_small(1).mul_pow2(i).bit_length(), i + 1);
}
for i in 1..32 * 40 - 1 {
// 010000...001
assert_eq!(
Big32x40::from_small(1).mul_pow2(i).add(&Big32x40::from_small(1)).bit_length(),
i + 1
);
// 110000...000
assert_eq!(Big32x40::from_small(3).mul_pow2(i).bit_length(), i + 2);
}
assert_eq!(Big32x40::from_small(0).bit_length(), 0);
assert_eq!(Big32x40::from_small(1).bit_length(), 1);
assert_eq!(Big32x40::from_small(5).bit_length(), 3);
assert_eq!(Big32x40::from_small(0x18).bit_length(), 5);
assert_eq!(Big32x40::from_u64(0x4073).bit_length(), 15);
assert_eq!(Big32x40::from_u64(0xffffff).bit_length(), 24);
assert_eq!(Big32x40::from_u64(0xffffffffffffffff).bit_length(), 64);
}

#[test]
fn test_ord() {
assert!(Big::from_u64(0) < Big::from_u64(0xffffff));
Expand Down