Skip to content

Commit 7f2d022

Browse files
authored
GCD bug fix
The standard-library documentation for `trailingZeroBitCount` states: > If the value is zero, then `trailingZeroBitCount` is equal to `bitWidth`. Thus, if `T` is not a fixed-width integer type, and `x == 0`, and `y` has more trailing zeros than the representation of `x`, then the gcd calculation will be incorrect. In particular, the left-shift by `min(xtz, ytz) == xtz` will not “undo” the earlier right-shift by `ytz`, so the final result will be incorrectly right-shifted by `ytz - xtz`. To fix this I have added an early exit when `x == 0`. I am not sure if this is the optimal solution, and I’m open to alternatives. *** This change ensures that `x != 0` at the top of the loop, so I have also converted it from a `while` to a `repeat-while` loop, in order to eliminate a redundant extra comparison on the first iteration. I don’t know if this actually affects performance, and I’m happy to change it back if that’s preferred.
1 parent 4ce74b4 commit 7f2d022

File tree

1 file changed

+6
-2
lines changed

1 file changed

+6
-2
lines changed

Sources/IntegerUtilities/GCD.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ public func gcd<T: BinaryInteger>(_ a: T, _ b: T) -> T {
2727
var x = a.magnitude
2828
var y = b.magnitude
2929

30+
if x == 0 { return T(y) }
3031
if y == 0 { return T(x) }
3132

3233
let xtz = x.trailingZeroBitCount
@@ -39,11 +40,14 @@ public func gcd<T: BinaryInteger>(_ a: T, _ b: T) -> T {
3940
// After the right-shift in the loop, both x and y are odd. Each pass removes
4041
// at least one low-order bit from the larger of the two, so the number of
4142
// iterations is bounded by the sum of the bit-widths of the inputs.
42-
while x != 0 {
43+
//
44+
// A tighter bound is the maximum bit-width of the inputs, which is achieved
45+
// by odd numbers that sum to a power of 2, though the proof is more involved.
46+
repeat {
4347
x >>= x.trailingZeroBitCount
4448
if x < y { swap(&x, &y) }
4549
x -= y
46-
}
50+
} while x != 0
4751

4852
return T(y << min(xtz, ytz))
4953
}

0 commit comments

Comments
 (0)