Menu

#298 CMake x86 and x86_64 CPU checks fail

1.26.x
closed-fixed
nobody
cmake (4)
5
2021-06-05
2020-10-31
No

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)
2 Attachments

Discussion

1 2 > >> (Page 1 of 2)
  • Thomas Orgis

    Thomas Orgis - 2020-11-02

    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?

     
  • Evgeni Poberezhnikov

    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.

     
  • Thomas Orgis

    Thomas Orgis - 2020-11-21

    So … this is exclusively for checking if the arch is x86-64. What about the 32 bit x86 check?

    elseif((MSVC AND CMAKE_GENERATOR_PLATFORM MATCHES "|Win32") OR (NOT MSVC AND CMAKE_SYSTEM_PROCESSOR MATCHES "i.86"))
    

    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 …).

     
  • Evgeni Poberezhnikov

    So … this is exclusively for checking if the arch is x86-64. What about the 32 bit x86 check?

    I've implemented x86 check too.

    check_cpu_arch_x86(ARCH_IS_X86)
    

    Actually it is stupid compile check, that runs simple program:

    int main(void) {
    #if @CHECK_CPU_ARCH_DEFINES@
        return 0;
    #else
        fail
    #endif
    }
    

    And this is CHECK_CPU_ARCH_X64 macro:

    macro(CHECK_CPU_ARCH_X64 VARIABLE)
        _CHECK_CPU_ARCH(x64 "defined(__amd64__) || defined(__amd64) || defined(__x86_64__) || defined(__x86_64) || defined(_M_X64) || defined(_M_AMD64)" ${VARIABLE})
    endmacro(CHECK_CPU_ARCH_X64)
    

    So, it is not problem to extend module for any other CPU.

    Here is good starting point: Pre-defined Compiler Macros.

     
  • Maarten

    Maarten - 2020-11-23

    Using the 1.26.3 source release, cmake always uses the 32-bit assembly sources.
    On my system, CMAKE_GENERATOR_PLATFORM was always undefined and CMAKE_SYSTEM_PROCESSOR was always AMD64 (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

     
  • Thomas Orgis

    Thomas Orgis - 2020-11-23

    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:

    CMAKE_SYSTEM_PROCESSOR will be AMD64 if you build for Visual Studio (ARM).

    I guess, eventually ARM support might be added to the cmake port. That won't work?

     
  • Maarten

    Maarten - 2020-11-24

    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.

    cmake_minimum_required(VERSION 3.15)
    project(dummy)
    message("CMAKE_SYSTEM_PROCESSOR: ${CMAKE_SYSTEM_PROCESSOR}")
    message("CMAKE_SIZEOF_VOID_P: ${CMAKE_SIZEOF_VOID_P}")
    

    The following command was executed:
    cmake -G "Visual Studio 16 2019" -A <PLATFORM>

    PLATFORM CMAKE_SYSTEM_PROCESSOR CMAKE_SIZEOF_VOID_P
    Win32 AMD64 4
    x64 AMD64 8
    ARM ARM64 4
    ARM64 ARM64 8

    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)

    C:\Users\maarten\.conan\data\mpg123\1.26.3\_\_\build\0aa61bbcd9ccb7515e4a609851670e4b891e1a3e\source_
    subfolder\src\libmpg123\optimize.c(139,40): error C2065: 'defopt': undeclared identifier [C:\Users\ma
    arten\.conan\data\mpg123\1.26.3\_\_\build\0aa61bbcd9ccb7515e4a609851670e4b891e1a3e\source_subfolder\p
    orts\cmake\src\libmpg123\libmpg123.vcxproj]
    
     
  • Maarten

    Maarten - 2020-11-24

    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.

     
  • Thomas Orgis

    Thomas Orgis - 2020-11-28

    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.

     
  • Maarten

    Maarten - 2020-12-07

    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.

     
  • Thomas Orgis

    Thomas Orgis - 2020-12-11

    I imported the fix and tested that the normal build still works.

    So … can we close this?

     
  • Maarten

    Maarten - 2020-12-11

    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:

    CC=/tmp/gcc-arm-10.2-2020.11-x86_64-arm-none-linux-gnueabihf/bin/arm-none-linux-gnueabihf-gcc cmake .. -DCMAKE_SYSTEM_NAME=Linux -DCMAKE_SYSTEM_PROCESSOR=arm -DCHECK_MODULES=dummy -DCMAKE_INSTALL_PREFIX=prefix
    make
    make install
    

    and

    CC=/tmp/gcc-arm-10.2-2020.11-x86_64-aarch64-none-linux-gnu/bin/aarch64-none-linux-gnu-gcc cmake .. -DCMAKE_SYSTEM_NAME=Linux -DCMAKE_SYSTEM_PROCESSOR=aarch64 -DCHECK_MODULES=dummy -DCMAKE_INSTALL_PREFIX=prefix
    make
    make install
    

    I extracted the toolchain to /tmp.
    Normally, CMAKE_SYSTEM_NAME and CMAKE_SYSTEM_PROCESSOR should be configured by toolchain files containing these defines. But this works too.

     
  • Thomas Orgis

    Thomas Orgis - 2020-12-11

    Yes, it is broken what is not tested. I updated trunk with your current patch.

     
  • Maarten

    Maarten - 2020-12-11

    Ok, current master/trunk was fine all along.
    I forgot to pass CMAKE_ASM_FLAGS=-m32.

     
  • Thomas Orgis

    Thomas Orgis - 2021-03-14
    • status: open --> closed-fixed
     
  • Thomas Orgis

    Thomas Orgis - 2021-05-29
    • status: closed-fixed --> open
     
  • Thomas Orgis

    Thomas Orgis - 2021-05-29

    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?

     
  • Thomas Orgis

    Thomas Orgis - 2021-05-29

    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
  • Evgeni Poberezhnikov

    I was also unpleasantly surprised that CMake does not have such a standard feature. Well, nothing is perfect. Moreover, the fix is trivial.

     
  • Thomas Orgis

    Thomas Orgis - 2021-05-29

    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.

     
  • Evgeni Poberezhnikov

    Confirm, it works:

    arm:

    [cmake] -- Check CPU architecture is x86
    [cmake] -- Check CPU architecture is x86 - no
    [cmake] -- Check CPU architecture is x64
    [cmake] -- Check CPU architecture is x64 - no
    [cmake] -- Check CPU architecture is arm32
    [cmake] -- Check CPU architecture is arm32 - yes
    [cmake] -- Check CPU architecture is arm64
    [cmake] -- Check CPU architecture is arm64 - no
    [cmake] -- Detected machine: arm32
    [cmake] CMake Warning at src/libmpg123/CMakeLists.txt:169 (message):
    [cmake]   Cannot use platform-specific assembly sources on MSVC
    [cmake] 
    [cmake] 
    [cmake] -- Configuring done
    [cmake] -- Generating done
    [cmake] -- Build files have been written to: D:/source/repos/libmpg123/build
    

    ar64:

    [cmake] -- Check CPU architecture is x86
    [cmake] -- Check CPU architecture is x86 - no
    [cmake] -- Check CPU architecture is x64
    [cmake] -- Check CPU architecture is x64 - no
    [cmake] -- Check CPU architecture is arm32
    [cmake] -- Check CPU architecture is arm32 - no
    [cmake] -- Check CPU architecture is arm64
    [cmake] -- Check CPU architecture is arm64 - yes
    [cmake] -- Detected machine: arm64
    [cmake] CMake Warning at src/libmpg123/CMakeLists.txt:141 (message):
    [cmake]   Cannot use platform-specific assembly sources on MSVC
    [cmake] 
    [cmake] 
    [cmake] -- Configuring done
    [cmake] -- Generating done
    [cmake] -- Build files have been written to: D:/source/repos/libmpg123/build
    
     
  • Evgeni Poberezhnikov

    No CheckCPUArch.cmake & CheckCPUArch.c.in in tarball.

     
  • Evgeni Poberezhnikov

    BTW it is better to move it to your CMake modules directory (where is read_api_version and company).

     
  • Thomas Orgis

    Thomas Orgis - 2021-06-05
    • status: open --> closed-fixed
     
1 2 > >> (Page 1 of 2)

Log in to post a comment.