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

cmake: xcc: fix compilation issues #79731

Merged
merged 3 commits into from
Oct 15, 2024
Merged

cmake: xcc: fix compilation issues #79731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e6dad9
cmake: xcc: do not use -fno-printf-return-value
dcpleung Oct 11, 2024
3fa0711
cmake: xcc: use xt-ld linker cmake code
dcpleung Oct 11, 2024
9dcbd21
cmake: xcc: remove TOOLCHAIN_LIBS
dcpleung Oct 11, 2024
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
3 changes: 3 additions & 0 deletions cmake/compiler/xcc/compiler_flags.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ set_compiler_property(PROPERTY no_position_independent "")

# Remove after testing that -Wshadow works
set_compiler_property(PROPERTY warning_shadow_variables)

# xcc does not recognize -fno-printf-return-value
set_compiler_property(PROPERTY no_printf_return_value)
6 changes: 0 additions & 6 deletions cmake/compiler/xcc/target.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,6 @@ foreach(file_name include/stddef.h include-fixed/limits.h)
list(APPEND NOSTDINC ${_OUTPUT})
endforeach()

list(APPEND TOOLCHAIN_LIBS
gcc
hal
)


# For CMake to be able to test if a compiler flag is supported by the
# toolchain we need to give CMake the necessary flags to compile and
# link a dummy C file.
Expand Down
3 changes: 2 additions & 1 deletion cmake/linker/xt-ld/linker_libraries.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
set_linker_property(NO_CREATE PROPERTY c_library "-lc")
set_linker_property(NO_CREATE PROPERTY rt_library "-lgcc")
set_linker_property(NO_CREATE PROPERTY c++_library "-lstdc++")
set_linker_property(PROPERTY link_order_library "c;rt")
set_linker_property(NO_CREATE PROPERTY hal_library "-lhal")
set_linker_property(PROPERTY link_order_library "c;rt;hal")
1 change: 0 additions & 1 deletion cmake/linker/xt-ld/target.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ function(toolchain_ld_link_elf)
${NO_WHOLE_ARCHIVE_LIBS}
$<TARGET_OBJECTS:${OFFSETS_LIB}>
-L${PROJECT_BINARY_DIR}
${TOOLCHAIN_LIBS}

${TOOLCHAIN_LD_LINK_ELF_DEPENDENCIES}
)
Expand Down
1 change: 1 addition & 0 deletions cmake/toolchain/xcc/generic.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ set(COMPILER xcc)
set(OPTIMIZE_FOR_DEBUG_FLAG "-O0")
set(CC xcc)
set(C++ xc++)
set(LINKER xt-ld)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need to use an obsolete linker in addition to using an obsolete compiler? The existing solution worked fine until the big CMake shuffle. Moreover, you seemed to have cracked the TOOLCHAIN_LIBS nut in v2 so is this switch really still necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is better to stay within the confine of the Xtensa toolchain instead of mixing toolchain components that may not be compatible with each other.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is debatable. Compilation and linkage are two steps fairly independent from each other with good compatibility and zephyr/cmake seems to assume they are indeed. xt-xcc has been a pre-C11 pain but it features proprietary customizations found no one else, so it's a necessary pain. What value does xt-ld bring to the table besides old linker bugs that no-one wants to debug and a lack of recent features? Isn't the same than ld, just older? Also: it has apparently NOT been used in production. Probably by accident but still.

But back to the initial issue: can't we have ONLY a fix for the CMake regression FIRST? And then only later a linker change once SOF CI is back on its feet?


list(APPEND TOOLCHAIN_C_FLAGS
-imacros${ZEPHYR_BASE}/include/zephyr/toolchain/xcc_missing_defs.h
Expand Down
Loading