microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
22.2k stars 6.16k forks source link

[speexdsp] Cross-compiling from Linux with i686 target fails due to SSE disabled #37574

Open dsvensson opened 3 months ago

dsvensson commented 3 months ago

Operating system

Linux

Compiler

i686-w64-mingw32-gcc

Steps to reproduce the behavior

The homegrown CMakeFile.txt for speexdsp does not work with cross-compiling to i686 with mingw. This happens because the portfile.cmake sets USE_SSE=ON, but the CMakeFile.txt doesn't consider that -msse2 is needed for that option to work.

Failure logs

In file included from /..../vcpkg/buildtrees/speexdsp/src/1.2.1-6efcf61068.clean/libspeexdsp/resample.c:100:
/..../vcpkg/buildtrees/speexdsp/src/1.2.1-6efcf61068.clean/libspeexdsp/resample_sse.h: In function ‘inner_product_single’:
/..../vcpkg/buildtrees/speexdsp/src/1.2.1-6efcf61068.clean/libspeexdsp/resample_sse.h:44:11: warning: SSE vector return without SSE enabled chang
es the ABI [-Wpsabi]
   44 |    __m128 sum = _mm_setzero_ps();
      |           ^~~
In file included from /usr/lib/gcc/i686-w64-mingw32/12-win32/include/xmmintrin.h:1319,
                 from /..../vcpkg/buildtrees/speexdsp/src/1.2.1-6efcf61068.clean/libspeexdsp/resample_sse.h:37:
/usr/lib/gcc/i686-w64-mingw32/12-win32/include/emmintrin.h: In function ‘inner_product_double’:
/usr/lib/gcc/i686-w64-mingw32/12-win32/include/emmintrin.h:111:1: error: inlining failed in call to ‘always_inline’ ‘_mm_setzero_pd’: target specific option mismatch
  111 | _mm_setzero_pd (void)
      | ^~~~~~~~~~~~~~

Additional context

Is there any workaround for passing that from my CMakePresets.json, or perhaps CMakeLists.txt?

FrankXie05 commented 3 months ago

@dsvensson You can modify the code in your local ports/speexdsp/portfile.cmake file like this and try again

if(VCPKG_TARGET_ARCHITECTURE STREQUAL "x64" OR VCPKG_TARGET_ARCHITECTURE STREQUAL "x86" OR VCPKG_TARGET_IS_MINGW)

https://github.com/microsoft/vcpkg/blob/9765877106f7c179995e8010bb7427a865a6bd1d/ports/speexdsp/portfile.cmake#L27

dsvensson commented 3 months ago

Doing that change would have no impact as that condition already evaluates to true. The problem is that the x86 mingw compiler does not automatically enable sse and sse2. I don't think it would be controversial for the mingw x86 triplet to enable all the SSE variants as I suspect it's more common to use vcpkg with 32 bit mingw output to run on x64 for potential performance reasons, rather than targetting actual old hardware, old enough to not have SSE2 and SSE. So I think a better change would be for the mingw32 triplet to just pass -msse2 or higher. It would be sweet if a CMakePresets.json could pass VCPKG_C_FLAGS, but if I understand correctly that needs to be passed via a triplet.

So in short, two things,

FrankXie05 commented 3 months ago

@dsvensson Thank you for your patient explanation and suggestions. :) For triplet files: The content itself contains some usage and definitions of architecture, CRT and system names (within vcpkg) and illustrates the architectures on which we can install ports.

The homegrown speexdsp CMakeFile ought to probe the toolchain for SSE2 support, and disable SSE2 features if missing.

We currently enable sse by default for all triples of x86 and x64 architectures and have compiled the sse2 and sse3 options. I think this will totally work with all the mingw triples(x86 and x64) we have currently.

x64-mingw-dynamic.cmake
x64-mingw-static.cmake
x64-mingw-static.cmake
x64-mingw-static.cmake

The mingw32 i686 triplet should perhaps bump its target architecture featureset to include at least SSE2.

Maybe I misunderstood. Although we currently do not have triple files for mingw32 or i686, I understand that you want to add functions like set VCPKG_C_FLAGS "" to the triple files. https://github.com/microsoft/vcpkg/tree/master/triplets

We may not accept such settings. As I said at the beginning, the triple file will only define some common settings, because not all ports need to use functions such as sse. Even if judgment is made, it should not be in the triple file.

dsvensson commented 3 months ago

Well, Ideally I would like my CMakePresets.json to be able to contribute to VCPKG_C_FLAGS per configurationPreset. There are other lightweight usecases such as adding -mmacosx-version-min=12.7 in some configurationPreset, and another version in another for macOS for example, but that's drifting off-topic.

You mention having sse enabled for x86 and x64, are you referring to speexdsp portfile.cmake's use of USE_SSE=ON, or the actual triplets? I only specify x86-mingw-static as VCPKG_TARGET_TRIPLET in my CMakePresets atm, and the build of speexdsp fails due to -msse2 not being added when the USE_SSE feature is enable.

mingw32 is just bad naming of me, I mean the w64 flavor, the binaries are called i686-w64-mingw32-gcc and x86_64-w64-mingw32-gcc on Linux.

So while this works just fine when building with x86_64-w64-mingw32-gcc, the build fails unless -msse2 is added when building with i686-w64-mingw32-gcc.

Are you perhaps referring to mingw64 builds on Windows, and is there perhaps a discrepancy between enabled features in the actual compiler build between mingw64 in Ubuntu vs the upstream binaries for Windows?

FrankXie05 commented 3 months ago

@dsvensson If you are using x86-mingw-static.cmake(Like: ./vcpkg install speexdsp:x86-mingw-static), VCPKG_TARGET_ARCHITECTURE will be preset and loaded as x86 during the build.

Then USE_SSE will be set to ON. After passing it to CMakeLists.txt, the command add_definitions(-DUSE_SSE -DUSE_SSE2) will be passed to cmake.

https://github.com/microsoft/vcpkg/blob/9765877106f7c179995e8010bb7427a865a6bd1d/triplets/community/x86-mingw-static.cmake#L1C1-L1C35

dsvensson commented 3 months ago

Yes, correct, and this is what's causing the problem, because the i686-w64-mingw32-gcc doesn't have -msse2 enabled by default, so the build will fail with the message I started the issue with.

FrankXie05 commented 3 months ago

It is strange.🧐 Could you please provide your config log? :)

FrankXie05 commented 3 months ago

And if it's convenient, can you provide your command line input and output?

dsvensson commented 3 months ago

Not at all weird, this is just the default behavior of i686-w64-mingw32-gcc. The Interweb is filled with examples. Here's another one, https://bugzilla.mozilla.org/show_bug.cgi?id=1331335 ... you can't just ask i686-w64-mingw32-gcc to use SSE2 features without also enabling -msse2.

FrankXie05 commented 3 months ago

This behavior is vcpkg's default pass on x86 and x64.

add_definitions (-DUSE_SSE -DUSE_SSE2)

And this macro is actually common to upstream. https://gitlab.xiph.org/search?search=USE_SSE&nav_source=navbar&project_id=22&group_id=2&search_code=true&repository_ref=master

Does it mean that for i686-w64-mingw32-gcc, it must be the transmission of -msse2 or -msse3?🤔

FrankXie05 commented 3 months ago

Sorry, I don't have the environment or any build logs to determine what exactly happened during the build.

dsvensson commented 3 months ago

The define is just arbitrarily named define for the code to use. The compiler doesn't care. It's in no way connected to what architecture feature sets are enabled in the compiler. Those feature sets are controlled via -m. The upstream project uses autohell that performs this check, similar to CheckCSourceCompiles in CMake-land:

AC_MSG_CHECKING(for SSE in current arch/CFLAGS)
AC_LINK_IFELSE([
AC_LANG_PROGRAM([[
#include <xmmintrin.h>
__m128 testfunc(float *a, float *b) {
  return _mm_add_ps(_mm_loadu_ps(a), _mm_loadu_ps(b));
}
]])],
[
has_sse=yes
],
[
has_sse=no
]
)
AC_MSG_RESULT($has_sse)

AC_MSG_CHECKING(for SSE2 in current arch/CFLAGS)
AC_LINK_IFELSE([
AC_LANG_PROGRAM([[
#include <emmintrin.h>
__m128d testfunc(double *a, double *b) {
  return _mm_add_pd(_mm_loadu_pd(a), _mm_loadu_pd(b));
}
]])],
[
has_sse2=yes
],
[
has_sse2=no
]
)
AC_MSG_RESULT($has_sse2)

Begs the question what other types of breakages there are in other ports that replace upstream build systems.

The above tests would pass if the i686-w64-mingw32-gcc (or more likely any x86 32 bit version of gcc) compiler was called with -msse2 (or equivalent arg to other 32 bit x86 compilers) and fail otherwise, gracefully omitting the USE_SSE* defines where such features aren't available. It even has the --enable-sse configure parameter that explictly appends the -msse -msse2 flags. Similar breakage is likely true for USE_NEON case, where configure.ac appends -march=armv7-a -mfpu=neon. Seems like the upstream configure.ac does a lot of stuff that the CMakeFile.txt just handwaves.

dg0yt commented 3 months ago

And if it's convenient, can you provide your command line input and output?

vcpkg.exe install speexdsp:x86-mingw-dynamic
dsvensson commented 3 months ago

Feels like at minimum the speexdsp port should gain CheckCSourceCompiles for sse, sse2, and neon features, and the ugly if(VCPKG_TARGET_ARCHITECTURE STREQUAL "x64" OR VCPKG_TARGET_ARCHITECTURE STREQUAL "x86") and if(VCPKG_TARGET_ARCHITECTURE STREQUAL "arm" OR VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64") could be removed in favor of the CheckCSourceCompiles result for both architecture features.

With such a change a vcpkg user could specify their own VCPKG_C_FLAGS in some triplet that enables -msse2, and that would allow speexdsp to enable USE_SSE flag, and unbreaks builds that don't care about that and use the standard triplet.

For x64 builds, such tests would always enable USE_SSE as per the gcc documentation:

For the x86-32 compiler, you must use -march=cpu-type, -msse or -msse2 switches to enable SSE extensions and make this option effective. For the x86-64 compiler, these extensions are enabled by default.

Also, why have you decided to only set the USE_SSE define, and not also USE_SSE2?