From 0ee1fb3ec8da08e48d1879c0b30e687a350dc600 Mon Sep 17 00:00:00 2001 From: Mamy Ratsimbazafy Date: Wed, 2 May 2018 17:49:31 +0200 Subject: [PATCH] Fix #30 - modulo failures (#32) * Add corner cases to test suite * Add failing div2n1n case * Fix test and fix #30 * Add to tests * Fix failing tests due to Nim string change between Apr 27 and Apr 29 (doc: https://github.com/nim-lang/Nim/commit/5237ef4f5280fef8df630f265cc67869d013c0ef) --- src/private/uint_div.nim | 14 ++++----- stint.nimble | 9 ++++++ tests/internal.nim | 10 +++++++ tests/internal_uint_div.nim | 60 +++++++++++++++++++++++++++++++++++++ tests/test_uint_muldiv.nim | 26 ++++++++++++++++ 5 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 tests/internal.nim create mode 100644 tests/internal_uint_div.nim diff --git a/src/private/uint_div.nim b/src/private/uint_div.nim index 29c35ff..6ca8d85 100644 --- a/src/private/uint_div.nim +++ b/src/private/uint_div.nim @@ -137,19 +137,19 @@ func div2n1n[T: SomeunsignedInt](q, r: var T, n_hi, n_lo, d: T) = halfSize = size div 2 halfMask = (1.T shl halfSize) - 1.T - template halfQR(n_hi, n_lo, d_hi, d_lo: T): tuple[q,r: T] = + template halfQR(n_hi, n_lo, d, d_hi, d_lo: T): tuple[q,r: T] = var (q, r) = divmod(n_hi, d_hi) let m = q * d_lo - r = (r shl halfSize) or n_lo + var r = (r shl halfSize) or n_lo # Fix the reminder, we're at most 2 iterations off if r < m: dec q - r += d_hi - if r >= d_hi and r < m: + r += d + if r >= d and r < m: dec q - r += d_hi + r += d r -= m (q, r) @@ -160,10 +160,10 @@ func div2n1n[T: SomeunsignedInt](q, r: var T, n_hi, n_lo, d: T) = n_lolo = nlo and halfMask # First half of the quotient - let (q1, r1) = halfQR(n_hi, n_lohi, d_hi, d_lo) + let (q1, r1) = halfQR(n_hi, n_lohi, d, d_hi, d_lo) # Second half - let (q2, r2) = halfQR(r1, n_lolo, d_hi, d_lo) + let (q2, r2) = halfQR(r1, n_lolo, d, d_hi, d_lo) q = (q1 shl halfSize) or q2 r = r2 diff --git a/stint.nimble b/stint.nimble index 004901e..6fe3bd5 100644 --- a/stint.nimble +++ b/stint.nimble @@ -20,6 +20,13 @@ proc test(name: string, lang: string = "c") = switch("out", ("./build/" & name)) setCommand lang, "tests/" & name & ".nim" +task test_internal_debug, "Run tests for internal procs - test implementation (StUint[64] = 2x uint32": + switch("define", "mpint_test") + test "internal" + +task test_internal_release, "Run tests for internal procs - prod implementation (StUint[64] = uint64": + test "internal" + task test_debug, "Run all tests - test implementation (StUint[64] = 2x uint32": switch("define", "mpint_test") test "all_tests" @@ -52,6 +59,8 @@ task test_property_uint256_release, "Run random tests (release mode) vs TTMath o test "property_based", "cpp" task test, "Run all tests - test and production implementation": + exec "nimble test_internal_debug" + exec "nimble test_internal_release" exec "nimble test_debug" exec "nimble test_release" exec "nimble test_property_debug" diff --git a/tests/internal.nim b/tests/internal.nim new file mode 100644 index 0000000..cf0e795 --- /dev/null +++ b/tests/internal.nim @@ -0,0 +1,10 @@ +# Stint +# Copyright 2018 Status Research & Development GmbH +# Licensed under either of +# +# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0) +# * MIT license ([LICENSE-MIT](LICENSE-MIT) or http://opensource.org/licenses/MIT) +# +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import internal_uint_div diff --git a/tests/internal_uint_div.nim b/tests/internal_uint_div.nim new file mode 100644 index 0000000..1ed2fc9 --- /dev/null +++ b/tests/internal_uint_div.nim @@ -0,0 +1,60 @@ +# Stint +# Copyright 2018 Status Research & Development GmbH +# Licensed under either of +# +# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0) +# * MIT license ([LICENSE-MIT](LICENSE-MIT) or http://opensource.org/licenses/MIT) +# +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +# Test implementation of internal proc: + +include ../src/private/uint_div +import unittest + +suite "implementation of internal division procecures": + test "Division of 2 words by 1 - specific carry case (issue #30)": + + # // Reference implementation from glibc: udiv.h + + # #include + # typedef uint32_t USItype; + + # #define udiv_qrnnd(q, r, n1, n0, dx) /* d renamed to dx avoiding "=d" */\ + # __asm__ ("divl %4" /* stringification in K&R C */ \ + # : "=a" (q), "=d" (r) \ + # : "0" ((USItype)(n0)), "1" ((USItype)(n1)), "rm" ((USItype)(dx))) + + ################################ + + # #include "udiv.h" + # #include + # int main( int argc, const char* argv[] ) + # { + # UWtype q, r, n1, n0, d; + + # q = 0; + # r = 0; + # n1 = 233; + # n0 = 1717253765; + # d = 2659025738; + + # udiv_qrnnd(q, r, n1, n0, d); + + # printf("q: %u\n", q); + # printf("r: %u\n", r); + # } + + ############################### + + var q, r: uint32 + let + n1 = uint32 233 + n0 = uint32 1717253765 + d = uint32 2659025738 + + div2n1n(q, r, n1, n0, d) + + check: + q == 376 + r == 2650956245'u32 diff --git a/tests/test_uint_muldiv.nim b/tests/test_uint_muldiv.nim index e77390c..1dd466c 100644 --- a/tests/test_uint_muldiv.nim +++ b/tests/test_uint_muldiv.nim @@ -68,3 +68,29 @@ suite "Testing unsigned int division and modulo implementation": check: q == 123456789123456789'u64 check: r == 0'u64 + +suite "Testing specific failures highlighted by property-based testing": + test "Modulo: 65696211516342324 mod 174261910798982": + + let u = 65696211516342324'u64 + let v = 174261910798982'u64 + + let a = cast[Stuint[64]](u) + let b = cast[Stuint[64]](v) + + let z = u mod v + let tz = cast[uint64](a mod b) + + check: z == tz + + test "Modulo: 15080397990160655 mod 600432699691": + let u = 15080397990160655'u64 + let v = 600432699691'u64 + + let a = cast[Stuint[64]](u) + let b = cast[Stuint[64]](v) + + let z = u mod v + let tz = cast[uint64](a mod b) + + check: z == tz