Why does qnumeric.h #include <initrin.h>

Perdrix

This rather messes things up if you wish to use simde.

So why do you do this:

#if defined(Q_CC_MSVC)
#  include <intrin.h>
#  include <float.h>
#  if defined(Q_PROCESSOR_X86_64) || defined(Q_PROCESSOR_ARM_64)
#    define Q_INTRINSIC_MUL_OVERFLOW64
#    define Q_UMULH(v1, v2) __umulh(v1, v2);
#    define Q_SMULH(v1, v2) __mulh(v1, v2);
#    pragma intrinsic(__umulh)
#    pragma intrinsic(__mulh)
#  endif
#endif

# if defined(Q_OS_INTEGRITY) && defined(Q_PROCESSOR_ARM_64)
#  include <arm64_ghs.h>
#  define Q_INTRINSIC_MUL_OVERFLOW64
#  define Q_UMULH(v1, v2) __MULUH64(v1, v2);
#  define Q_SMULH(v1, v2) __MULSH64(v1, v2);
#endif

David

Perdrix

I provided a patch and it was accepted. :)

https://bugreports.qt.io/browse/QTBUG-131757

Christian Ehrlicher

@Perdrix said in Why does qnumeric.h #include <initrin.h>:

So why do you do this:

Because it is needed - remove it and see what breaks.

Perdrix

That really doesn't qualify as the most informative of replies :(

Christian Ehrlicher

@Perdrix said in Why does qnumeric.h #include <initrin.h>:

That really doesn't qualify as the most informative of replies :(

What do you expect? There are no useless includes so they are needed. If you want to know where it is needed - remove it and see where it breaks...

SGaist

Hi,
Can you show the exact problem you have ?

Qt uses SIMD in QString.

Perdrix

The problem occurs when we include the simde header - all the stuff it defines is already defined by the inclusion of intrin.h. The result is thousands of errors about re-definitions. No, we can't use the definitions from intrin.h :(

I've made a post to the Interest mailing list about this.

David

Perdrix

Here's a (very) few lines showing the problem:

1>C:\Users\mtoeltsch\dev\DSS\DeepSkyStackerKernel\simde\x86\avx512\types.h(624,26): error C2371: '__m512d': redefinition; different basic types
1>(compiling source file '/avx.cpp')
1>    C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.42.34433\include\zmmintrin.h(51,62):
1>    see declaration of '__m512d'
1>C:\Users\mtoeltsch\dev\DSS\DeepSkyStackerKernel\simde\x86\avx512\types.h(634,27): error C2371: '__m128bh': redefinition; different basic types
1>(compiling source file '/avx.cpp')
1>    C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.42.34433\include\zmmintrin.h(4900,17):
1>    see declaration of '__m128bh'```

Perdrix

And here's what I posted to the mailing list:
The header file qnumeric.h includes <intrin.h> if the MVSC compiler is in use so that implementations of qMulOverflow and qAddOverFlow that exploit the intrinsics can be defined.

As far as I can determine, the code also implements “generic” versions of those which do not rely on the use of MSVC intrinsics.

Why am going on about this. The project which I work on (DeepSkyStacker) uses SIMD on Intel right now and we want to port our code to macOS Arm and Windows Arm and after a lot of deliberation we decided we would use SIMD Everywhere (SIMDe) .

We use pre-compiled headers and have a lot of Qt code, so we long ago placed an include for <QtCore> (and other Qt headers) in our precompiled header files.

The problem we hit is that when we then include the simde headers we get LOTS of errors because of duplicate symbols (we compile with warnings as errors).

I will be quite a lot of effort for us to work around that.

I believe, but don't know for sure that qnumeric.h could be changed without a lot of effort so that it didn’t use intrinsics for qMulOverflow and qAddOverFlow and therefore not include <intrin.h>

If that is possible, I'd like to request that this be done.

Please could someone who knows that stuff better than I please comment.

Christian Ehrlicher

@Perdrix said in Why does qnumeric.h #include <initrin.h>:

If that is possible, I'd like to request that this be done.

Feel free to provide a patch ... Qt is opensource and when you think something has to change you can do it by yourself. Or pay for it.

Perdrix

@Christian-Ehrlicher If I was sure it was safe to provide a patch, I would happily do so. But at this stage I await the input from the folks that know how that header is used better than I.

I've been investigating where and how qAddOverflow and qMulOverflow are used and I suspect that qnumeric.h could be made private to the build of Qt, rather than being a public header.

mtoeltsch

@Christian-Ehrlicher
You use the "mulh" intrinsics to detect overflows in 2 template specialisations:

template <> inline bool qMulOverflow(...);

Can I suggest that you define these specialisations in a .cpp file instead of a header file? Then you only need to include <intrin.h> in the .cpp file and avoid pollution of the entire client code (as in our case).

Martin

mtoeltsch

@SGaist said in Why does qnumeric.h #include <initrin.h>:

Qt uses SIMD in QString.

That's fine as long as you do it in .cpp files only and not in (public) header files.
<qstring.h> does not use SIMD intrinsics AFAIK.

Christian Ehrlicher

@mtoeltsch said in Why does qnumeric.h #include <initrin.h>:

Can I suggest that you define these specialisations in a .cpp file instead of a header file?

Why should I do here anything? It's neither my code nor do I have a problem with this. Feel free to provide a patch if you think it can work another way and see if it gets accepted.

Perdrix

I provided a patch and it was accepted. :)

https://bugreports.qt.io/browse/QTBUG-131757

SGaist

Congratulations !