`intrinsic-test`: Adding x86 behavioural testing. #1894

madhav-madhusoodanan · 2025-08-03T13:31:06Z

Context

This is a redo of PR #1814, since a lot of details have changed with PRs #1863, #1862, #1861, #1852.

crates/intrinsic-test/src/x86/intrinsic.rs

crates/intrinsic-test/src/x86/mod.rs

folkertdev · 2025-08-05T10:36:48Z

crates/intrinsic-test/src/x86/types.rs

+                match str::parse::<u32>(etype_processed.as_str()) {
+                    Ok(value) => data.bit_len = Some(value),
+                    Err(_) => {
+                        data.bit_len = match data.kind() {
+                            TypeKind::Char(_) => Some(8),
+                            TypeKind::BFloat => Some(16),
+                            TypeKind::Int(_) => Some(32),
+                            TypeKind::Float => Some(32),
+                            _ => None,
+                        };
+                    }
+                }


why are only some type kinds covered here? Maybe this could be a method on TypeKind?

crates/intrinsic-test/src/x86/compile.rs

folkertdev

you should rebase on top of the upstream master branch instead of merging it in. That keeps the git history clean.

folkertdev · 2025-08-05T22:48:30Z

ci/run.sh

+    x86_64-unknown-linux-gnu*)
+        CPPFLAGS="${TEST_CPPFLAGS}" RUSTFLAGS="${HOST_RUSTFLAGS}" RUST_LOG=warn \
+            cargo run "${INTRINSIC_TEST}" "${PROFILE}"  \
+            --bin intrinsic-test -- intrinsics_data/x86-intel.xml \
+            --runner "${TEST_RUNNER}" \
+            --cppcompiler "${TEST_CXX_COMPILER}" \
+            --target "${TARGET}"
+        ;;


we'll have to see how to do this exactly, but we want to split these out of the main CI job to speed it up

madhav-madhusoodanan · 2025-09-05T08:19:24Z

Seems like the CI run at this point failed due to this error. I'll retry shortly:

#6 39.45   Could not connect to archive.ubuntu.com:80 (185.125.190.82), connection timed out Could not connect to archive.ubuntu.com:80 (185.125.190.83), connection timed out Could not connect to archive.ubuntu.com:80 (91.189.91.83), connection timed out Could not connect to archive.ubuntu.com:80 (185.125.190.81), connection timed out Could not connect to archive.ubuntu.com:80 (91.189.91.81), connection timed out Could not connect to archive.ubuntu.com:80 (185.125.190.36), connection timed out Could not connect to archive.ubuntu.com:80 (185.125.190.39), connection timed out Could not connect to archive.ubuntu.com:80 (91.189.91.82), connection timed out
#6 39.45   Unable to connect to archive.ubuntu.com:80:
#6 39.45 Fetched 126 kB in 39s (3208 B/s)
#6 39.45 Reading package lists...
#6 39.46 W: Failed to fetch http://archive.ubuntu.com/ubuntu/dists/questing/InRelease  Unable to connect to archive.ubuntu.com:80:
#6 39.46 W: Failed to fetch http://archive.ubuntu.com/ubuntu/dists/questing-updates/InRelease  Unable to connect to archive.ubuntu.com:80:
#6 39.46 W: Failed to fetch http://archive.ubuntu.com/ubuntu/dists/questing-backports/InRelease  Unable to connect to archive.ubuntu.com:80:
#6 39.46 W: Some index files failed to download. They have been ignored, or old ones used instead.

Notes: 1. chunk_info has been moved to `common/mod.rs` since it will be needed for all architectures

struct

folkertdev · 2025-09-14T10:43:54Z

This is looking good code-wise I think, now it just needs to work :)

Running `/intel-sde/sde64 -cpuid-in /checkout/ci/docker/x86_64-unknown-linux-gnu/cpuid.def -rtm-mode full -tsx -- target/release/intrinsic-test intrinsics_data/x86-intel.xml --runner '/intel-sde/sde64             -cpuid-in /checkout/ci/docker/x86_64-unknown-linux-gnu/cpuid.def             -rtm-mode full -tsx --' --cppcompiler clang++-19 --target x86_64-unknown-linux-gnu`

Looks like the command is malformed? Where does that inner --runner come from, that doesn't look right.

madhav-madhusoodanan · 2025-09-14T13:12:51Z

ci/run.sh

+        CPPFLAGS="${TEST_CPPFLAGS}" RUSTFLAGS="${HOST_RUSTFLAGS}" RUST_LOG=warn \
+            cargo run "${INTRINSIC_TEST}" "${PROFILE}"  \
+            --bin intrinsic-test -- intrinsics_data/x86-intel.xml \
+            --runner "${TEST_RUNNER}" \


@folkertdev It seems like the error comes from here, but I'm not sure how the outer runner command comes to be.

ah, right, that comes from

stdarch/ci/docker/x86_64-unknown-linux-gnu/Dockerfile

Lines 14 to 16 in 818f064

ENV CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="/intel-sde/sde64 \

-cpuid-in /checkout/ci/docker/x86_64-unknown-linux-gnu/cpuid.def \

-rtm-mode full -tsx --"

In other words, any cargo run is actually run with that runner. That is useful for the unit tests, but not here where we actually want to run the intrinsic-test binary on the host. On aarch64 etc. we don't configure the runner.

It might work if you add CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="" to this command?

Apparently CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER cannot have a zero size value, so I set it to "true" instead

that'll need a comment at least

I read more on what "true" does.

Apparently, it simply exits successfully and does nothing else. I think this was the reason why the CI test for x86 passed, because it did not run in the first place.

Thank you so much for pointing out this one. I've changed it to be 'bash" instead.

still I think it's useful here to mention that you're resetting the runner, because we want to run this binary directly on the host

Yes, I've mentioned it in a comment right above this highlighted section.

By the way, even bash failed because it cannot execute binary files. I think eval might be a better command for this purpose.

… instead of simd_len for AVX register types)

madhav-madhusoodanan · 2025-09-16T11:01:37Z

Hang on, it isn't supposed to work like this.
Let me check if there are errors that we are not catching.

folkertdev · 2025-09-17T13:01:13Z

crates/intrinsic-test/src/common/argument.rs

-                "{indentation}const {ty} {name}_vals[] = {values};",
+                "{indentation}alignas(64) const {ty} {name}_vals[] = {values};",


interesting, I suppose we're casting values to simd vectors and then performing some sort of aligned read? In any case, can you leave a comment on why the alignment is required?

Really, I added this as a general precautionary measure.

Fortunately when I was working on x86, there were intrinsics that helped with unaligned reads (like _mm_loadu_epi16 and the like), but I'm not sure if the same could be told for other architectures too.

But yes, I'll add a comment on the same.

@folkertdev I wish to confer with you on this. I chose 64 so that it generalizes well across 16-bit, 32-bit and 64-bit alignment requirements.

Would 64 be a good value, or is it overkill?

I'd go with either 64 or remove it completely, any other inbetween value does not really make sense.

crates/intrinsic-test/src/common/gen_rust.rs

folkertdev · 2025-09-17T13:06:57Z

crates/intrinsic-test/src/common/mod.rs

+                    if !output.status.success() {
+                        io::stdout()
+                            .write_all(&output.stdout)
+                            .expect("Failed to write to stdout!");
+                        io::stderr()
+                            .write_all(&output.stderr)
+                            .expect("Failed to write to stderr!");
+                    }


the debug print that is part of the assert should already print the stdoud and stderr data. Is this to get better formatting?

Oh yes, you're right.
I hadn't noticed the debug print of the assert, my bad.

folkertdev · 2025-09-17T13:09:54Z

crates/intrinsic-test/src/arm/mod.rs

+    const PLATFORM_C_HEADERS: &[&str] = &[
+        "iostream",
+        "cstring",
+        "iomanip",
+        "sstream",
+        "cstddef",
+        "cstdint",
+        "arm_neon.h",
+        "arm_acle.h",
+        "arm_fp16.h",
+    ];


why are additional headers needed on arm?

There were a couple of headers that were part of common/gen_c.rs which I had brought here.

Oh, I think cstddef and cstdint won't be necessary in arm though.

sayantn · 2025-09-17T15:35:25Z

@madhav-madhusoodanan there is already a version of the x86-intel.xml file in crates/stdarch-verify, you can just move that file to intrinsics-data (and change the path in the crates/stdarch-verify/tests/x86-intel.rs)

sayantn · 2025-09-17T15:45:43Z

crates/intrinsic-test/src/x86/mod.rs

+            .into_iter()
+            // Not sure how we would compare intrinsic that returns void.
+            .filter(|i| i.results.kind() != TypeKind::Void)
+            .filter(|i| i.results.kind() != TypeKind::BFloat)


This can probably be dropped, x86 stdarch has the bfloat16 newtype that takes the place of C __bfloat16 (at least for now)

…cripts that will be used in CI

neatly organize C++ headers

rustbot assigned folkertdev Aug 3, 2025

madhav-madhusoodanan commented Aug 3, 2025

View reviewed changes

crates/intrinsic-test/src/x86/intrinsic.rs Outdated Show resolved Hide resolved

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch 2 times, most recently from 41db5a8 to 5fc0f3b Compare August 5, 2025 10:16

folkertdev reviewed Aug 5, 2025

View reviewed changes

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from 9e28106 to 111cd5d Compare August 5, 2025 16:22

folkertdev reviewed Aug 5, 2025

View reviewed changes

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from f3f87f2 to 2ec747c Compare August 9, 2025 12:20

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from 2ec747c to a8313d0 Compare September 5, 2025 08:16

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch 9 times, most recently from a119be1 to 0f3900b Compare September 8, 2025 19:25

madhav-madhusoodanan added 11 commits September 10, 2025 00:10

feat: added the skeleton structure of the x86 module

1eee9ed

feat: added the XML intrinsic parser for x86

07f20f4

feat: updated intrinsics creation

191614a

feat: update building C code for x86 architecture.

a952d3b

Notes: 1. chunk_info has been moved to `common/mod.rs` since it will be needed for all architectures

fix: code cleanup

417f729

chore: added Regex crate, updated the structure of X86IntrinsicType

4c19bd2

struct

feat: implemented build_rust_file of x86 module

3e3bf65

feat: implemented compare_outputs of x86 module

3e95708

feat: implement print_result_c for Intrinsic<X86IntrinsicType>

fbb8214

feat: Added x86 to CI pipeline

6c69404

fix: update arch flags being sent to the x86 compilation command

0d62fe4

madhav-madhusoodanan commented Sep 14, 2025

View reviewed changes

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from 0e37477 to 2699a55 Compare September 16, 2025 09:28

madhav-madhusoodanan added 2 commits September 16, 2025 15:00

feat: correcting errors with generated C artifacts

33ead37

fix: vec_len -> simd_len (an error was present due to setting vec_len…

30e0642

… instead of simd_len for AVX register types)

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch 4 times, most recently from eadffb0 to d64935f Compare September 16, 2025 10:17

feat: updating intrinsic-run execution bash script

6f9e90e

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from d64935f to 6f9e90e Compare September 16, 2025 10:43

chore: revert default target

6fbdf07

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from 3f6010e to 6fbdf07 Compare September 16, 2025 11:38

madhav-madhusoodanan marked this pull request as ready for review September 16, 2025 11:56

folkertdev mentioned this pull request Sep 17, 2025

Adding the x86 part of behavioural testing for std::arch intrinsics #1814

Closed

folkertdev reviewed Sep 17, 2025

View reviewed changes

sayantn reviewed Sep 17, 2025

View reviewed changes

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch 3 times, most recently from ff8a8cc to 571eab0 Compare September 17, 2025 17:43

chore: adding comments about memory alignment of variables and bash s…

d3dbbd6

…cripts that will be used in CI

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from 571eab0 to d3dbbd6 Compare September 17, 2025 18:28

chore: adding backtracing for better debugging

adb8124

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from d4f9db8 to adb8124 Compare September 17, 2025 19:24

chore: add compilation flags

218c360

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from 665b698 to 381a439 Compare September 18, 2025 06:59

chore: add better error handling when writing and compiling mod_{i}.cpp,

e32b078

neatly organize C++ headers

madhav-madhusoodanan force-pushed the intrinsic-test-x86-addition branch from 381a439 to e32b078 Compare September 18, 2025 08:11

	ENV CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="/intel-sde/sde64 \
	-cpuid-in /checkout/ci/docker/x86_64-unknown-linux-gnu/cpuid.def \
	-rtm-mode full -tsx --"

		"{indentation}const {ty} {name}_vals[] = {values};",
		"{indentation}alignas(64) const {ty} {name}_vals[] = {values};",

intrinsic-test: Adding x86 behavioural testing. #1894

Are you sure you want to change the base?

intrinsic-test: Adding x86 behavioural testing. #1894

Uh oh!

Conversation

madhav-madhusoodanan commented Aug 3, 2025

Context

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

folkertdev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madhav-madhusoodanan commented Sep 5, 2025

Uh oh!

folkertdev commented Sep 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madhav-madhusoodanan commented Sep 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madhav-madhusoodanan Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sayantn commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

`intrinsic-test`: Adding x86 behavioural testing. #1894

`intrinsic-test`: Adding x86 behavioural testing. #1894

madhav-madhusoodanan Sep 17, 2025 •

edited

Loading