cmov: make Cmov::cmovz a provided method (#871)

This makes it easier to implement the `Cmov` trait for foreign types.

It additionally makes the `value` argument an `&Self` reference, which
should make it easier to impl the trait for non-`Copy` types where it's
desirable to avoid a move.

Note: breaking change

Author: tarcieri

Cargo.lock

@@ -6,7 +6,7 @@ constant-time and not be rewritten as branches by the compiler.
 Provides wrappers for the CMOV family of instructions on x86/x86_64
 and CSEL on AArch64.
 """
-version = "0.2.0"
+version = "0.3.0-pre"
 authors = ["RustCrypto Developers"]
 license = "Apache-2.0 OR MIT"
 repository = "https://github.com/RustCrypto/utils/tree/master/cmov"

cmov/Cargo.toml

@@ -19,71 +19,71 @@ macro_rules! csel {
 
 impl Cmov for u16 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
         csel!(
             "cmp {0:w}, 0",
-            "csel {1:w}, {2:w}, {3:w}, EQ",
+            "csel {1:w}, {2:w}, {3:w}, NE",
             self,
-            value,
+            *value,
             condition
         );
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
         csel!(
             "cmp {0:w}, 0",
-            "csel {1:w}, {2:w}, {3:w}, NE",
+            "csel {1:w}, {2:w}, {3:w}, EQ",
             self,
-            value,
+            *value,
             condition
         );
     }
 }
 
 impl Cmov for u32 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
         csel!(
             "cmp {0:w}, 0",
-            "csel {1:w}, {2:w}, {3:w}, EQ",
+            "csel {1:w}, {2:w}, {3:w}, NE",
             self,
-            value,
+            *value,
             condition
         );
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
         csel!(
             "cmp {0:w}, 0",
-            "csel {1:w}, {2:w}, {3:w}, NE",
+            "csel {1:w}, {2:w}, {3:w}, EQ",
             self,
-            value,
+            *value,
             condition
         );
     }
 }
 
 impl Cmov for u64 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
         csel!(
             "cmp {0:x}, 0",
-            "csel {1:x}, {2:x}, {3:x}, EQ",
+            "csel {1:x}, {2:x}, {3:x}, NE",
             self,
-            value,
+            *value,
             condition
         );
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
         csel!(
             "cmp {0:x}, 0",
-            "csel {1:x}, {2:x}, {3:x}, NE",
+            "csel {1:x}, {2:x}, {3:x}, EQ",
             self,
-            value,
+            *value,
             condition
         );
     }

cmov/src/aarch64.rs

@@ -24,56 +24,58 @@ pub type Condition = u8;
 /// Conditional move
 // TODO(tarcieri): make one of `cmovz`/`cmovnz` a provided method which calls the other?
 pub trait Cmov {
-    /// Move if zero.
-    ///
-    /// Uses a `cmp` instruction to check if the given `condition` value is
-    /// equal to zero, and if so, conditionally moves `value` to `self`
-    /// when `condition` is equal to zero.
-    fn cmovz(&mut self, value: Self, condition: Condition);
-
     /// Move if non-zero.
     ///
     /// Uses a `test` instruction to check if the given `condition` value is
     /// equal to zero, conditionally moves `value` to `self` when `condition` is
     /// equal to zero.
-    fn cmovnz(&mut self, value: Self, condition: Condition);
+    fn cmovnz(&mut self, value: &Self, condition: Condition);
+
+    /// Move if zero.
+    ///
+    /// Uses a `cmp` instruction to check if the given `condition` value is
+    /// equal to zero, and if so, conditionally moves `value` to `self`
+    /// when `condition` is equal to zero.
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
+        self.cmovnz(value, !condition)
+    }
 }
 
 impl Cmov for u8 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
         let mut tmp = *self as u16;
-        tmp.cmovz(value as u16, condition);
+        tmp.cmovnz(&(*value as u16), condition);
         *self = tmp as u8;
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
         let mut tmp = *self as u16;
-        tmp.cmovnz(value as u16, condition);
+        tmp.cmovz(&(*value as u16), condition);
         *self = tmp as u8;
     }
 }
 
 impl Cmov for u128 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
         let mut lo = (*self & u64::MAX as u128) as u64;
         let mut hi = (*self >> 64) as u64;
 
-        lo.cmovz((value & u64::MAX as u128) as u64, condition);
-        hi.cmovz((value >> 64) as u64, condition);
+        lo.cmovnz(&((*value & u64::MAX as u128) as u64), condition);
+        hi.cmovnz(&((*value >> 64) as u64), condition);
 
         *self = (lo as u128) | (hi as u128) << 64;
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
         let mut lo = (*self & u64::MAX as u128) as u64;
         let mut hi = (*self >> 64) as u64;
 
-        lo.cmovnz((value & u64::MAX as u128) as u64, condition);
-        hi.cmovnz((value >> 64) as u64, condition);
+        lo.cmovz(&((*value & u64::MAX as u128) as u64), condition);
+        hi.cmovz(&((*value >> 64) as u64), condition);
 
         *self = (lo as u128) | (hi as u128) << 64;
     }

cmov/src/lib.rs

@@ -10,47 +10,47 @@ use crate::{Cmov, Condition};
 
 impl Cmov for u16 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
         let mut tmp = *self as u64;
-        tmp.cmovz(value as u64, condition);
+        tmp.cmovnz(&(*value as u64), condition);
         *self = tmp as u16;
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
         let mut tmp = *self as u64;
-        tmp.cmovnz(value as u64, condition);
+        tmp.cmovz(&(*value as u64), condition);
         *self = tmp as u16;
     }
 }
 
 impl Cmov for u32 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
         let mut tmp = *self as u64;
-        tmp.cmovz(value as u64, condition);
+        tmp.cmovnz(&(*value as u64), condition);
         *self = tmp as u32;
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
         let mut tmp = *self as u64;
-        tmp.cmovnz(value as u64, condition);
+        tmp.cmovz(&(*value as u64), condition);
         *self = tmp as u32;
     }
 }
 
 impl Cmov for u64 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
-        let mask = (1 ^ is_non_zero(condition)).wrapping_sub(1);
-        *self = (*self & mask) | (value & !mask);
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
+        let mask = is_non_zero(condition).wrapping_sub(1);
+        *self = (*self & mask) | (*value & !mask);
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
-        let mask = is_non_zero(condition).wrapping_sub(1);
-        *self = (*self & mask) | (value & !mask);
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
+        let mask = (1 ^ is_non_zero(condition)).wrapping_sub(1);
+        *self = (*self & mask) | (*value & !mask);
     }
 }
 

cmov/src/portable.rs

@@ -27,62 +27,62 @@ macro_rules! cmov {
 
 impl Cmov for u16 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
-        cmov!("cmovz {1:e}, {2:e}", self, value, condition);
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
+        cmov!("cmovnz {1:e}, {2:e}", self, *value, condition);
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
-        cmov!("cmovnz {1:e}, {2:e}", self, value, condition);
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
+        cmov!("cmovz {1:e}, {2:e}", self, *value, condition);
     }
 }
 
 impl Cmov for u32 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
-        cmov!("cmovz {1:e}, {2:e}", self, value, condition);
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
+        cmov!("cmovnz {1:e}, {2:e}", self, *value, condition);
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
-        cmov!("cmovnz {1:e}, {2:e}", self, value, condition);
-    }
-}
-
-#[cfg(target_arch = "x86_64")]
-impl Cmov for u64 {
-    #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
-        cmov!("cmovz {1:r}, {2:r}", self, value, condition);
-    }
-
-    #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
-        cmov!("cmovnz {1:r}, {2:r}", self, value, condition);
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
+        cmov!("cmovz {1:e}, {2:e}", self, *value, condition);
     }
 }
 
 #[cfg(target_arch = "x86")]
 impl Cmov for u64 {
     #[inline(always)]
-    fn cmovz(&mut self, value: Self, condition: Condition) {
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
         let mut lo = (*self & u32::MAX as u64) as u32;
         let mut hi = (*self >> 32) as u32;
 
-        lo.cmovz((value & u32::MAX as u64) as u32, condition);
-        hi.cmovz((value >> 32) as u32, condition);
+        lo.cmovnz(&((*value & u32::MAX as u64) as u32), condition);
+        hi.cmovnz(&((*value >> 32) as u32), condition);
 
         *self = (lo as u64) | (hi as u64) << 32;
     }
 
     #[inline(always)]
-    fn cmovnz(&mut self, value: Self, condition: Condition) {
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
         let mut lo = (*self & u32::MAX as u64) as u32;
         let mut hi = (*self >> 32) as u32;
 
-        lo.cmovnz((value & u32::MAX as u64) as u32, condition);
-        hi.cmovnz((value >> 32) as u32, condition);
+        lo.cmovz(&((*value & u32::MAX as u64) as u32), condition);
+        hi.cmovz(&((*value >> 32) as u32), condition);
 
         *self = (lo as u64) | (hi as u64) << 32;
     }
 }
+
+#[cfg(target_arch = "x86_64")]
+impl Cmov for u64 {
+    #[inline(always)]
+    fn cmovnz(&mut self, value: &Self, condition: Condition) {
+        cmov!("cmovnz {1:r}, {2:r}", self, *value, condition);
+    }
+
+    #[inline(always)]
+    fn cmovz(&mut self, value: &Self, condition: Condition) {
+        cmov!("cmovz {1:r}, {2:r}", self, *value, condition);
+    }
+}