From 69376af18316510e568af21b9198be7af9bef8df Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Sat, 13 Aug 2016 16:58:44 -0500 Subject: [PATCH 1/3] quickcheck: better generation of input arguments closes #31 --- src/lib.rs | 8 +++- src/mul.rs | 11 +++-- src/qc.rs | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/shift.rs | 11 +++-- src/udiv.rs | 19 +++++--- 5 files changed, 159 insertions(+), 13 deletions(-) create mode 100644 src/qc.rs diff --git a/src/lib.rs b/src/lib.rs index 6d5580b..823170a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,8 +1,8 @@ #![allow(unused_features)] -#![no_std] #![feature(asm)] #![feature(core_intrinsics)] #![feature(naked_functions)] +#![cfg_attr(not(test), no_std)] // TODO(rust-lang/rust#35021) uncomment when that PR lands // #![feature(rustc_builtins)] @@ -13,6 +13,9 @@ #[macro_use] extern crate quickcheck; +#[cfg(test)] +extern crate core; + #[cfg(target_arch = "arm")] pub mod arm; @@ -20,6 +23,9 @@ pub mod udiv; pub mod mul; pub mod shift; +#[cfg(test)] +mod qc; + /// Trait for some basic operations on integers trait Int { fn bits() -> u32; diff --git a/src/mul.rs b/src/mul.rs index a4350b5..b93fdb7 100644 --- a/src/mul.rs +++ b/src/mul.rs @@ -72,13 +72,17 @@ mulo!(__mulodi4: i64); #[cfg(test)] mod tests { + use qc::{I32, I64, U64}; + quickcheck! { - fn muldi(a: u64, b: u64) -> bool { + fn muldi(a: U64, b: U64) -> bool { + let (a, b) = (a.0, b.0); let r = super::__muldi4(a, b); r == a.wrapping_mul(b) } - fn mulosi(a: i32, b: i32) -> bool { + fn mulosi(a: I32, b: I32) -> bool { + let (a, b) = (a.0, b.0); let mut overflow = 2; let r = super::__mulosi4(a, b, &mut overflow); if overflow != 0 && overflow != 1 { @@ -87,7 +91,8 @@ mod tests { (r, overflow != 0) == a.overflowing_mul(b) } - fn mulodi(a: i64, b: i64) -> bool { + fn mulodi(a: I64, b: I64) -> bool { + let (a, b) = (a.0, b.0); let mut overflow = 2; let r = super::__mulodi4(a, b, &mut overflow); if overflow != 0 && overflow != 1 { diff --git a/src/qc.rs b/src/qc.rs new file mode 100644 index 0000000..6b37cce --- /dev/null +++ b/src/qc.rs @@ -0,0 +1,123 @@ +// When testing functions, QuickCheck (QC) uses small values for integer (`u*`/`i*`) arguments +// (~ `[-100, 100]`), but these values don't stress all the code paths in our intrinsics. Here we +// create newtypes over the primitive integer types with the goal of having full control over the +// random values that will be used to test our intrinsics. + +use std::boxed::Box; +use std::fmt; + +use quickcheck::{Arbitrary, Gen}; + +use LargeInt; + +// Generates values in the full range of the integer type +macro_rules! arbitrary { + ($TY:ident : $ty:ident) => { + #[derive(Clone, Copy)] + pub struct $TY(pub $ty); + + impl Arbitrary for $TY { + fn arbitrary(g: &mut G) -> $TY + where G: Gen + { + $TY(g.gen()) + } + + fn shrink(&self) -> Box> { + struct Shrinker { + x: $ty, + } + + impl Iterator for Shrinker { + type Item = $TY; + + fn next(&mut self) -> Option<$TY> { + self.x /= 2; + if self.x == 0 { + None + } else { + Some($TY(self.x)) + } + } + } + + if self.0 == 0 { + ::quickcheck::empty_shrinker() + } else { + Box::new(Shrinker { x: self.0 }) + } + } + } + + impl fmt::Debug for $TY { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Debug::fmt(&self.0, f) + } + } + } +} + +arbitrary!(I32: i32); +arbitrary!(U32: u32); + +// These integers are "too large". If we generate e.g. `u64` values in the full range then there's +// only `1 / 2^32` chance of seeing a value smaller than `2^32` (i.e. whose higher "word" (32-bits) +// is `0`)! But this is an important group of values to tests because we have special code paths for +// them. Instead we'll generate e.g. `u64` integers this way: uniformly pick between (a) setting the +// low word to 0 and generating a random high word, (b) vice versa: high word to 0 and random low +// word or (c) generate both words randomly. This let's cover better the code paths in our +// intrinsics. +macro_rules! arbitrary_large { + ($TY:ident : $ty:ident) => { + #[derive(Clone, Copy)] + pub struct $TY(pub $ty); + + impl Arbitrary for $TY { + fn arbitrary(g: &mut G) -> $TY + where G: Gen + { + if g.gen() { + $TY($ty::from_parts(g.gen(), g.gen())) + } else if g.gen() { + $TY($ty::from_parts(0, g.gen())) + } else { + $TY($ty::from_parts(g.gen(), 0)) + } + } + + fn shrink(&self) -> Box> { + struct Shrinker { + x: $ty, + } + + impl Iterator for Shrinker { + type Item = $TY; + + fn next(&mut self) -> Option<$TY> { + self.x /= 2; + if self.x == 0 { + None + } else { + Some($TY(self.x)) + } + } + } + + if self.0 == 0 { + ::quickcheck::empty_shrinker() + } else { + Box::new(Shrinker { x: self.0 }) + } + } + } + + impl fmt::Debug for $TY { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Debug::fmt(&self.0, f) + } + } + } +} + +arbitrary_large!(I64: i64); +arbitrary_large!(U64: u64); diff --git a/src/shift.rs b/src/shift.rs index 909d6f4..7fa88ab 100644 --- a/src/shift.rs +++ b/src/shift.rs @@ -61,9 +61,12 @@ lshr!(__lshrdi3: u64); #[cfg(test)] mod tests { use quickcheck::TestResult; + use qc::{I64, U64}; + // NOTE We purposefully stick to `u32` for `b` here because we want "small" values (b < 64) quickcheck! { - fn ashldi(a: u64, b: u32) -> TestResult { + fn ashldi(a: U64, b: u32) -> TestResult { + let a = a.0; if b >= 64 { TestResult::discard() } else { @@ -72,7 +75,8 @@ mod tests { } } - fn ashrdi(a: i64, b: u32) -> TestResult { + fn ashrdi(a: I64, b: u32) -> TestResult { + let a = a.0; if b >= 64 { TestResult::discard() } else { @@ -81,7 +85,8 @@ mod tests { } } - fn lshrdi(a: u64, b: u32) -> TestResult { + fn lshrdi(a: U64, b: u32) -> TestResult { + let a = a.0; if b >= 64 { TestResult::discard() } else { diff --git a/src/udiv.rs b/src/udiv.rs index 95a550f..f1037c6 100644 --- a/src/udiv.rs +++ b/src/udiv.rs @@ -229,9 +229,11 @@ pub extern "C" fn __udivmoddi4(n: u64, d: u64, rem: Option<&mut u64>) -> u64 { #[cfg(test)] mod tests { use quickcheck::TestResult; + use qc::{U32, U64}; quickcheck!{ - fn udivdi3(n: u64, d: u64) -> TestResult { + fn udivdi3(n: U64, d: U64) -> TestResult { + let (n, d) = (n.0, d.0); if d == 0 { TestResult::discard() } else { @@ -240,7 +242,8 @@ mod tests { } } - fn umoddi3(n: u64, d: u64) -> TestResult { + fn umoddi3(n: U64, d: U64) -> TestResult { + let (n, d) = (n.0, d.0); if d == 0 { TestResult::discard() } else { @@ -249,7 +252,8 @@ mod tests { } } - fn udivmoddi4(n: u64, d: u64) -> TestResult { + fn udivmoddi4(n: U64, d: U64) -> TestResult { + let (n, d) = (n.0, d.0); if d == 0 { TestResult::discard() } else { @@ -259,7 +263,8 @@ mod tests { } } - fn udivsi3(n: u32, d: u32) -> TestResult { + fn udivsi3(n: U32, d: U32) -> TestResult { + let (n, d) = (n.0, d.0); if d == 0 { TestResult::discard() } else { @@ -268,7 +273,8 @@ mod tests { } } - fn umodsi3(n: u32, d: u32) -> TestResult { + fn umodsi3(n: U32, d: U32) -> TestResult { + let (n, d) = (n.0, d.0); if d == 0 { TestResult::discard() } else { @@ -277,7 +283,8 @@ mod tests { } } - fn udivmodsi4(n: u32, d: u32) -> TestResult { + fn udivmodsi4(n: U32, d: U32) -> TestResult { + let (n, d) = (n.0, d.0); if d == 0 { TestResult::discard() } else { From f3eb08a96d3c17c8f042d5f0a53de90f179d06f1 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Sat, 13 Aug 2016 18:45:00 -0500 Subject: [PATCH 2/3] fix a bug in udivmoddi4 --- src/udiv.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udiv.rs b/src/udiv.rs index f1037c6..82932e3 100644 --- a/src/udiv.rs +++ b/src/udiv.rs @@ -186,8 +186,8 @@ pub extern "C" fn __udivmoddi4(n: u64, d: u64, rem: Option<&mut u64>) -> u64 { if sr > u32::bits() - 1 { if let Some(rem) = rem { *rem = n; - return 0; } + return 0; } sr += 1; From 45aec943d346031256c967641eaeb77249ad9c7f Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Sat, 13 Aug 2016 18:47:10 -0500 Subject: [PATCH 3/3] use wrapping_add in muldi4 --- src/mul.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mul.rs b/src/mul.rs index b93fdb7..77dba26 100644 --- a/src/mul.rs +++ b/src/mul.rs @@ -19,7 +19,7 @@ macro_rules! mul { low += (t & lower_mask) << half_bits; high += t >> half_bits; high += (a.low() >> half_bits) * (b.low() >> half_bits); - high += a.high().wrapping_mul(b.low()) + a.low().wrapping_mul(b.high()); + high = high.wrapping_add(a.high().wrapping_mul(b.low()).wrapping_add(a.low().wrapping_mul(b.high()))); <$ty>::from_parts(low, high) } }