CMake x86 and x86_64 CPU checks fail
Brought to you by:
sobukus
CMAKE_GENERATOR_PLATFORM check is not sufficent, even for Visual Studio generator it could be unset (assuming x64
).
Things getting worse with Ninja generator, CMAKE_GENERATOR_PLATFORM is always empty.
CMAKE_SYSTEM_PROCESSOR will be AMD64 if you build for Visual Studio (ARM).
So, variable based CPU detection in CMake is unusable. The only working solution is compiler checks.
I've made CheckCPUArch module for FLAC (see attachments), feel free to use.
list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../cmake) include(CheckCPUArch) check_cpu_arch_x86(ARCH_IS_X86) check_cpu_arch_x64(ARCH_IS_X64) - if((MSVC AND CMAKE_GENERATOR_PLATFORM STREQUAL "x64") OR (NOT MSVC AND CMAKE_SYSTEM_PROCESSOR MATCHES "([xX]86_|AMD|amd)64")) + if(ARCH_IS_X64)
What the heck … I'll look into this before doing the next maintenance release.
There's really no proper way to get the current arch even if not cross-compiling?
There may be an easier way, but I didn't find it. On the other hand, this one works as it should.
So … this is exclusively for checking if the arch is x86-64. What about the 32 bit x86 check?
Does that work unchanged to detect a ix86 architecture in CMake? The build system handles x86-64, x86, and generic as fallback (hm, ARM is missing there …).
I've implemented x86 check too.
Actually it is stupid compile check, that runs simple program:
And this is
CHECK_CPU_ARCH_X64
macro:So, it is not problem to extend module for any other CPU.
Here is good starting point: Pre-defined Compiler Macros.
Using the 1.26.3 source release, cmake always uses the 32-bit assembly sources.
On my system,
CMAKE_GENERATOR_PLATFORM
was always undefined andCMAKE_SYSTEM_PROCESSOR
was alwaysAMD64
(no matter whether I compiled for 32-bit or 64-bit intel architecture).Using the attached patch, I could get it working on MSVC2015-2019.
I'm using it in my conan recipe at https://github.com/conan-io/conan-center-index/pull/3630
Hm, that's nicer in that it doesn't need any extra checks/files. I see that approach also for opencv … but what about this statement:
I guess, eventually ARM support might be added to the cmake port. That won't work?
Looking at https://cmake.org/cmake/help/latest/manual/cmake-generators.7.html#visual-studio-generators,
CMAKE_GENERATOR_PLATFORM
is only available for MSVC starting from MSVC2017.I just did a small experiment with a simple cmake script using Visual Studio 2019.
From what I see, ARM support should be straightforward.
The following command was executed:
cmake -G "Visual Studio 16 2019" -A <PLATFORM>
PLATFORM
CMAKE_SYSTEM_PROCESSOR
CMAKE_SIZEOF_VOID_P
Win32
AMD64
x64
AMD64
ARM
ARM64
ARM64
ARM64
When using my conan build recipe (setting
CMAKE_SYSTEM_PROCESSOR=AMD64
as described in https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html),I got the following compile error below. I did not look further into it (I might have used wrong cmake flags)
I was partially incorrect in my last message.
CMAKE_GENERATOR_PLATFORM
is supported for more versions of MSVC then I thought yesterday.The documentation states that
CMAKE_GENERATOR_PLATFORM
may be set.The variable is generator specific so I would not depend on it as it will not be used when using the cmake
NMake Makefiles
generator.The defopt failure is just telling that we got no branch in the CMake files that defines some OPT_xxx and pulls in the correct sources (assembly).
So, with the matrix if AMD64 and ARM64 and the void sizes, the cmake build can support the necessary variants? Can one of you prepare a patch that achieves that? The necessary defines and sources for ARM should be apparent from configure.ac.
The attached patch makes it compile for me.
yasm only supports intel architecture, so I don't know how to assembly the ARM assembly files when using MSVC.
I opted to fallback on
OPT_GENERIC
.Tested on:
- gcc9 (x86/x64) @ Linux
- mingw9 (x86/x64)@ Linux
- mingw8 (x64) @ Windows
- msvc (x86/x64/arm/arm64) @ Windows
I currently have no working aarch64 toolchain on Linux, so I couldn't test it.
Please test it to have greater confidence.
I imported the fix and tested that the normal build still works.
So … can we close this?
I will test it with a toolchain from https://developer.arm.com/tools-and-software/open-source-software/developer-tools/gnu-toolchain/gnu-a/downloads today, and report back.
I found 2 bugs:
- a wrong define (
-DDOPT_MULTI
instead of-DOPT_MULTI
)- missing assembly sources for arm32
I tested it with the 32-bit and 64-bit Linux toolchains (gcc-arm-10.2-2020.11-x86_64-aarch64-none-linux-gnu and gcc-arm-10.2-2020.11-x86_64-arm-none-linux-gnueabihf) with the following cmake configure commands:
and
I extracted the toolchain to
/tmp
.Normally,
CMAKE_SYSTEM_NAME
andCMAKE_SYSTEM_PROCESSOR
should be configured by toolchain files containing these defines. But this works too.Yes, it is broken what is not tested. I updated trunk with your current patch.
Ok, current master/trunk was fine all along.
I forgot to pass
CMAKE_ASM_FLAGS=-m32
.Let's reopen that. The initial suggestion got buried. Seems like we have to do stupid compiler macro checks. This is insane. CMake be that braindead? Or are people supposed to set the processor variable themselves?
Well … another way is hacking around VCPKG env … https://github.com/microsoft/vcpkg/pull/13465 it's all ugly.
Edit: Also, all this talk about having toolchain files if you want to build things … that's just an alien concept to something like mpg123. We assume the toolchain being your platform's cc. It should do the right thing. Good old days, when you got an OS and a compiler that came with it.
Last edit: Thomas Orgis 2021-05-29
I was also unpleasantly surprised that CMake does not have such a standard feature. Well, nothing is perfect. Moreover, the fix is trivial.
I implemented that now, extending your suggestion to ARM. I hope that fixes the machine choice.
The last thing then is to settle the header usage over in bug 310.
Confirm, it works:
arm:
ar64:
No
CheckCPUArch.cmake
&CheckCPUArch.c.in
in tarball.BTW it is better to move it to your CMake modules directory (where is
read_api_version
and company).