Refactor cpuid by AntoinePrv · Pull Request #1251 · xtensor-stack/xsimd

AntoinePrv · 2026-01-30T11:31:35Z

This is a no-addition, non-breaking refactor of existing CPU id features as first class citizen as part of #1245.

Two individual and simple classes parsing for XCR0 and CPUID
The classes/header is safe on all platform, avoiding
supported_arch keeps the same structure and merges use of both class at the moment but both class could be combined a user-friendly x86_cpu_features.

serge-sans-paille

Some early review, a few open questions but nothing looks bad here. Way to go!

serge-sans-paille · 2026-01-30T16:20:36Z

include/xsimd/xsimd_cpu_features_x86.hpp

+    {
+        inline void get_cpuid(int reg[4], int level, int count = 0) noexcept;
+
+        inline std::uint32_t get_xcr0_low() noexcept;


it would be good to somehow ensure that this type is the same as ref_t

And this could be a private static method for better encapsulation right?

include/xsimd/xsimd_cpu_features_x86.hpp

include/xsimd/utils/bits.hpp

serge-sans-paille · 2026-01-30T16:25:27Z

include/xsimd/xsimd_cpu_features_x86.hpp

+        constexpr bool fma4() const noexcept
+        {
+            return utils::bit_is_set<16>(m_regs.reg8[2]);
+        }


Open question: should we have a macro that makes the generation of those field cleaner to the eye?

Either way. I am personally not big on macros. I find it easier read this way (could also be made to fit on one line) and find&replace is quite enough a change is required.

serge-sans-paille · 2026-01-30T16:27:14Z

include/xsimd/xsimd_cpu_features_x86.hpp

+
+    namespace detail
+    {
+        inline void get_cpuid(int reg[4], int level, int count) noexcept


Same here: could be private for better encapsulation.

On the contrary, I was wondering whether to make it part of public API.
It's a well defined function, unlikely to change, and that could be useful for users to get features we do not expose (not sure we'll go to cover 100% of cpuid).

serge-sans-paille · 2026-01-30T16:27:40Z

include/xsimd/types/xsimd_all_registers.hpp

 * The full license is in the file LICENSE, distributed with this software. *
 ****************************************************************************/

+#include "../config/xsimd_inline.hpp"


why do you need this?

It for header including what they use (otherwise I get nasty errors in my editor).
But this one did not belong here but in xsimd_register.hpp where XSIMD_INLINE is used.

AntoinePrv

Comments on change may make relative to the previous implementation

AntoinePrv · 2026-02-02T10:53:19Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+#elif defined(__INTEL_COMPILER)
+            __cpuid(reg.data(), level);


Should we change to use inline ASM for intel compiler? Missing the count option here.

I assumle so, feel free to give it a try!

AntoinePrv · 2026-02-02T10:54:31Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+#elif defined(_MSC_VER) && _MSC_VER >= 1400
+            return static_cast<xcr0_reg_t>(_xgetbv(0));
+
+#elif defined(__GNUC__)


What about __clang__ and __INTEL_COMPILER? Should we reproduce the get_cpuid pattern?

Yes, I think so. Also, IIRC, __clang__ comes with __GNUC__ defined too, and Intel compiler defines __GNUC__ or _MSC_VER depending on the platform. So we may simplify this logic (but first we need to verify that my asumption is right).

It's true that clang defines __GNUC__. And it happens to be true for icx too, see https://godbolt.org/z/56j57vhnT

AntoinePrv · 2026-02-03T09:07:42Z

@serge-sans-paille what do you think of this?

I wanted to add an x86_cpu_features that mixed xcr0, cpuid, as well as future opinionated features (e.g. override avx2 detection from cpuid if the specific implementation is known not to be performant).
However today that felt very redundant, perhaps in the future if we simplify the logic of supported_arch.

include/xsimd/xsimd_cpu_features_x86.hpp

include/xsimd/config/xsimd_cpu_features_x86.hpp

JohanMabille · 2026-02-19T14:28:33Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+#elif defined(_MSC_VER) && _MSC_VER >= 1400
+            return static_cast<xcr0_reg_t>(_xgetbv(0));
+
+#elif defined(__GNUC__)


Yes, I think so. Also, IIRC, __clang__ comes with __GNUC__ defined too, and Intel compiler defines __GNUC__ or _MSC_VER depending on the platform. So we may simplify this logic (but first we need to verify that my asumption is right).

serge-sans-paille · 2026-02-20T21:49:53Z

@AntoinePrv please ping me when you need a review!

AntoinePrv · 2026-02-23T08:26:46Z

@serge-sans-paille this is ready. This is intended to make a clean abstraction for CPUID.
In follow-up PRs, I will:

Add some sort of similar abstraction for arm64
Propose a generic user-facing interface.

serge-sans-paille · 2026-02-23T11:44:43Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+        constexpr static x86_xcr0 safe_default() noexcept
+        {
+            reg_t low = {};
+            low = utils::set_bit<static_cast<reg_t>(bit::sse)>(low);


Coild be a call to make_mask directly, or (better ?) use an overload of set_bit with no parameter.

serge-sans-paille · 2026-02-23T11:46:32Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+        {
+            // Check all SSE, AVX, and AVX512 bits even though AVX512 must
+            // imply AVX and SSE
+            return bit_is_set<bit::sse, bit::avx, bit::zmm_hi256>(m_low);


it's a bit strange to have a function named bit_is_set that takes several bits as parameter. Maybe all_bits_set<...> ?

serge-sans-paille · 2026-02-23T11:50:12Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+        /**
+         * Read the CpuId registers from the CPU if on the correct architecture.
+         *
+         * This is only safe to call if bit 18 of CR4.OSXSAVE has been set.


Instead of a comment, you could make this an assert

serge-sans-paille · 2026-02-23T11:54:41Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+            regs.reg1 = detail::get_cpuid(0x1);
+            regs.reg7 = detail::get_cpuid(0x7);
+            regs.reg7a = detail::get_cpuid(0x7, 0x1);
+            regs.reg8 = detail::get_cpuid(0x80000001);


Open question: could we avoid some get_cpuid call in some situation? For instance reg8 is only used for fma4, and reg7 for avx2 or later, which mean we could skip filling reg7 if we don't have avx, and fma4 if have fma3.

serge-sans-paille · 2026-02-23T11:55:35Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+#elif defined(__INTEL_COMPILER)
+            __cpuid(reg.data(), level);


I assumle so, feel free to give it a try!

serge-sans-paille · 2026-02-23T11:58:12Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+#if !XSIMD_TARGET_X86
+            (void)level;
+            (void)count;
+            return {}; // All bits to zero


you could avoid this return and let the return reg at the end do the job. I wouldn't mind having

#if XSIMD_TARGET_X86 // current implementation #else inline cpuid_reg_t get_cpuid(int , int ) noexcept { return {}; // All bits to zero }

serge-sans-paille · 2026-02-23T12:00:54Z

include/xsimd/config/xsimd_cpu_features_x86.hpp

+#elif defined(_MSC_VER) && _MSC_VER >= 1400
+            return static_cast<xcr0_reg_t>(_xgetbv(0));
+
+#elif defined(__GNUC__)


It's true that clang defines __GNUC__. And it happens to be true for icx too, see https://godbolt.org/z/56j57vhnT

serge-sans-paille · 2026-02-23T12:07:00Z

include/xsimd/config/xsimd_cpuid.hpp

-
-                auto get_cpuid = [](int reg[4], int level, int count = 0) noexcept
-                {
+                // Safe on all platforms, we simply be false


that english sentence is... difficult to understand

serge-sans-paille · 2026-02-23T12:12:31Z

include/xsimd/config/xsimd_cpuid.hpp


-#if defined(_MSC_VER)
-                    __cpuidex(reg, level, count);
+                sse2 = cpuid.sse2() && xcr0.sse_enabled();


some random thoughts: it looks like we actually want supported_arch to have a filed member that matches the data structure of cpuid, and have a similar layout for xcr0 so that we could just write something like *this = cpuid & xcr0 and implement this very efficiently.

serge-sans-paille · 2026-02-23T12:13:52Z

include/xsimd/utils/bits.hpp

+    {
+        template <typename I>
+        constexpr I make_bit_mask(I bit)
+        {


you could static_assert that 8 * bit < sizeof(I), altough I guess the compiler would issue a warning in that case.

serge-sans-paille reviewed Jan 30, 2026

View reviewed changes

AntoinePrv force-pushed the cpuid branch from b62ca87 to a450fa7 Compare February 2, 2026 10:52

AntoinePrv commented Feb 2, 2026

View reviewed changes

AntoinePrv force-pushed the cpuid branch from a450fa7 to f2c5403 Compare February 2, 2026 14:49

AntoinePrv marked this pull request as draft February 2, 2026 17:18

AntoinePrv marked this pull request as ready for review February 4, 2026 10:13

AntoinePrv force-pushed the cpuid branch from 01ec79e to cf96d20 Compare February 18, 2026 16:05

JohanMabille reviewed Feb 19, 2026

View reviewed changes

AntoinePrv added 15 commits February 19, 2026 17:48

Refactor x86 CPU features

1482e8e

Remove unsafe memset

44ab310

Fix warning

3c0af6f

Fix inline

f4a8fae

Refactor bit utils

e27e3fe

Fix inline header in wrong location

a5b1732

Fix cpuid aliases

a5f9338

Add future TODO

d324351

Fix missing var

1691bad

Com[act lines

525e8f8

Add doc

a5cdb36

Shorter name

f629a99

Add default minimal factory for xcr0

db51ffe

Move cpu features to config

bde1481

Make register class implementation private

8083e0c

AntoinePrv force-pushed the cpuid branch from cf96d20 to 8083e0c Compare February 19, 2026 16:48

Document all xcr0 bits

363258b

AntoinePrv force-pushed the cpuid branch from de6053a to 363258b Compare February 19, 2026 17:15

serge-sans-paille reviewed Feb 23, 2026

View reviewed changes

Comments

Conversation

AntoinePrv commented Jan 30, 2026

Uh oh!

serge-sans-paille left a comment

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!

Uh oh!

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!

AntoinePrv Jan 30, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

AntoinePrv left a comment

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!

JohanMabille Feb 19, 2026 • 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!

AntoinePrv commented Feb 3, 2026

Uh oh!

Uh oh!

Uh oh!

JohanMabille Feb 19, 2026 • 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!

serge-sans-paille commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AntoinePrv commented Feb 23, 2026

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!

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!

Reviewers

Assignees

Labels

Projects

AntoinePrv Jan 30, 2026 •

edited

Loading

JohanMabille Feb 19, 2026 •

edited

Loading

JohanMabille Feb 19, 2026 •

edited

Loading

serge-sans-paille commented Feb 20, 2026 •

edited

Loading