Enable verification of more intrinsics (rust-lang#309)

Looks like intrinsics that weren't listing a target feature were accidentally
omitted from the verification logic, so this commit fixes that!

Along the way I've ended up filing rust-lang#307 and rust-lang#308 for detected inconsistencies.

coresimd/src/x86/i586/bswap.rs

@@ -12,17 +12,8 @@ pub unsafe fn _bswap(x: i32) -> i32 {
     bswap_i32(x)
 }
 
-/// Return an integer with the reversed byte order of x
-#[inline]
-#[cfg_attr(test, assert_instr(bswap))]
-pub unsafe fn _bswap64(x: i64) -> i64 {
-    bswap_i64(x)
-}
-
 #[allow(improper_ctypes)]
 extern "C" {
-    #[link_name = "llvm.bswap.i64"]
-    fn bswap_i64(x: i64) -> i64;
     #[link_name = "llvm.bswap.i32"]
     fn bswap_i32(x: i32) -> i32;
 }
@@ -38,12 +29,4 @@ mod tests {
             assert_eq!(_bswap(0x00000000), 0x00000000);
         }
     }
-
-    #[test]
-    fn test_bswap64() {
-        unsafe {
-            assert_eq!(_bswap64(0x0EADBEEFFADECA0E), 0x0ECADEFAEFBEAD0E);
-            assert_eq!(_bswap64(0x0000000000000000), 0x0000000000000000);
-        }
-    }
 }

coresimd/src/x86/i586/rdtsc.rs

@@ -17,7 +17,7 @@ use stdsimd_test::assert_instr;
 /// high-order 32 bits of each of RAX and RDX are cleared.
 #[inline]
 #[cfg_attr(test, assert_instr(rdtsc))]
-pub unsafe fn _rdtsc() -> u64 {
+pub unsafe fn _rdtsc() -> i64 {
     rdtsc()
 }
 
@@ -37,14 +37,14 @@ pub unsafe fn _rdtsc() -> u64 {
 /// high-order 32 bits of each of RAX, RDX, and RCX are cleared.
 #[inline]
 #[cfg_attr(test, assert_instr(rdtscp))]
-pub unsafe fn _rdtscp(aux: *mut u32) -> u64 {
+pub unsafe fn __rdtscp(aux: *mut u32) -> u64 {
     rdtscp(aux as *mut _)
 }
 
 #[allow(improper_ctypes)]
 extern "C" {
     #[link_name = "llvm.x86.rdtsc"]
-    fn rdtsc() -> u64;
+    fn rdtsc() -> i64;
     #[link_name = "llvm.x86.rdtscp"]
     fn rdtscp(aux: *mut u8) -> u64;
 }
@@ -63,7 +63,7 @@ mod tests {
     #[simd_test = "sse2"]
     unsafe fn _rdtscp() {
         let mut aux = 0;
-        let r = rdtsc::_rdtscp(&mut aux);
+        let r = rdtsc::__rdtscp(&mut aux);
         assert_ne!(r, 0); // The chances of this being 0 are infinitesimal
     }
 }

coresimd/src/x86/x86_64/bswap.rs

@@ -0,0 +1,32 @@
+//! Byte swap intrinsics.
+
+#![cfg_attr(feature = "cargo-clippy", allow(stutter))]
+
+#[cfg(test)]
+use stdsimd_test::assert_instr;
+
+/// Return an integer with the reversed byte order of x
+#[inline]
+#[cfg_attr(test, assert_instr(bswap))]
+pub unsafe fn _bswap64(x: i64) -> i64 {
+    bswap_i64(x)
+}
+
+#[allow(improper_ctypes)]
+extern "C" {
+    #[link_name = "llvm.bswap.i64"]
+    fn bswap_i64(x: i64) -> i64;
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_bswap64() {
+        unsafe {
+            assert_eq!(_bswap64(0x0EADBEEFFADECA0E), 0x0ECADEFAEFBEAD0E);
+            assert_eq!(_bswap64(0x0000000000000000), 0x0000000000000000);
+        }
+    }
+}

coresimd/src/x86/x86_64/mod.rs

@@ -34,3 +34,6 @@ pub use self::bmi2::*;
 
 mod avx2;
 pub use self::avx2::*;
+
+mod bswap;
+pub use self::bswap::*;

stdsimd-verify/src/lib.rs

@@ -46,13 +46,7 @@ pub fn x86_functions(input: TokenStream) -> TokenStream {
         if f.unsafety.is_none() {
             return false;
         }
-        f.attrs
-            .iter()
-            .filter_map(|a| a.interpret_meta())
-            .any(|a| match a {
-                syn::Meta::List(i) => i.ident == "target_feature",
-                _ => false,
-            })
+        true
     });
     assert!(functions.len() > 0);
 
@@ -79,7 +73,10 @@ pub fn x86_functions(input: TokenStream) -> TokenStream {
                 }
             };
             let instrs = find_instrs(&f.attrs);
-            let target_feature = find_target_feature(f.ident, &f.attrs);
+            let target_feature = match find_target_feature(&f.attrs) {
+                Some(i) => my_quote! { Some(#i) },
+                None => my_quote! { None },
+            };
             my_quote! {
                 Function {
                     name: stringify!(#name),
@@ -119,6 +116,7 @@ fn to_type(t: &syn::Type) -> Tokens {
             "u32" => my_quote! { &U32 },
             "u64" => my_quote! { &U64 },
             "u8" => my_quote! { &U8 },
+            "CpuidResult" => my_quote! { &CPUID },
             s => panic!("unspported type: {}", s),
         },
         syn::Type::Ptr(syn::TypePtr { ref elem, .. })
@@ -128,7 +126,7 @@ fn to_type(t: &syn::Type) -> Tokens {
         }
         syn::Type::Slice(_) => panic!("unsupported slice"),
         syn::Type::Array(_) => panic!("unsupported array"),
-        syn::Type::Tuple(_) => panic!("unsupported tup"),
+        syn::Type::Tuple(_) => my_quote! { &TUPLE },
         _ => panic!("unsupported type"),
     }
 }
@@ -207,9 +205,7 @@ fn find_instrs(attrs: &[syn::Attribute]) -> Vec<syn::Ident> {
         .collect()
 }
 
-fn find_target_feature(
-    name: syn::Ident, attrs: &[syn::Attribute]
-) -> syn::Lit {
+fn find_target_feature(attrs: &[syn::Attribute]) -> Option<syn::Lit> {
     attrs
         .iter()
         .filter_map(|a| a.interpret_meta())
@@ -243,5 +239,4 @@ fn find_target_feature(
             }
         })
         .next()
-        .expect(&format!("failed to find target_feature for {}", name))
 }

stdsimd-verify/tests/x86-intel.rs

@@ -16,7 +16,7 @@ struct Function {
     name: &'static str,
     arguments: &'static [&'static Type],
     ret: Option<&'static Type>,
-    target_feature: &'static str,
+    target_feature: Option<&'static str>,
     instrs: &'static [&'static str],
     file: &'static str,
 }
@@ -41,6 +41,9 @@ static M256: Type = Type::M256;
 static M256I: Type = Type::M256I;
 static M256D: Type = Type::M256D;
 
+static TUPLE: Type = Type::Tuple;
+static CPUID: Type = Type::CpuidResult;
+
 #[derive(Debug)]
 enum Type {
     PrimFloat(u8),
@@ -55,6 +58,8 @@ enum Type {
     M256D,
     M256I,
     Bool,
+    Tuple,
+    CpuidResult,
 }
 
 x86_functions!(static FUNCTIONS);
@@ -84,6 +89,23 @@ struct Instruction {
     name: String,
 }
 
+fn skip_intrinsic(name: &str) -> bool {
+    match name {
+        // This intrinsic has multiple definitions in the XML, so just
+        // ignore it.
+        "_mm_prefetch" => true,
+
+        // FIXME(#307)
+        "__readeflags" |
+        "__writeeflags" => true,
+        "__cpuid_count" => true,
+        "__cpuid" => true,
+        "__get_cpuid_max" => true,
+
+        _ => false,
+    }
+}
+
 #[test]
 fn verify_all_signatures() {
     // This XML document was downloaded from Intel's site. To update this you
@@ -101,10 +123,8 @@ fn verify_all_signatures() {
         serde_xml_rs::deserialize(xml).expect("failed to deserialize xml");
     let mut map = HashMap::new();
     for intrinsic in &data.intrinsics {
-        // This intrinsic has multiple definitions in the XML, so just ignore
-        // it.
-        if intrinsic.name == "_mm_prefetch" {
-            continue;
+        if skip_intrinsic(&intrinsic.name) {
+            continue
         }
 
         // These'll need to get added eventually, but right now they have some
@@ -117,16 +137,15 @@ fn verify_all_signatures() {
     }
 
     for rust in FUNCTIONS {
-        // This was ignored above, we ignore it here as well.
-        if rust.name == "_mm_prefetch" {
+        if skip_intrinsic(&rust.name) {
             continue;
         }
 
         // these are all AMD-specific intrinsics
-        if rust.target_feature.contains("sse4a")
-            || rust.target_feature.contains("tbm")
-        {
-            continue;
+        if let Some(feature) = rust.target_feature {
+            if feature.contains("sse4a") || feature.contains("tbm") {
+                continue;
+            }
         }
 
         let intel = match map.get(rust.name) {
@@ -137,14 +156,25 @@ fn verify_all_signatures() {
         // Verify that all `#[target_feature]` annotations are correct,
         // ensuring that we've actually enabled the right instruction
         // set for this intrinsic.
-        assert!(!intel.cpuid.is_empty(), "missing cpuid for {}", rust.name);
+        match rust.name {
+            "_bswap" => {}
+            "_bswap64" => {}
+            _ => {
+                assert!(!intel.cpuid.is_empty(), "missing cpuid for {}", rust.name);
+            }
+        }
         for cpuid in &intel.cpuid {
             // this is needed by _xsave and probably some related intrinsics,
             // but let's just skip it for now.
             if *cpuid == "XSS" {
                 continue;
             }
 
+            // FIXME(#308)
+            if *cpuid == "TSC" || *cpuid == "RDTSCP" {
+                continue;
+            }
+
             let cpuid = cpuid
                 .chars()
                 .flat_map(|c| c.to_lowercase())
@@ -158,11 +188,13 @@ fn verify_all_signatures() {
                 cpuid
             };
 
+            let rust_feature = rust.target_feature
+                .expect(&format!("no target feature listed for {}", rust.name));
             assert!(
-                rust.target_feature.contains(&cpuid),
+                rust_feature.contains(&cpuid),
                 "intel cpuid `{}` not in `{}` for {}",
                 cpuid,
-                rust.target_feature,
+                rust_feature,
                 rust.name
             );
         }
@@ -228,8 +260,6 @@ fn verify_all_signatures() {
                 match *arg {
                     Type::PrimSigned(64) |
                     Type::PrimUnsigned(64) => true,
-                    // Type::Ptr(&Type::PrimSigned(64)) |
-                    // Type::Ptr(&Type::PrimUnsigned(64)) => true,
                     _ => false,
                 }
             });
@@ -254,6 +284,10 @@ fn verify_all_signatures() {
             "_mm256_setr_epi64x" |
             "_mm256_set1_epi64x" => true,
 
+            // FIXME(#308)
+            "_rdtsc" |
+            "__rdtscp" => true,
+
             _ => false,
         };
         if any_i64 && !any_i64_exempt {