Skip to content

Commit adeb400

Browse files
Address review comments (added seed for RNG, improved README)
* Add ci job for simd models tests, with logging of rng seed. * Fix testable simd models CI job. * Use LAzyLock instead of lazy_static. * edited readme * revert to working neon intrinsics --------- Co-authored-by: karthikbhargavan <[email protected]>
1 parent 52cd8d9 commit adeb400

File tree

8 files changed

+303
-74
lines changed

8 files changed

+303
-74
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# This workflow runs the tests for testable simd models.
2+
3+
name: Testable simd models
4+
5+
on:
6+
workflow_dispatch:
7+
merge_group:
8+
pull_request:
9+
branches: [ main ]
10+
push:
11+
paths:
12+
- '.github/workflows/testable-simd-models.yml'
13+
- 'testable-simd-models/**'
14+
15+
defaults:
16+
run:
17+
shell: bash
18+
19+
jobs:
20+
testable-simd-models:
21+
name: Test testable simd models
22+
runs-on: ubuntu-latest
23+
24+
steps:
25+
- name: Checkout Repository
26+
uses: actions/checkout@v4
27+
28+
- name: Run tests
29+
working-directory: testable-simd-models
30+
run: cargo test -- --test-threads=1 --nocapture
31+

testable-simd-models/README.md

Lines changed: 99 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,11 @@ pub fn phaddw(a: i16x16, b: i16x16) -> i16x16 {
125125

126126
### Modeling defined intrinsics semi-automatically
127127

128-
To model a defined intrinsic, we essentially copy the Rust code of
129-
the intrinsic from `core::arch` and adapt it to use our underlying abstractions. The
130-
changes needed to the code are sometimes scriptable, and indeed most
131-
of our models were generated from a script, but some changes are still
128+
To model a defined intrinsic, we essentially copy the Rust code of the
129+
intrinsic from `core::arch` and adapt it to use our underlying
130+
abstractions. The changes needed to the code are sometimes
131+
scriptable, and indeed most of our models were generated from a script
132+
(see the ANNEX at the bottom of this file), but some changes are still
132133
needed by hand.
133134

134135
For example, let us say the intrinsic we are modeling is
@@ -176,8 +177,19 @@ pub fn _mm256_bsrli_epi128<const IMM8: i32>(a: __m256i) -> __m256i {
176177
```
177178

178179
Thus, we then go to `core_arch/x86/models/avx2.rs`, and add this implementation.
179-
The only change it requires here is that the `simd_shuffle` macro is a function in our model,
180-
and we discard all the function attributes.
180+
The only changes it requires here are that the `simd_shuffle` macro is a function in our model,
181+
the `ZERO` constant is now a function, and we discard all the function attributes.
182+
183+
The exact diff between the original and edited code for this function is:
184+
185+
```diff
186+
13,14c13,14
187+
< let r: i8x32 = simd_shuffle(
188+
< i8x32::ZERO(),
189+
---
190+
> let r: i8x32 = simd_shuffle!(
191+
> i8x32::ZERO,
192+
```
181193

182194
For other intrinsics, we sometimes need to make more changes. Since our model of the builtin intrinsics
183195
is more precise concerning the type of their arguments compared to their Rust counterparts, we
@@ -224,3 +236,84 @@ us](https://github.com/rust-lang/stdarch/issues/1822) using a failing
224236
test case generated from the testable model and then fixed by [our
225237
PR](https://github.com/rust-lang/stdarch/pull/1823) in the 2025-06-30
226238
version of `stdarch`.
239+
240+
241+
## ANNEX: Extraction Script
242+
243+
The following Rust program is a simple script that uses the `syn` crate to process an input Rust file
244+
containing SIMD intrinsics into one suitable for the models described in this document. This code
245+
is provided as illustration; for each set of core libraries we wish to model and test, there will
246+
likely be need for a similar (or extended) script to automate the modeling process.
247+
248+
```rust
249+
use syn::*;
250+
use std::fs;
251+
use std::env;
252+
253+
fn extract_model(input_file_path: &str, output_file_path: &str) -> Result<()> {
254+
let source_code = fs::read_to_string(input_file_path).expect("unable to read file");
255+
let mut syntax_tree: File = parse_file(&source_code)?;
256+
257+
syntax_tree.items.retain(|item|
258+
match item {
259+
Item::Use(_) => false,
260+
_ => true
261+
}
262+
);
263+
264+
// Clear attributes from the file's top-level items
265+
for item in &mut syntax_tree.items {
266+
match item {
267+
Item::Const(const_item) => {
268+
const_item.attrs.retain(|attr| attr.path().is_ident("doc"));
269+
},
270+
Item::Fn(item_fn) => {
271+
item_fn.attrs.retain(|attr| attr.path().is_ident("doc"));
272+
item_fn.block.stmts.retain(|stmt|
273+
match stmt {
274+
Stmt::Item(Item::ForeignMod(_)) => false,
275+
_ => true
276+
}
277+
);
278+
for stmt in &mut item_fn.block.stmts {
279+
match stmt {
280+
Stmt::Expr(Expr::Unsafe(u), tok) => *stmt = Stmt::Expr(Expr::Block(
281+
ExprBlock {attrs : Vec::new(), label : None, block : u.block.clone()}), *tok),
282+
_ => ()
283+
}
284+
}
285+
},
286+
Item::Struct(item_struct) => {
287+
item_struct.attrs.clear();
288+
for field in &mut item_struct.fields {
289+
field.attrs.retain(|attr| attr.path().is_ident("doc"));
290+
}
291+
},
292+
Item::Enum(item_enum) => {
293+
item_enum.attrs.clear();
294+
for variant in &mut item_enum.variants {
295+
variant.attrs.retain(|attr| attr.path().is_ident("doc"));
296+
}
297+
},
298+
// Add more cases for other Item types (e.g., Item::Mod, Item::Impl, etc.)
299+
_ => {
300+
// For other item types, if they have an 'attrs' field, clear it.
301+
// This requires more specific matching or a helper trait.
302+
}
303+
}
304+
}
305+
306+
let formatted_string = prettyplease::unparse(&syntax_tree);
307+
fs::write(output_file_path, formatted_string).expect("unable to write file");
308+
309+
Ok(())
310+
}
311+
312+
fn main() -> Result<()> {
313+
let args: Vec<String> = env::args().collect();
314+
if args.len() < 3 {
315+
println!("usage: modelize <path to input Rust file> <path to output Rust file>")
316+
}
317+
extract_model(&args[1], &args[2])
318+
}
319+
```

testable-simd-models/src/abstractions/bitvec.rs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,6 @@ impl<const N: u32> BitVec<N> {
106106
.map(int_from_bit_slice)
107107
.collect()
108108
}
109-
110-
/// Generate a random BitVec.
111-
pub fn rand() -> Self {
112-
use rand::prelude::*;
113-
let random_source: Vec<_> = {
114-
let mut rng = rand::rng();
115-
(0..N).map(|_| rng.random::<bool>()).collect()
116-
};
117-
Self::from_fn(|i| random_source[i as usize].into())
118-
}
119109
}
120110

121111
impl<const N: u32> BitVec<N> {

testable-simd-models/src/core_arch/arm_shared/models/neon.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -870,4 +870,4 @@ pub fn vcle_u32(a: uint32x2_t, b: uint32x2_t) -> uint32x2_t {
870870

871871
pub fn vcleq_u32(a: uint32x4_t, b: uint32x4_t) -> uint32x4_t {
872872
simd_le(a, b)
873-
}
873+
}

testable-simd-models/src/core_arch/x86/models/avx2_handwritten.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,6 @@ pub fn psravd(a: i32x4, count: i32x4) -> i32x4 {
419419
}
420420

421421
pub fn psravd256(a: i32x8, count: i32x8) -> i32x8 {
422-
dbg!(a, count);
423422
i32x8::from_fn(|i| {
424423
if count[i] > 31 || count[i] < 0 {
425424
if a[i] < 0 {

testable-simd-models/src/core_arch/x86/tests/avx.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ fn _mm256_movemask_ps() {
4949
let a: BitVec<256> = BitVec::random();
5050
assert_eq!(
5151
super::super::models::avx::_mm256_movemask_ps(a.into()),
52-
unsafe { upstream::_mm256_movemask_ps(a.into()) }
52+
unsafe { upstream::_mm256_movemask_ps(a.into()) },
53+
"Failed with input value: {:?}",
54+
a
5355
);
5456
}
5557
}
@@ -62,7 +64,9 @@ fn _mm256_movemask_pd() {
6264
let a: BitVec<256> = BitVec::random();
6365
assert_eq!(
6466
super::super::models::avx::_mm256_movemask_pd(a.into()),
65-
unsafe { upstream::_mm256_movemask_pd(a.into()) }
67+
unsafe { upstream::_mm256_movemask_pd(a.into()) },
68+
"Failed with input value: {:?}",
69+
a
6670
);
6771
}
6872
}
@@ -76,7 +80,10 @@ fn _mm256_testz_si256() {
7680
let b: BitVec<256> = BitVec::random();
7781
assert_eq!(
7882
super::super::models::avx::_mm256_testz_si256(a.into(), b.into()),
79-
unsafe { upstream::_mm256_testz_si256(a.into(), b.into()) }
83+
unsafe { upstream::_mm256_testz_si256(a.into(), b.into()) },
84+
"Failed with input values: {:?}, {:?}",
85+
a,
86+
b
8087
);
8188
}
8289
}
@@ -90,7 +97,10 @@ fn _mm256_testc_si256() {
9097
let b: BitVec<256> = BitVec::random();
9198
assert_eq!(
9299
super::super::models::avx::_mm256_testc_si256(a.into(), b.into()),
93-
unsafe { upstream::_mm256_testc_si256(a.into(), b.into()) }
100+
unsafe { upstream::_mm256_testc_si256(a.into(), b.into()) },
101+
"Failed with input values: {:?}, {:?}",
102+
a,
103+
b
94104
);
95105
}
96106
}
@@ -116,7 +126,9 @@ fn _mm256_cvtsi256_si32() {
116126
let a: BitVec<256> = BitVec::random();
117127
assert_eq!(
118128
super::super::models::avx::_mm256_cvtsi256_si32(a.into()),
119-
unsafe { upstream::_mm256_cvtsi256_si32(a.into()) }
129+
unsafe { upstream::_mm256_cvtsi256_si32(a.into()) },
130+
"Failed with input value: {:?}",
131+
a
120132
);
121133
}
122134
}

0 commit comments

Comments
 (0)