Series of linker infrastructure cleanups and alignments #77887
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commit 282f77e removed the only place defining LIBC_INCLUDE_DIR. Remove the corresponding use of LIBC_INCLUDE_DIR from newlib/CMakeLists.txt as this setting is no longer being defined anywhere. Signed-off-by: Torsten Rasmussen <[email protected]>
The macro `toolchain_cc_nostdinc` were made obsolete with PR#24851 and are no longer invoked. Remove the macro implementations. Signed-off-by: Torsten Rasmussen <[email protected]>
The support of PROPERTY_LINKER_SCRIPT_DEFINES has been broken since the transition to CMake in 12f8f76. The intention was probably to allow users / projects to adjust PROPERTY_LINKER_SCRIPT_DEFINES by setting a CMake cache variable. The implementation tests for the CMake variable (local scope or cache) PROPERTY_LINKER_SCRIPT_DEFINES, but it never uses such CMake variable. Instead it uses a CMake global property named PROPERTY_LINKER_SCRIPT_DEFINES. CMake variables and CMake global properties are two very different things, and therefore the current implementation has never worked. The fact that no one has never noticed this flaw, means that the feature has never been used as intended. Simplify the code by removing the use of the global CMake property and instead set the value of the property on the linker script pre-processing invocation. Signed-off-by: Torsten Rasmussen <[email protected]>
The selection of the runtime library when using LLVM has already been done in llvm/target.cmake and passed to the toolchain_ld_base() function through the global TOOLCHAIN_LD_FLAGS setting. The only reason this haven't been noticed is because of CMake's built-in symbol de-duplication feature. Remove the redundant runtime library handling. Signed-off-by: Torsten Rasmussen <[email protected]>
Fix check_set_linker_property() to correctly handle multiple linker flags passed on function call. Signed-off-by: Torsten Rasmussen <[email protected]>
To ease the use of linker flag properties and to simplify the use of generator expression then an optional PROPERTY argument has been added to the zephyr_link_libraries() function. This means a call such as: zephyr_link_libraries($<TARGET_PROPERTY:linker,<property-name>) can instead be simplified to: zephyr_link_libraries(PROPERTY <property-name>) Thus making intention clearer and keeping the complexity and minimizes the risk of typos when writing generator expressions. Signed-off-by: Torsten Rasmussen <[email protected]>
zephyrbot
added
area: Toolchains
zephyrbot
requested review from
57300,
abrodkin,
aescolar,
cfriedt,
evgeniy-paltsev,
jeremybettis,
keith-packard,
nashif,
nordicjm and
stephanosio
tejlmand
removed request for
nashif,
keith-packard,
cfriedt,
abrodkin,
stephanosio,
aescolar,
nordicjm,
jeremybettis and
57300
Transition the linker flag for the toolchain_ld_base and toolchain_ld_cpp macros to linker flag properties. This work follows the toolchain abstraction started in PR#24851. toolchain_ld_base() was intended for base linker flags, but has slowly become a 'set-any-linker-flag-here' and thus having several `if(<check>)` or `<func>_ifdef(<check> ...)`. Move the check to the top-level Zephyr CMakeLists.txt file, so that it becomes cleaner which part is responsible for setting a value, and then move the actual value (the linker flag) definition to the linker flag property location. It also helps adding support for new linkers, as it becomes clearer which linker flags Zephyr always expects, for example `base` and `cpp_base`, as well as those settings which are targeting for a given purpose, such as linker sort alignment. It also makes it clearer when those are used, for example in top-level CMakeLists.txt with CONFIG_LINKER_SORT_BY_ALIGNMENT compared to this information being buried in a linker support macro. Signed-off-by: Torsten Rasmussen <[email protected]>
tejlmand
force-pushed
the
linker_abstraction_cleanup
branch
from
9c7b641 to
460508d
Compare
nordicjm
previously approved these changes
Sep 3, 2024
Move linker flag setting from toolchain_ld_baremetal() to linker flag properties as to follow the principle used in previos commits and from PR#24851. Signed-off-by: Torsten Rasmussen <[email protected]>
Remove the toolchain_ld_<base|baremetal|cpp> macro as all the macro handling is now done through the use of linker properties. Keep support for calling the old macros for out of tree toolchains which have not been updated to the new property approach. Signed-off-by: Torsten Rasmussen <[email protected]>
tejlmand
force-pushed
the
linker_abstraction_cleanup
branch
from
460508d to
bcab941
Compare
nordicjm
approved these changes
Sep 3, 2024
pdgendt
approved these changes
Sep 3, 2024
|
@keith-packard would you mind approving if you are happy with @tejlmand's replies? |
keith-packard
approved these changes
Sep 4, 2024
marc-hb
reviewed
Sep 4, 2024
| set_property(TARGET linker PROPERTY partial_linking "-r") | ||
|
|
||
| check_set_linker_property(TARGET linker PROPERTY no_relax ${LINKERFLAGPREFIX},--no-relax) | ||
| ) |
There was a problem hiding this comment.
Commit 9a9e252 broke the build with every xt-clang toolchain we have. Any idea?
14:01:36 CMake Error at /srv/home/jenkins/workspace/sof_generic_build/zephyr/cmake/linker/xt-ld/linker_flags.cmake:32:
14:01:36 Parse error. Expected a command name, got right paren with text ")".
14:01:36 Call Stack (most recent call first):
14:01:36 /srv/home/jenkins/workspace/sof_generic_build/zephyr/cmake/target_toolchain_flags.cmake:42 (include)
14:01:36 /srv/home/jenkins/workspace/sof_generic_build/zephyr/cmake/modules/kernel.cmake:148 (include)
14:01:36 /srv/home/jenkins/workspace/sof_generic_build/zephyr/cmake/modules/zephyr_default.cmake:142 (include)
14:01:36 /srv/home/jenkins/workspace/sof_generic_build/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
14:01:36 /srv/home/jenkins/workspace/sof_generic_build/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:97 (include_boilerplate)
14:01:36 CMakeLists.txt:5 (find_package)
There was a problem hiding this comment.
Nevermind, it's trivial. Fix incoming.
EDIT:
tejlmand
added a commit
to tejlmand/zephyr
that referenced
this pull request
Sep 12, 2024
Follow-up: zephyrproject-rtos#77887 The macros: - toolchain_ld_base - toolchain_ld_baremetal - toolchain_ld_cpp was deprecated in 5db1f1a but no check was added to FindDeprecated.cmake, meaning toolchains still providing those macros was not getting a proper deprecation warning. Signed-off-by: Torsten Rasmussen <[email protected]>
tejlmand
added a commit
to tejlmand/zephyr
that referenced
this pull request
Sep 12, 2024
Follow-up: zephyrproject-rtos#77887 The macros: - toolchain_ld_base - toolchain_ld_baremetal - toolchain_ld_cpp was deprecated in 5db1f1a but no check was added to FindDeprecated.cmake, meaning toolchains still providing those macros was not getting a proper deprecation warning. Signed-off-by: Torsten Rasmussen <[email protected]>
tejlmand
added a commit
to tejlmand/zephyr
that referenced
this pull request
Sep 12, 2024
Follow-up: zephyrproject-rtos#77887 The macros: - toolchain_ld_base - toolchain_ld_baremetal - toolchain_ld_cpp was deprecated in 5db1f1a but no check was added to FindDeprecated.cmake, meaning toolchains still providing those macros was not getting a proper deprecation warning. Signed-off-by: Torsten Rasmussen <[email protected]>
carlescufi
pushed a commit
that referenced
this pull request
Sep 16, 2024
Follow-up: #77887 The macros: - toolchain_ld_base - toolchain_ld_baremetal - toolchain_ld_cpp was deprecated in 5db1f1a but no check was added to FindDeprecated.cmake, meaning toolchains still providing those macros was not getting a proper deprecation warning. Signed-off-by: Torsten Rasmussen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains a series of cleanup commit and commits which aligns linker flag handling with the compiler infrastructure introduced in PR #24851 .
This work cleanup and alignment is done to prepare for upcoming work related to improved linker handling, especially related to runtime library linking.
This will be needed for improved LLVM integration with Zephyr.
No functional change expected.