From 98ec734cec6ffc8c66edd2643c052a9734a1e806 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Fri, 10 Aug 2018 20:38:14 +0200 Subject: [PATCH 1/5] Verify code generation of the AVX-512 rotates --- ci/run.sh | 13 ++++-- verify/Cargo.toml | 8 ++++ verify/src/api.rs | 1 + verify/src/api/ops.rs | 1 + verify/src/api/ops/vector_rotates.rs | 70 ++++++++++++++++++++++++++++ verify/src/lib.rs | 10 ++++ 6 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 verify/Cargo.toml create mode 100644 verify/src/api.rs create mode 100644 verify/src/api/ops.rs create mode 100644 verify/src/api/ops/vector_rotates.rs create mode 100644 verify/src/lib.rs diff --git a/ci/run.sh b/ci/run.sh index be93b514b..04411aeff 100755 --- a/ci/run.sh +++ b/ci/run.sh @@ -27,6 +27,10 @@ if [[ ${TARGET} == "x86_64-apple-ios" ]] || [[ ${TARGET} == "i386-apple-ios" ]]; export CARGO_TARGET_I386_APPLE_IOS_RUNNER=$HOME/runtest fi +# The source directory is read-only. Need to copy internal crates to the target +# directory for their Cargo.lock to be properly written. +mkdir target || true + rustc --version cargo --version echo "TARGET=${TARGET}" @@ -61,10 +65,11 @@ cargo_test_impl() { cargo_test_impl cargo_test_impl --release --features=into_bits,coresimd -# Examples - the source directory is read-only. -# Need to copy them to the target directory for the Cargo.lock to be -# properly written. -mkdir target || true +# Verify code generation +if [[ "${NOVERIFY}" != "1" ]]; then + cp -r verify target/verify + cargo_test "--release --manifest-path=target/verify/Cargo.toml" +fi # FIXME: https://github.com/rust-lang-nursery/packed_simd/issues/55 # All examples fail to build for `armv7-apple-ios`. diff --git a/verify/Cargo.toml b/verify/Cargo.toml new file mode 100644 index 000000000..6f7cff9e5 --- /dev/null +++ b/verify/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "verify" +version = "0.1.0" +authors = ["gnzlbg "] + +[dev-dependencies] +stdsimd-test = { git = "https://github.com/rust-lang-nursery/stdsimd.git" } +packed_simd = { path = ".." } \ No newline at end of file diff --git a/verify/src/api.rs b/verify/src/api.rs new file mode 100644 index 000000000..eb2d0e1fa --- /dev/null +++ b/verify/src/api.rs @@ -0,0 +1 @@ +mod ops; diff --git a/verify/src/api/ops.rs b/verify/src/api/ops.rs new file mode 100644 index 000000000..76b33e60c --- /dev/null +++ b/verify/src/api/ops.rs @@ -0,0 +1 @@ +mod vector_rotates; diff --git a/verify/src/api/ops/vector_rotates.rs b/verify/src/api/ops/vector_rotates.rs new file mode 100644 index 000000000..f8878a3e0 --- /dev/null +++ b/verify/src/api/ops/vector_rotates.rs @@ -0,0 +1,70 @@ +mod u64x8 { + use packed_simd::*; + use stdsimd_test::assert_instr; + + #[inline] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + target_feature(enable = "avx512f,avx512vl") + )] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + assert_instr(vpro) + )] + unsafe fn rotate_right_variable(x: u64x8, v: u64x8) -> u64x8 { + x.rotate_right(v) + } + + #[inline] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + target_feature(enable = "avx512f,avx512vl") + )] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + assert_instr(vpro) + )] + unsafe fn rotate_left_variable(x: u64x8, v: u64x8) -> u64x8 { + x.rotate_left(v) + } + + #[inline] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + target_feature(enable = "avx512f") + )] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + assert_instr(vpro) + )] + unsafe fn rotate_right(x: u64x8) -> u64x8 { + x.rotate_right(u64x8::splat(12)) + } + + #[inline] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + target_feature(enable = "avx512f") + )] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + assert_instr(vpro) + )] + unsafe fn rotate_left(x: u64x8) -> u64x8 { + x.rotate_left(u64x8::splat(12)) + } + + #[inline] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + target_feature(enable = "avx512f") + )] + #[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + assert_instr(vpro) + )] + unsafe fn rotate_left_x2(x: u64x2) -> u64x2 { + x.rotate_left(u64x2::splat(12)) + } + +} diff --git a/verify/src/lib.rs b/verify/src/lib.rs new file mode 100644 index 000000000..af08396c0 --- /dev/null +++ b/verify/src/lib.rs @@ -0,0 +1,10 @@ +#![feature(rust_2018_preview, use_extern_macros, avx512_target_feature)] + +#[cfg(test)] +extern crate packed_simd; + +#[cfg(test)] +extern crate stdsimd_test; + +#[cfg(test)] +mod api; From f5f838a3c7f23461801351a8f323ce26594f6f1f Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Fri, 10 Aug 2018 20:52:14 +0200 Subject: [PATCH 2/5] document verification --- readme.md | 8 ++++++++ verify/readme.md | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 verify/readme.md diff --git a/readme.md b/readme.md index 523e3532e..7b86c12c6 100644 --- a/readme.md +++ b/readme.md @@ -91,6 +91,14 @@ there are correctness bugs open in the issue tracker. [**] it is currently not easily possible to run these platforms on CI. +# Machine code verification + +The +[`verify/`](https://github.com/rust-lang-nursery/packed_simd/tree/master/verify) +crate tests disassembles the portable packed vector APIs at run-time and +compares the generated machine code against the desired one to make sure that +this crate remains efficient. + # License This project is licensed under either of diff --git a/verify/readme.md b/verify/readme.md new file mode 100644 index 000000000..b3e6f5ed5 --- /dev/null +++ b/verify/readme.md @@ -0,0 +1,36 @@ +# Machine code verification + +This crates verifies the machine code generated for some of the portable packed +vector APIs by disassembling the API at run-time and comparing the machine code +generated against the desired one for a particular target and target features. + +This is done by using the +[`stdsimd-test`](https://github.com/rust-lang-nursery/stdsimd/tree/master/crates/stdsimd-test) +crate, which exposes the `assert_instr` procedural macro. It is used like this: + +```rust +// The verification functions must be #[inline]: +#[inline] +// Enable the target features required for the desired code generation +// on the different targets: +#[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + target_feature(enable = "avx512f,avx512vl") +)] +// Check that the disassembly contains a particular instruction: +#[cfg_attr( + any(target_arch = "x86", target_arch = "x86_64"), + assert_instr(vpro) +)] +unsafe fn rotate_right_variable(x: u64x8, v: u64x8) -> u64x8 { + x.rotate_right(v) +} +``` + +The `assert_instr` procedural macro creates a test that contains a +`#[inline(never)]` function that calls the API. It then gets a function pointer +to this function, and calls `stdsimd_test::assert` with it, the function name, +and the expected assembly instruction. `stdsimd_test` uses `objdump` or similar +to disassemble itself, it then looks for the function address and name in the +disassembly, and verifies that the machine code for the function contains the +instruction. From 22ccf1c19832c2d8ece5ea97c79ccf56b7fe1fd8 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Fri, 10 Aug 2018 20:54:37 +0200 Subject: [PATCH 3/5] explain how to run the verification tests --- verify/readme.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/verify/readme.md b/verify/readme.md index b3e6f5ed5..a04433e22 100644 --- a/verify/readme.md +++ b/verify/readme.md @@ -1,5 +1,22 @@ # Machine code verification +## Quick start + +To run the verification tests run: + +``` +cargo test --release +``` + +on this crate, eventually passing the required target features via `RUSTFLAGS`. +For example, `RUSTFLAGS="-C target-feature=+avx2"`. + +This crate only contains tests, and the tests only run in `--release` mode. +Therefore, building this crate with anything different from `cargo test +--release` does not make much sense. + +## How it works + This crates verifies the machine code generated for some of the portable packed vector APIs by disassembling the API at run-time and comparing the machine code generated against the desired one for a particular target and target features. From 67577c7c703f7dd8e59acbbf230644b2f5d7252a Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sun, 12 Aug 2018 13:29:41 +0200 Subject: [PATCH 4/5] pass cli arguments as a separated list instead of string --- ci/run.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/run.sh b/ci/run.sh index 04411aeff..ce5f2b706 100755 --- a/ci/run.sh +++ b/ci/run.sh @@ -68,7 +68,7 @@ cargo_test_impl --release --features=into_bits,coresimd # Verify code generation if [[ "${NOVERIFY}" != "1" ]]; then cp -r verify target/verify - cargo_test "--release --manifest-path=target/verify/Cargo.toml" + cargo_test --release --manifest-path=target/verify/Cargo.toml fi # FIXME: https://github.com/rust-lang-nursery/packed_simd/issues/55 From 6ba5f73fc8852b150b8c0a07d0e70b673cac241c Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sun, 12 Aug 2018 14:52:21 +0200 Subject: [PATCH 5/5] update verify path so that it works from within target/ --- ci/run.sh | 2 +- verify/{ => verify}/Cargo.toml | 2 +- verify/{ => verify}/readme.md | 0 verify/{ => verify}/src/api.rs | 0 verify/{ => verify}/src/api/ops.rs | 0 verify/{ => verify}/src/api/ops/vector_rotates.rs | 0 verify/{ => verify}/src/lib.rs | 0 7 files changed, 2 insertions(+), 2 deletions(-) rename verify/{ => verify}/Cargo.toml (84%) rename verify/{ => verify}/readme.md (100%) rename verify/{ => verify}/src/api.rs (100%) rename verify/{ => verify}/src/api/ops.rs (100%) rename verify/{ => verify}/src/api/ops/vector_rotates.rs (100%) rename verify/{ => verify}/src/lib.rs (100%) diff --git a/ci/run.sh b/ci/run.sh index ce5f2b706..293511ef2 100755 --- a/ci/run.sh +++ b/ci/run.sh @@ -67,7 +67,7 @@ cargo_test_impl --release --features=into_bits,coresimd # Verify code generation if [[ "${NOVERIFY}" != "1" ]]; then - cp -r verify target/verify + cp -r verify/verify target/verify cargo_test --release --manifest-path=target/verify/Cargo.toml fi diff --git a/verify/Cargo.toml b/verify/verify/Cargo.toml similarity index 84% rename from verify/Cargo.toml rename to verify/verify/Cargo.toml index 6f7cff9e5..a4f1c59b1 100644 --- a/verify/Cargo.toml +++ b/verify/verify/Cargo.toml @@ -5,4 +5,4 @@ authors = ["gnzlbg "] [dev-dependencies] stdsimd-test = { git = "https://github.com/rust-lang-nursery/stdsimd.git" } -packed_simd = { path = ".." } \ No newline at end of file +packed_simd = { path = "../.." } \ No newline at end of file diff --git a/verify/readme.md b/verify/verify/readme.md similarity index 100% rename from verify/readme.md rename to verify/verify/readme.md diff --git a/verify/src/api.rs b/verify/verify/src/api.rs similarity index 100% rename from verify/src/api.rs rename to verify/verify/src/api.rs diff --git a/verify/src/api/ops.rs b/verify/verify/src/api/ops.rs similarity index 100% rename from verify/src/api/ops.rs rename to verify/verify/src/api/ops.rs diff --git a/verify/src/api/ops/vector_rotates.rs b/verify/verify/src/api/ops/vector_rotates.rs similarity index 100% rename from verify/src/api/ops/vector_rotates.rs rename to verify/verify/src/api/ops/vector_rotates.rs diff --git a/verify/src/lib.rs b/verify/verify/src/lib.rs similarity index 100% rename from verify/src/lib.rs rename to verify/verify/src/lib.rs