Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix CMake detection of cross-compilers on arm/arm64 build arch #35084

Merged
merged 1 commit into from
Dec 5, 2023
Merged

Fix CMake detection of cross-compilers on arm/arm64 build arch #35084

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions scripts/toolchains/linux.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "x86")
string(APPEND VCPKG_LINKER_FLAGS " -m32")
elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "arm")
set(CMAKE_SYSTEM_PROCESSOR armv7l CACHE STRING "")
if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux" AND CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64")
if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the test entirely seems wrong as that will do the wrong thing when arm is the native architecture?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong, yes. But not worse than current support for modern arm64 hosts.
How much of vcpkg works out of the box on 32-bit arm hosts? The user must provide cmake and ninja, so maybe it is fair to ask for providing the compiler variables, too.

Couldn't we do find_program(var NAMES <prefix>-tool tool ...) ? This would consider user input, prefix, default. Hard-coded values belong into custom toolchain files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I declare the table of possible combinations of host & target instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we do find_program(var NAMES <prefix>-tool tool ...) ? This would consider user input, prefix, default. Hard-coded values belong into custom toolchain files.

FTR find_program is what scripts/cmake/mingw.cmake does:

vcpkg/scripts/toolchains/mingw.cmake

Lines 24 to 25 in 23ceb9c

find_program(CMAKE_C_COMPILER "${CMAKE_SYSTEM_PROCESSOR}-w64-mingw32-gcc")
find_program(CMAKE_CXX_COMPILER "${CMAKE_SYSTEM_PROCESSOR}-w64-mingw32-g++")

(However, I noticed this now due to insufficient initalization of CMAKE_SYSTEM_PROCESSOR in a test project.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong, yes. But not worse than current support for modern arm64 hosts.

OK, I'm sold by 'not worse than the status quo'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user must provide cmake and ninja, so maybe it is fair to ask for providing the compiler variables, too.

Sadly that doesn't seem to be so easy, overriding the variables from the command-line doesn't work... maybe via a triplet, but that would introduce a fair bit of hassle for anyone who wants to perform a native build on an ARM machine. Ideally, we should just make the defaults work (see my comment below and #38113).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we should just make the defaults work

Yes, we can agree on that, and #38113 is a good direction. It is just not the whole discussion from the last days.

if(NOT DEFINED CMAKE_CXX_COMPILER)
set(CMAKE_CXX_COMPILER "arm-linux-gnueabihf-g++")
endif()
Expand All @@ -25,12 +25,12 @@ elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "arm")
endif()
if(NOT DEFINED CMAKE_ASM-ATT_COMPILER)
set(CMAKE_ASM-ATT_COMPILER "arm-linux-gnueabihf-as")
endif()
message(STATUS "Cross compiling arm on host x86_64, use cross compiler: ${CMAKE_CXX_COMPILER}/${CMAKE_C_COMPILER}")
endif()
message(STATUS "Cross compiling arm on host ${CMAKE_HOST_SYSTEM_PROCESSOR}, use cross compiler: ${CMAKE_CXX_COMPILER}/${CMAKE_C_COMPILER}")
endif()
elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64")
set(CMAKE_SYSTEM_PROCESSOR aarch64 CACHE STRING "")
if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux" AND CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64")
if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux")
if(NOT DEFINED CMAKE_CXX_COMPILER)
set(CMAKE_CXX_COMPILER "aarch64-linux-gnu-g++")
endif()
Expand All @@ -42,8 +42,8 @@ elseif(VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64")
endif()
if(NOT DEFINED CMAKE_ASM-ATT_COMPILER)
set(CMAKE_ASM-ATT_COMPILER "aarch64-linux-gnu-as")
endif()
message(STATUS "Cross compiling arm64 on host x86_64, use cross compiler: ${CMAKE_CXX_COMPILER}/${CMAKE_C_COMPILER}")
endif()
message(STATUS "Cross compiling arm64 on host ${CMAKE_HOST_SYSTEM_PROCESSOR}, use cross compiler: ${CMAKE_CXX_COMPILER}/${CMAKE_C_COMPILER}")
endif()
endif()

Expand Down