Skip to content

Commit b68f729

Browse files
authored
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.
1 parent e38d5ac commit b68f729

File tree

6 files changed

+97
-50
lines changed

6 files changed

+97
-50
lines changed

coresimd/src/x86/i586/bswap.rs

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,8 @@ pub unsafe fn _bswap(x: i32) -> i32 {
1212
bswap_i32(x)
1313
}
1414

15-
/// Return an integer with the reversed byte order of x
16-
#[inline]
17-
#[cfg_attr(test, assert_instr(bswap))]
18-
pub unsafe fn _bswap64(x: i64) -> i64 {
19-
bswap_i64(x)
20-
}
21-
2215
#[allow(improper_ctypes)]
2316
extern "C" {
24-
#[link_name = "llvm.bswap.i64"]
25-
fn bswap_i64(x: i64) -> i64;
2617
#[link_name = "llvm.bswap.i32"]
2718
fn bswap_i32(x: i32) -> i32;
2819
}
@@ -38,12 +29,4 @@ mod tests {
3829
assert_eq!(_bswap(0x00000000), 0x00000000);
3930
}
4031
}
41-
42-
#[test]
43-
fn test_bswap64() {
44-
unsafe {
45-
assert_eq!(_bswap64(0x0EADBEEFFADECA0E), 0x0ECADEFAEFBEAD0E);
46-
assert_eq!(_bswap64(0x0000000000000000), 0x0000000000000000);
47-
}
48-
}
4932
}

coresimd/src/x86/i586/rdtsc.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use stdsimd_test::assert_instr;
1717
/// high-order 32 bits of each of RAX and RDX are cleared.
1818
#[inline]
1919
#[cfg_attr(test, assert_instr(rdtsc))]
20-
pub unsafe fn _rdtsc() -> u64 {
20+
pub unsafe fn _rdtsc() -> i64 {
2121
rdtsc()
2222
}
2323

@@ -37,14 +37,14 @@ pub unsafe fn _rdtsc() -> u64 {
3737
/// high-order 32 bits of each of RAX, RDX, and RCX are cleared.
3838
#[inline]
3939
#[cfg_attr(test, assert_instr(rdtscp))]
40-
pub unsafe fn _rdtscp(aux: *mut u32) -> u64 {
40+
pub unsafe fn __rdtscp(aux: *mut u32) -> u64 {
4141
rdtscp(aux as *mut _)
4242
}
4343

4444
#[allow(improper_ctypes)]
4545
extern "C" {
4646
#[link_name = "llvm.x86.rdtsc"]
47-
fn rdtsc() -> u64;
47+
fn rdtsc() -> i64;
4848
#[link_name = "llvm.x86.rdtscp"]
4949
fn rdtscp(aux: *mut u8) -> u64;
5050
}
@@ -63,7 +63,7 @@ mod tests {
6363
#[simd_test = "sse2"]
6464
unsafe fn _rdtscp() {
6565
let mut aux = 0;
66-
let r = rdtsc::_rdtscp(&mut aux);
66+
let r = rdtsc::__rdtscp(&mut aux);
6767
assert_ne!(r, 0); // The chances of this being 0 are infinitesimal
6868
}
6969
}

coresimd/src/x86/x86_64/bswap.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//! Byte swap intrinsics.
2+
3+
#![cfg_attr(feature = "cargo-clippy", allow(stutter))]
4+
5+
#[cfg(test)]
6+
use stdsimd_test::assert_instr;
7+
8+
/// Return an integer with the reversed byte order of x
9+
#[inline]
10+
#[cfg_attr(test, assert_instr(bswap))]
11+
pub unsafe fn _bswap64(x: i64) -> i64 {
12+
bswap_i64(x)
13+
}
14+
15+
#[allow(improper_ctypes)]
16+
extern "C" {
17+
#[link_name = "llvm.bswap.i64"]
18+
fn bswap_i64(x: i64) -> i64;
19+
}
20+
21+
#[cfg(test)]
22+
mod tests {
23+
use super::*;
24+
25+
#[test]
26+
fn test_bswap64() {
27+
unsafe {
28+
assert_eq!(_bswap64(0x0EADBEEFFADECA0E), 0x0ECADEFAEFBEAD0E);
29+
assert_eq!(_bswap64(0x0000000000000000), 0x0000000000000000);
30+
}
31+
}
32+
}

coresimd/src/x86/x86_64/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,6 @@ pub use self::bmi2::*;
3434

3535
mod avx2;
3636
pub use self::avx2::*;
37+
38+
mod bswap;
39+
pub use self::bswap::*;

stdsimd-verify/src/lib.rs

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,7 @@ pub fn x86_functions(input: TokenStream) -> TokenStream {
4646
if f.unsafety.is_none() {
4747
return false;
4848
}
49-
f.attrs
50-
.iter()
51-
.filter_map(|a| a.interpret_meta())
52-
.any(|a| match a {
53-
syn::Meta::List(i) => i.ident == "target_feature",
54-
_ => false,
55-
})
49+
true
5650
});
5751
assert!(functions.len() > 0);
5852

@@ -79,7 +73,10 @@ pub fn x86_functions(input: TokenStream) -> TokenStream {
7973
}
8074
};
8175
let instrs = find_instrs(&f.attrs);
82-
let target_feature = find_target_feature(f.ident, &f.attrs);
76+
let target_feature = match find_target_feature(&f.attrs) {
77+
Some(i) => my_quote! { Some(#i) },
78+
None => my_quote! { None },
79+
};
8380
my_quote! {
8481
Function {
8582
name: stringify!(#name),
@@ -119,6 +116,7 @@ fn to_type(t: &syn::Type) -> Tokens {
119116
"u32" => my_quote! { &U32 },
120117
"u64" => my_quote! { &U64 },
121118
"u8" => my_quote! { &U8 },
119+
"CpuidResult" => my_quote! { &CPUID },
122120
s => panic!("unspported type: {}", s),
123121
},
124122
syn::Type::Ptr(syn::TypePtr { ref elem, .. })
@@ -128,7 +126,7 @@ fn to_type(t: &syn::Type) -> Tokens {
128126
}
129127
syn::Type::Slice(_) => panic!("unsupported slice"),
130128
syn::Type::Array(_) => panic!("unsupported array"),
131-
syn::Type::Tuple(_) => panic!("unsupported tup"),
129+
syn::Type::Tuple(_) => my_quote! { &TUPLE },
132130
_ => panic!("unsupported type"),
133131
}
134132
}
@@ -207,9 +205,7 @@ fn find_instrs(attrs: &[syn::Attribute]) -> Vec<syn::Ident> {
207205
.collect()
208206
}
209207

210-
fn find_target_feature(
211-
name: syn::Ident, attrs: &[syn::Attribute]
212-
) -> syn::Lit {
208+
fn find_target_feature(attrs: &[syn::Attribute]) -> Option<syn::Lit> {
213209
attrs
214210
.iter()
215211
.filter_map(|a| a.interpret_meta())
@@ -243,5 +239,4 @@ fn find_target_feature(
243239
}
244240
})
245241
.next()
246-
.expect(&format!("failed to find target_feature for {}", name))
247242
}

stdsimd-verify/tests/x86-intel.rs

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ struct Function {
1616
name: &'static str,
1717
arguments: &'static [&'static Type],
1818
ret: Option<&'static Type>,
19-
target_feature: &'static str,
19+
target_feature: Option<&'static str>,
2020
instrs: &'static [&'static str],
2121
file: &'static str,
2222
}
@@ -41,6 +41,9 @@ static M256: Type = Type::M256;
4141
static M256I: Type = Type::M256I;
4242
static M256D: Type = Type::M256D;
4343

44+
static TUPLE: Type = Type::Tuple;
45+
static CPUID: Type = Type::CpuidResult;
46+
4447
#[derive(Debug)]
4548
enum Type {
4649
PrimFloat(u8),
@@ -55,6 +58,8 @@ enum Type {
5558
M256D,
5659
M256I,
5760
Bool,
61+
Tuple,
62+
CpuidResult,
5863
}
5964

6065
x86_functions!(static FUNCTIONS);
@@ -84,6 +89,23 @@ struct Instruction {
8489
name: String,
8590
}
8691

92+
fn skip_intrinsic(name: &str) -> bool {
93+
match name {
94+
// This intrinsic has multiple definitions in the XML, so just
95+
// ignore it.
96+
"_mm_prefetch" => true,
97+
98+
// FIXME(#307)
99+
"__readeflags" |
100+
"__writeeflags" => true,
101+
"__cpuid_count" => true,
102+
"__cpuid" => true,
103+
"__get_cpuid_max" => true,
104+
105+
_ => false,
106+
}
107+
}
108+
87109
#[test]
88110
fn verify_all_signatures() {
89111
// This XML document was downloaded from Intel's site. To update this you
@@ -101,10 +123,8 @@ fn verify_all_signatures() {
101123
serde_xml_rs::deserialize(xml).expect("failed to deserialize xml");
102124
let mut map = HashMap::new();
103125
for intrinsic in &data.intrinsics {
104-
// This intrinsic has multiple definitions in the XML, so just ignore
105-
// it.
106-
if intrinsic.name == "_mm_prefetch" {
107-
continue;
126+
if skip_intrinsic(&intrinsic.name) {
127+
continue
108128
}
109129

110130
// These'll need to get added eventually, but right now they have some
@@ -117,16 +137,15 @@ fn verify_all_signatures() {
117137
}
118138

119139
for rust in FUNCTIONS {
120-
// This was ignored above, we ignore it here as well.
121-
if rust.name == "_mm_prefetch" {
140+
if skip_intrinsic(&rust.name) {
122141
continue;
123142
}
124143

125144
// these are all AMD-specific intrinsics
126-
if rust.target_feature.contains("sse4a")
127-
|| rust.target_feature.contains("tbm")
128-
{
129-
continue;
145+
if let Some(feature) = rust.target_feature {
146+
if feature.contains("sse4a") || feature.contains("tbm") {
147+
continue;
148+
}
130149
}
131150

132151
let intel = match map.get(rust.name) {
@@ -137,14 +156,25 @@ fn verify_all_signatures() {
137156
// Verify that all `#[target_feature]` annotations are correct,
138157
// ensuring that we've actually enabled the right instruction
139158
// set for this intrinsic.
140-
assert!(!intel.cpuid.is_empty(), "missing cpuid for {}", rust.name);
159+
match rust.name {
160+
"_bswap" => {}
161+
"_bswap64" => {}
162+
_ => {
163+
assert!(!intel.cpuid.is_empty(), "missing cpuid for {}", rust.name);
164+
}
165+
}
141166
for cpuid in &intel.cpuid {
142167
// this is needed by _xsave and probably some related intrinsics,
143168
// but let's just skip it for now.
144169
if *cpuid == "XSS" {
145170
continue;
146171
}
147172

173+
// FIXME(#308)
174+
if *cpuid == "TSC" || *cpuid == "RDTSCP" {
175+
continue;
176+
}
177+
148178
let cpuid = cpuid
149179
.chars()
150180
.flat_map(|c| c.to_lowercase())
@@ -158,11 +188,13 @@ fn verify_all_signatures() {
158188
cpuid
159189
};
160190

191+
let rust_feature = rust.target_feature
192+
.expect(&format!("no target feature listed for {}", rust.name));
161193
assert!(
162-
rust.target_feature.contains(&cpuid),
194+
rust_feature.contains(&cpuid),
163195
"intel cpuid `{}` not in `{}` for {}",
164196
cpuid,
165-
rust.target_feature,
197+
rust_feature,
166198
rust.name
167199
);
168200
}
@@ -228,8 +260,6 @@ fn verify_all_signatures() {
228260
match *arg {
229261
Type::PrimSigned(64) |
230262
Type::PrimUnsigned(64) => true,
231-
// Type::Ptr(&Type::PrimSigned(64)) |
232-
// Type::Ptr(&Type::PrimUnsigned(64)) => true,
233263
_ => false,
234264
}
235265
});
@@ -254,6 +284,10 @@ fn verify_all_signatures() {
254284
"_mm256_setr_epi64x" |
255285
"_mm256_set1_epi64x" => true,
256286

287+
// FIXME(#308)
288+
"_rdtsc" |
289+
"__rdtscp" => true,
290+
257291
_ => false,
258292
};
259293
if any_i64 && !any_i64_exempt {

0 commit comments

Comments
 (0)