Skip to content

stdarch-test: various cleanups #1860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+58 −49
Open

stdarch-test: various cleanups #1860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0875002
update `Cargo.lock`
folkertdev Jul 10, 2025
5bf90b6
`stdarch-test`: use `std::sync::LazyLock` instead of `lazy_static!`
folkertdev Jul 10, 2025
b56f7fe
`stdarch-test`: misc cleanups
folkertdev Jul 10, 2025
4f5fc03
`stdarch-test`: error if only part of an instruction matched
folkertdev Jul 10, 2025
ca8a120
s390x: fix tests that matched on prefix of instruction
folkertdev Jul 10, 2025
fb989f8
aarch64/arm: fix tests that matched on prefix of instruction
folkertdev Jul 10, 2025
ded4a23
`stdarch-test`: for now, allow if only part of an instruction matched
folkertdev Jul 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 27 additions & 28 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions crates/core_arch/src/s390x/vector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2254,14 +2254,14 @@ mod sealed {

#[inline]
#[target_feature(enable = "vector")]
#[cfg_attr(test, assert_instr("vlbb"))]
#[cfg_attr(test, assert_instr(vlbb))]
unsafe fn test_vec_load_bndry(ptr: *const i32) -> MaybeUninit<vector_signed_int> {
vector_signed_int::vec_load_bndry::<512>(ptr)
}

#[inline]
#[target_feature(enable = "vector")]
#[cfg_attr(test, assert_instr(vst))]
#[cfg_attr(test, assert_instr(vstl))]
unsafe fn test_vec_store_len(vector: vector_signed_int, ptr: *mut i32, byte_count: u32) {
vector.vec_store_len(ptr, byte_count)
}
Expand Down Expand Up @@ -2787,11 +2787,11 @@ mod sealed {
}

test_impl! { vec_vmal_ib(a: vector_signed_char, b: vector_signed_char, c: vector_signed_char) -> vector_signed_char [simd_mladd, vmalb ] }
test_impl! { vec_vmal_ih(a: vector_signed_short, b: vector_signed_short, c: vector_signed_short) -> vector_signed_short[simd_mladd, vmalh ] }
test_impl! { vec_vmal_ih(a: vector_signed_short, b: vector_signed_short, c: vector_signed_short) -> vector_signed_short[simd_mladd, vmalhw ] }
test_impl! { vec_vmal_if(a: vector_signed_int, b: vector_signed_int, c: vector_signed_int) -> vector_signed_int [simd_mladd, vmalf ] }

test_impl! { vec_vmal_ub(a: vector_unsigned_char, b: vector_unsigned_char, c: vector_unsigned_char) -> vector_unsigned_char [simd_mladd, vmalb ] }
test_impl! { vec_vmal_uh(a: vector_unsigned_short, b: vector_unsigned_short, c: vector_unsigned_short) -> vector_unsigned_short[simd_mladd, vmalh ] }
test_impl! { vec_vmal_uh(a: vector_unsigned_short, b: vector_unsigned_short, c: vector_unsigned_short) -> vector_unsigned_short[simd_mladd, vmalhw ] }
test_impl! { vec_vmal_uf(a: vector_unsigned_int, b: vector_unsigned_int, c: vector_unsigned_int) -> vector_unsigned_int [simd_mladd, vmalf ] }

impl_mul!([VectorMladd vec_mladd] vec_vmal_ib (vector_signed_char, vector_signed_char, vector_signed_char) -> vector_signed_char );
Expand Down
4 changes: 2 additions & 2 deletions crates/stdarch-gen-arm/spec/neon/arm_shared.spec.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14152,8 +14152,8 @@ intrinsics:
- ['vld1q_dup_u32', '*const u32', 'uint32x4_t', 'vld1.32', 'ld1r', 'u32x4::splat']
- ['vld1q_dup_f32', '*const f32', 'float32x4_t', 'vld1.32', 'ld1r', 'f32x4::splat']

- ['vld1q_dup_s64', '*const i64', 'int64x2_t', 'vldr', 'ld1', 'i64x2::splat']
- ['vld1q_dup_u64', '*const u64', 'uint64x2_t', 'vldr', 'ld1', 'u64x2::splat']
- ['vld1q_dup_s64', '*const i64', 'int64x2_t', 'vldr', 'ld1r', 'i64x2::splat']
- ['vld1q_dup_u64', '*const u64', 'uint64x2_t', 'vldr', 'ld1r', 'u64x2::splat']
compose:
- FnCall:
- transmute
Expand Down
1 change: 0 additions & 1 deletion crates/stdarch-test/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ edition = "2024"
[dependencies]
assert-instr-macro = { path = "../assert-instr-macro" }
simd-test-macro = { path = "../simd-test-macro" }
lazy_static = "1.0"
rustc-demangle = "0.1.8"
cfg-if = "1.0"

Expand Down
39 changes: 25 additions & 14 deletions crates/stdarch-test/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@
#![deny(rust_2018_idioms)]
#![allow(clippy::missing_docs_in_private_items, clippy::print_stdout)]

#[macro_use]
extern crate lazy_static;
#[macro_use]
extern crate cfg_if;

pub use assert_instr_macro::*;
pub use simd_test_macro::*;
use std::{cmp, collections::HashSet, env, hash, hint::black_box, str};
use std::{cmp, collections::HashSet, env, hash, hint::black_box, str, sync::LazyLock};

cfg_if! {
if #[cfg(target_arch = "wasm32")] {
Expand All @@ -25,9 +23,7 @@ cfg_if! {
}
}

lazy_static! {
static ref DISASSEMBLY: HashSet<Function> = disassemble_myself();
}
static DISASSEMBLY: LazyLock<HashSet<Function>> = LazyLock::new(disassemble_myself);

#[derive(Debug)]
struct Function {
Expand Down Expand Up @@ -65,11 +61,12 @@ pub fn assert(shim_addr: usize, fnname: &str, expected: &str) {
black_box(shim_addr);

//eprintln!("shim name: {fnname}");
let function = &DISASSEMBLY
.get(&Function::new(fnname))
.unwrap_or_else(|| panic!("function \"{fnname}\" not found in the disassembly"));
let Some(function) = &DISASSEMBLY.get(&Function::new(fnname)) else {
panic!("function `{fnname}` not found in the disassembly")
};
//eprintln!(" function: {:?}", function);

// Trim any filler instructions.
let mut instrs = &function.instrs[..];
while instrs.last().is_some_and(|s| s == "nop" || s == "int3") {
instrs = &instrs[..instrs.len() - 1];
Expand All @@ -84,12 +81,26 @@ pub fn assert(shim_addr: usize, fnname: &str, expected: &str) {
// 2. It is a mark, indicating that the instruction will be
// compiled into other instructions - mainly because of llvm
// optimization.
let expected = if expected == "unknown" {
"<unknown>" // Workaround for rust-lang/stdarch#1674, todo: remove when the issue is fixed
} else {
expected
let expected = match expected {
// `<unknown>` is what LLVM will generate for unknown instructions. We use this to fail
// loudly when LLVM does start supporting these instructions.
//
// This was introduced in https://github.com/rust-lang/stdarch/pull/1674 to work around the
// RISC-V P extension not yet being supported.
"unknown" => "<unknown>",
_ => expected,
};
let found = expected == "nop" || instrs.iter().any(|s| s.starts_with(expected));

// Check whether the given instruction is part of the disassemblied body.
let found = expected == "nop"
|| instrs.iter().any(|instruction| {
instruction.starts_with(expected)
// Check that the next character is non-alphabetic. This prevents false negatives
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect digits (is_ascii_digit) to also be part of the instruction name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is what I had earlier, but it gave lots of hits for ld1 where the test just asserts on ld

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any intrinsics that specifically match on "ld". All the load instrinsics explicitly specify the number since this is an important part of the instruction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm maybe it was a different instruction? In any case it gave a lot of false positives somewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if this included digits and had a way to explicitly opt-in to wildcard matching.

// when e.g. `fminnm` was used but `fmin` was expected.
//
// TODO: resolve the conflicts (x86_64 has a bunch, probably others)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you intending to have this merged as it is, or wait for x86 conflicts to be resolved first? I'm happy to merge either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good to go, we can work through the false positives over time (it's not the most fun task).

// && !instruction[expected.len()..].starts_with(|c: char| c.is_ascii_alphabetic())
});

// Look for subroutine call instructions in the disassembly to detect whether
// inlining failed: all intrinsics are `#[inline(always)]`, so calling one
Expand Down