From 424f38f211c23a9cf3d70a3bed17c30e79d49760 Mon Sep 17 00:00:00 2001
From: Christopher Swenson <swenson@swenson.io>
Date: Mon, 10 Jan 2022 14:18:28 -0800
Subject: [PATCH 1/2] Simplification of BigNum::bit_length

As indicated in the comment, the BigNum::bit_length function could be
optimized by using CLZ, which is often a single instruction instead a
loop.

I think the code is also simpler now without the loop.

I added some additional tests for Big8x3 and Big32x40 to ensure that
there were no regressions.
---
 library/core/src/num/bignum.rs   | 13 +++---------
 library/core/tests/num/bignum.rs | 35 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/library/core/src/num/bignum.rs b/library/core/src/num/bignum.rs
index 8a06a0988829b..65c1753895e71 100644
--- a/library/core/src/num/bignum.rs
+++ b/library/core/src/num/bignum.rs
@@ -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;
-                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.
diff --git a/library/core/tests/num/bignum.rs b/library/core/tests/num/bignum.rs
index 1457064cc8d57..416e7cea7a67b 100644
--- a/library/core/tests/num/bignum.rs
+++ b/library/core/tests/num/bignum.rs
@@ -1,4 +1,5 @@
 use core::num::bignum::tests::Big8x3 as Big;
+use core::num::bignum::Big32x40;
 
 #[test]
 #[should_panic]
@@ -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);
@@ -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));

From 0589cace8c943d40a60b7356d8c772baf2879cee Mon Sep 17 00:00:00 2001
From: Christopher Swenson <swenson@swenson.io>
Date: Mon, 10 Jan 2022 15:31:11 -0800
Subject: [PATCH 2/2] Simplify BigNum::bit_length() with log2()

Thank you to @scottmcm for suggesting the handy `log2()` function.
---
 library/core/src/num/bignum.rs | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/library/core/src/num/bignum.rs b/library/core/src/num/bignum.rs
index 65c1753895e71..98d8a8a1d74ae 100644
--- a/library/core/src/num/bignum.rs
+++ b/library/core/src/num/bignum.rs
@@ -158,17 +158,15 @@ macro_rules! define_bignum {
             /// Returns the number of bits necessary to represent this value. Note that zero
             /// is considered to need 0 bits.
             pub fn bit_length(&self) -> usize {
-                // Skip over the most significant digits which are zero.
+                let digitbits = <$ty>::BITS as usize;
                 let digits = self.digits();
-                let zeros = digits.iter().rev().take_while(|&&x| x == 0).count();
-                let end = digits.len() - zeros;
-                if end == 0 {
+                // Find the most significant non-zero digit.
+                let msd = digits.iter().rposition(|&x| x != 0);
+                match msd {
+                    Some(msd) => msd * digitbits + digits[msd].log2() as usize + 1,
                     // There are no non-zero digits, i.e., the number is zero.
-                    return 0;
+                    _ => 0,
                 }
-                let digitbits = <$ty>::BITS as usize;
-                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.