Skip to content

Conversation

@tejlmand
Copy link
Contributor

List of commits from Zephyr main improving support for LLVM.

@NordicBuilder
Copy link
Contributor

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
picolibc zephyrproject-rtos/picolibc@06bde1f zephyrproject-rtos/picolibc@27746bb zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@tejlmand tejlmand added the backport v3.7.99-ncs1-branch Relates to NCS v2.8-branch label Nov 27, 2024
@tejlmand tejlmand force-pushed the llvm_improvements branch 2 times, most recently from 418dd51 to f8c4def Compare December 2, 2024 14:31
@tejlmand tejlmand removed the backport v3.7.99-ncs1-branch Relates to NCS v2.8-branch label Dec 3, 2024
@carlescufi carlescufi added this to the ncs-2.9.0 milestone Dec 3, 2024
@jukkar
Copy link
Contributor

jukkar commented Dec 3, 2024

CI complains now

ERROR: Unable to cherry-pick commit 2e3873adde2386e0bc9acdbcdc8d389fdf4b9aee.
This means that the upstream commit does not apply cleanly into the NCS fork.
This can happen if you modified an upstream commit in order to resolve conflicts, but this is not allowed. 
Instead, revert any [nrf noup] commits that may be causing the conflict and cherry-pick any additional 
prior commits from upstream that may be needed in order to avoid a merge conflict. 
Then you can re-apply the reverted [nrf noup] commits.

@tejlmand
Copy link
Contributor Author

tejlmand commented Dec 3, 2024

CI complains now

Because upstream contains more commits which alters top-level CMakeLists.txt file, some toolchain infrastructure, and the upstream manifest, and this causes some of the cherry-picked commits to not apply cleanly.
Feel free to do a manual inspection and ask question if needed:
Examples:
top-level CMakeList.txt and toolchain infrastructure: 28e7eb5
Upstream cherry-picked commit: zephyrproject-rtos/zephyr@2e3873a

picolibc manifest commit in this PR: 168c233
Upstream cherry-picked commit: zephyrproject-rtos/zephyr@b2eeef4

This is one of the cases wher a manual override is needed and accepted.
cc @carlescufi

@jukkar
Copy link
Contributor

jukkar commented Dec 4, 2024

This is one of the cases wher a manual override is needed and accepted.

Ok, makes sense, just checking.

jonathonpenix and others added 7 commits December 4, 2024 13:38
…d final build

Currently, the linker that is used when performing various cmake checks
(check_c_compiler_flag, for example) may be different than the linker that
will be used during the actual build. This happens as we currently specify
'-fuse-ld' to force the appropriate linker a) after many such checks have
already happened and b) in a way which is not automatically propagated to
check_c_compiler_flag (and friends). As a result, the toolchain's default
linker will generally be used for such checks regardless of which linker
was selected in Zephyr.

This can lead to a number of surprises when building Zephyr, particularly
when building with clang. For example:

- If the linker is misconfigured, where the build will fail can vary
  depending on whether the linker is the toolchain's default. When the
  configured linker happens to be the toolchain's default, the build
  (helpfully) fails quickly on the checks for a basic working toochain.
  When the configured linker isn't the default, the build won't fail until
  the final link steps.
- The build can fail due to issues with a linker other than the one
  configured by the user in Zephyr. For example, LLVM toolchains without
  lld will generally fail to build Zephyr (the checks for a basic
  working toochain will fail) for targets where lld is the default in LLVM
  even if GNU ld is configured in Zephyr and would otherwise be used in the
  final build.
- Flags which are only added if check_c_compiler_flag (or similar) succeeds
  may be unexpectedly omitted during the final build if the flag is
  supported in the configured linker but is unsupported in the toolchain's
  default linker (as check_c_compiler_flag will test using the default
  one).

Note that this isn't limited to clang--even when we are building with
Zephyr's SDK and force ld.bfd, we seem to use the 'ld' variant during the
cmake checks (though this generally seems fairly harmless compared to
mixing ld/lld or other proprietary linkers).

To fix this, ensure the appropriate 'fuse-ld' is set early enough and in
such a way that the same linker will be used throughout the entire build.

Signed-off-by: Jonathon Penix <[email protected]>
Signed-off-by: Torsten Rasmussen <[email protected]>
(cherry picked from commit 9fe6c5e)
Zephyr is a bare metal build where standard libs are disabled.

This means that c and runtime libraries must manually be linked in.

This has generally been handled by using CMake's link libraries handling
but the issue with that is both de-duplication but also library link
order.

Standard libraries must be linked at last location to ensure symbols
are always available, however this is not optimal with
target_link_libraries() because this would ultimately require every
library to know the c library to link with, which is not desired.

Therefore, setup standard C and runtime library linking in linker
CMake files for toolchains where this is required.

This commit expands the principle introduced with toolchain abstraction,
see PR#24851.

This means that a toolchain implementation may specify standard C,
runtime, C++, etc libraries, as well as their link order.
Because a property approach is used, then Zephyr modules, such as the
Picolibc module can adjust such properties.

An optional `zephyr_linker_finalize()` macro is called at the end of
Zephyr's CMakeList process and can be used by the toolchain
implementation to define the final linker invocation.

This aligns the linker handling flow to the principle introduced in
PR#24851 and improves the flexibility and robustness of Zephyr build
system.

Signed-off-by: Torsten Rasmussen <[email protected]>
(cherry picked from commit 2e3873a)
…nk libraries

Extend zephyr_link_libraries to allow an optional value together with
the `zephyr_link_libraries(PROPERTY <property> [<value>])`.

This allow setting linker property combined with a value when linking
Zephyr. The value will only be applied if the property is defined.

Extend zephyr_compile_options to support the same PROPERTY flag that
has been introduced for zephyr_link_libraries().
This remove the need for developers to write complex generator
expressions for compiler flags and thus minimizes mistakes.

The following syntax is now supported in addition to the existing
syntax: `zephyr_compile_options(PROPERTY <property> [<value>])`

Signed-off-by: Torsten Rasmussen <[email protected]>
(cherry picked from commit 718b726)
The `check_set_linker_property()` and `set_linker_property()` takes a
target argument. Make the target argument optional and use the target
`linker` as default target.

The function name `set_linker_property()` already implies that we are
setting a property and the linker target.

Remove the need to specify `TARGET linker` when using the default linker
property target.

Signed-off-by: Torsten Rasmussen <[email protected]>
(cherry picked from commit 102b3fc)
…in properties

Moving specs argument to compiler and linker properties so that the
compiler and linker in use can decide how the flags are mapped / handled
for the compiler and linker in use.

This avoids specifying `--specs=spec.picolibc` for clang which prints a
warning about an unused argument.

Signed-off-by: Torsten Rasmussen <[email protected]>
(cherry picked from commit 9d835fe)
… property

This commit updates picolibc module to remove the need for hard-coding
linking with `-lgcc`.

Instead it sets the c_library linker property and thereby allows the
Zephyr toolchain infrastructure to properly handle the linking of
C and runtime libraries.

Signed-off-by: Torsten Rasmussen <[email protected]>
(cherry picked from commit b2eeef4)
Newlib or Picolibc libraries for LLVM may be compiled or installed from
pre-built sources independently of LLVM itself.

This means that always indicating that TOOLCHAIN_HAS_NEWLIB=OFF and
TOOLCHAIN_HAS_PICOLIBC=OFF are wrong. But it could be just as wrong to
always indicate suport for newlib or picolibc.

Some pre-built LLVM toolchains are provided with default picolibc
support, such as LLVM for Arm embedded, but can also be used with newlib
be installing newlib add-on package.

Unfortunately it's not possible to query LLVM regarding newlib or
picolibc support.

Developers have the option of `-DTOOLCHAIN_HAS_<NEWLIB|PICOLIBC>=ON`,
but this is not widely known and cumbersome to do for each build.

An indication of newlib or picolibc support is the presence of library
specific headers, so to improve current situation we check for library
specific headers, and if those are present we assume support for the
library.

This commit improves the current support for LLVM in Zephyr when
cross-compiling, especially for users of LLVM for Arm embedded.

Signed-off-by: Torsten Rasmussen <[email protected]>
(cherry picked from commit 0274bcb)
@tejlmand
Copy link
Contributor Author

tejlmand commented Dec 4, 2024

closed in favor of #2336

@tejlmand tejlmand closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants