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

host-gcc: exclude -lgcc to fix -mx32 [qemu_]x86_64 regression #12805

Merged
merged 1 commit into from
Jan 31, 2019
+31 −13
Merged

host-gcc: exclude -lgcc to fix -mx32 [qemu_]x86_64 regression #12805

Show file tree
Hide file tree
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
2 changes: 2 additions & 0 deletions cmake/compiler/gcc/target.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ find_program(CMAKE_NM ${CROSS_COMPILE}nm PATH ${TOOLCHAIN_HOME} NO_
# x86_64 should pick up a proper cross compiler if one is provided,
# but falling back to using the host toolchain is a very sane behavior
# too.
# As an alternative to this fall back try ZEPHYR_TOOLCHAIN_VARIANT=host
# directly.
if(CONFIG_X86_64)
if(CMAKE_C_COMPILER STREQUAL CMAKE_C_COMPILER-NOTFOUND)
find_program(CMAKE_C_COMPILER gcc )
Expand Down
41 changes: 28 additions & 13 deletions cmake/compiler/host-gcc/target.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,19 @@ find_program(CMAKE_RANLILB ranlib )
find_program(CMAKE_READELF readelf)
find_program(CMAKE_GDB gdb )

set(CMAKE_ASM_FLAGS -m32 )
set(CMAKE_C_FLAGS -m32 )
set(CMAKE_CXX_FLAGS -m32 )
set(CMAKE_SHARED_LINKER_FLAGS -m32 ) # unused?
# -march={pentium,lakemont,...} do not automagically imply -m32, so
# adding it here.

# There's only one 64bits ARCH (actually: -mx32). Let's exclude it to
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: Until arm aarch64 is added :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

... but not MIPS64? :-)

This is actually why I'm suggesting in the next paragraph to move this -m32 flag entirely out of this file in the longer term and spread them instead next to the corresponding -march flags. @SebastianBoe ?
This would be a significant refactoring though, way beyond the scope of this small regression fix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

By x32 you mean the x32 ABI which has more registers than i386, right?
https://en.wikipedia.org/wiki/X32_ABI

But native_posix requires 32-bit always (so should be i386 compatible (and x32 I suppose)) and then you want to not set -m32 if we are on x86-64? What am I missing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes native_posix is neither 64 bits nor x32, which is why it's unrelated to this fix.

then you want to not set -m32 if we are on x86-64?

When compiling x86_64 with ZEPHYR_TOOLCHAIN_VARIANT=host right now, -m32 is (silently) overridden by -mx32 because -mx32 (luckily?) comes last. That doesn't seem right. This first part of the patch doesn't fix anything but it removes this element of luck.

# avoid a confusing game of "who's last on the command line wins".
# Maybe the -m32/-miamcu FLAGS should all be next to -march= in the
# longer term?
if(NOT CONFIG_X86_64)
set(CMAKE_ASM_FLAGS -m32 )
set(CMAKE_C_FLAGS -m32 )
set(CMAKE_CXX_FLAGS -m32 )
set(CMAKE_SHARED_LINKER_FLAGS -m32 ) # unused?
endif()

if(CONFIG_CPLUSPLUS)
set(cplusplus_compiler g++)
Expand All @@ -28,19 +37,25 @@ else()
endif()
find_program(CMAKE_CXX_COMPILER ${cplusplus_compiler} CACHE INTERNAL " " FORCE)

# This libgcc code is partially duplicated in compiler/*/target.cmake
execute_process(
COMMAND ${CMAKE_C_COMPILER} ${CMAKE_C_FLAGS} --print-libgcc-file-name
OUTPUT_VARIABLE LIBGCC_FILE_NAME
OUTPUT_STRIP_TRAILING_WHITESPACE
)
assert_exists(LIBGCC_FILE_NAME)
# The x32 version of libgcc is usually not available (can't trust gcc
# -mx32 --print-libgcc-file-name) so don't fail to build for something
# that is currently not needed. See comments in compiler/gcc/target.cmake
if (NOT CONFIG_X86_64)
# This libgcc code is partially duplicated in compiler/*/target.cmake
execute_process(
COMMAND ${CMAKE_C_COMPILER} ${CMAKE_C_FLAGS} --print-libgcc-file-name
OUTPUT_VARIABLE LIBGCC_FILE_NAME
OUTPUT_STRIP_TRAILING_WHITESPACE
)
assert_exists(LIBGCC_FILE_NAME)

# While most x86_64 Linux distributions implement "multilib" and have
# 32 bits libraries off the shelf, things like
# "/usr/lib/gcc/x86_64-linux-gnu/7/IAMCU/libgcc.a" are unheard of.
# So this does not support CONFIG_X86_IAMCU=y
LIST(APPEND TOOLCHAIN_LIBS gcc)
# So this fails CONFIG_X86_IAMCU=y with a "cannot find -lgcc" error which
# is clearer than "undefined reference to __udivdi3, etc."
LIST(APPEND TOOLCHAIN_LIBS gcc)
endif()

# Load toolchain_cc-family macros
# Significant overlap with freestanding gcc compiler so reuse it
Expand Down
1 change: 1 addition & 0 deletions cmake/usage/usage.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ set(arch_list
riscv32
posix
x86
x86_64
xtensa
)

Expand Down