Sync-up Main arm 12-18-2024 #3
Open
Conversation
This file contains hidden or 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
Introduced a configuration file which enables several optimisation flags. Change-Id: Idf6334f828177e7facfb12bbbb6a0159a877ee31
Applying the 0002-libc-tests-with-picolibc-disable-large-tests.patch llvm-project patch failed because of llvm-project commit 3497500946c9b6a1b2e1452312a24c41ee412b34: "[libc++] Clean up and update deployment target features (#96312)" This patch should make the application succeed again.
I've renamed it so it doesn't have NEWLIB in the name: now it's just
called `LLVM_TOOLCHAIN_LIBRARY_OVERLAY_INSTALL`. You configure it to
make an overlay package of newlib in particular by also defining
`LLVM_TOOLCHAIN_C_LIBRARY=newlib` (which you already had to do anyway,
even with the previous option name).
The old option name is no longer a cmake cache entry, but it is
detected on the command line, and used to set the default for the new
option. So existing cmake command lines should still work.
I've also removed some of the literal uses of the string 'newlib' in
code conditionalised by the option. Now those strings are obtained by
expanding `${LLVM_TOOLCHAIN_C_LIBRARY}`. So when we add another libc
type in the next commit, they should automatically adapt.
This is extremely new and barely tested: I've compiled and run a hello-world program, and that is literally all. But that's a start – if you can do that, you can also investigate what else does and doesn't compile or run successfully. So this is enough to allow users to experiment. As documented in the new `docs/llvmlibc.md`, this currently builds C libraries only (not C++), for AArch32 only. I don't know how much effort it will take to fix each of those: I only know that in each case _at least one_ cause of build failure exists, but I don't know how many more are hiding behind that one. You can use the new `-DLLVM_TOOLCHAIN_C_LIBRARY` with or without the option to build an overlay package. If you have both a newlib and an llvm-libc overlay package, then it should be possible to install both alongside each other on top of the same main toolchain. Unlike picolibc and newlib, llvm-libc doesn't come with a crt0.o startup file. Compiled for bare metal (as we're doing here), it also leaves some support functions undefined (e.g. for writing to stdout or stderr), expecting the user to provide them. So I've also added a small source directory containing the missing pieces. That builds two additional libraries, -lcrt0 and -lsemihost. At present, I haven't set up a llvm-libc specific ld script, and there's no code in my new crt0 that copies RW data out of ROM (as there is in picolibc). The only definition required by the crt0 is `__stack`, which you can define via an ld script of your own if you want, or directly on the ld.lld command ilne if you prefer.
This copies `libc/LICENSE.TXT` from the llvm-project repository into the `third-party-licenses` directory, renamed to `LIBC-LICENSE.TXT` so you can tell it apart from all the other subdirectory licenses. (We were already doing similar renames for libcxx, libunwind, etc.) When you build the toolchain in llvm-libc-only mode, that file shows up along with all the others at the top level of `third-party-licenses`. If you build an overlay package, it ends up in the `third-party-licenses/llvmlibc` subdirectory.
…s long one (ARM-software#457) Co-authored-by: Vrukesh V Panse <[email protected]>
The motivation for this change is that users may wish to disable LTO when using the the Omax.cfg file. Removing the LTO flags to a seperate file allows the user to enable LTO related flags seperately. Change-Id: Id9ed8d8051f7ca4611b640f838715f658790d9cf
ARM-software#458) Co-authored-by: Vrukesh V Panse <[email protected]>
…software#459) * Update pre-requisite for executing on Windows 10 and Windows 11 * Update pre-requisite for executing on Windows --------- Co-authored-by: Vrukesh V Panse <[email protected]>
…are#460) * Do not default to linking crt0 when using crt0-semihosting This commit adds the -nostartfiles option to tests using semihosting. This is in response to a recent change in clang which modified the driver to include crt0.o in the link: llvm/llvm-project#101258 While the change to clang removes the need to include the crt0 library directly, the tests using semihosting need to link against a different crt0 library. Therefore the new default behavior needs to be suppressed in these cases, which can be done by adding the -nostartfiles option. * Also remove -lcrt0 from make.bat
Updating the README.md file to detail usage of Omax.cfg and OmaxLTO.cfg.
I made two mistakes in this function: it used size_t without including any header file that defines it, and it was indecisive about whether its argument was called `code` or `status`. But both mistakes only affected the AArch64 branch of the #ifdef, which we weren't using yet, so they didn't cause a compile failure.
We couldn't previously build AArch64 llvmlibc, because the build failed due to lack of setjmp/longjmp. But now llvmlibc does have those functions for AArch64, so we can make building AArch64 library variants unconditional again, not dependent on which libc you want. The other missing thing for AArch64 is a configuration file in libc/config/baremetal (it only has configurations for two architectures, and AArch64 isn't one of them). I think the easiest approach is to reuse the AArch32 configuration, so that we build the same set of library functions into all our bare-metal libraries, 32- or 64-bit.
llvmlibc used to use a 32-bit time_t unconditionally on AArch32, commented as being for compatibility with glibc. But now it has an option to choose 64-bit instead, if you don't need to conform to glibc's ABI. A bare-metal libc indeed has no need to worry about glibc's time_t ABI (even for so long as it _stays_ 32-bit, which is probably not much longer). So it makes sense for us to use 64-bit time_t from the start, by setting the new config option.
llvmlibc's own test suite is written in C++, just like the library itself. But unlike the library, the test suite also depends on having a C++ library available: it uses iostreams, fstreams, vectors and so on. So until it's possible to build libc++ on top of a bare-metal version of llvmlibc, I can't see how we'd be able to run the tests.
…--defsym for library selection (ARM-software#463) * Document the behaviour difference between LLD and GNU LD while using --defsym for library selection * Addressing the comments. 1. Add backquotes 2. Add the reference to https://github.com/picolibc/picolibc/blob/main/doc/printf.md 3. rephrase some words. * Addressing the suggestions about wording/formatting.
Commit hash e434fa1f41a83a888d6498922ab7f140011933d1 was the last one working. Set it as the default revision while recent breakages are not fixed.
A header comment in CMakeLists.txt suggests a sequence of commands for manually checking out the source repos and applying the patches. But it doesn't work, because it includes the command git -C repos/llvm-project am -k ../../patches/llvm-project/*.patch which, under POSIX shell semantics, is handled by first trying to expand the wildcard, then running the git command, which will change directory into `repos/llvm-project` because of the `-C` option. But the command only works if the wildcard is expanded _after_ changing directory, because the `../..` on the front of the pathname is intended to get back out of `repos/llvm-project`. To fix this, I've replaced the relative paths with `$PWD`, which will be expanded before git changes directory, so the pathnames will refer to the root of this repo. (I wondered if the command would have worked as written on Windows, where wildcards are expanded by the application and not the shell. But it doesn't look like it: on Windows, `git am` doesn't seem to expand wildcards at all. So it would be much more painful to rewrite this command sequence in a Windows-compatible way, and in this commit I haven't tried.)
In the `Building` section of building-from-source.md, twice we are trying to
apply a patch with the following command:
git -C repos/<repo> am -k "$PWD"/../patches/<repo>/*.patch
This command is executed from the root of the repo. The original thought
for this must have been that because of the `-C`, our path at time of
execution is `repos/<repo>`, and thus we need to move a level up from our
current directory. However `$PWD` will be replaced by the shell before the
command is executed. So we need to remove the spurious `../`.
This adds the folowing library variants with support for the PACBTI-M architecture extension: * armv8.1m.main_soft_nofp_nomve_pacret_bti[_exn_rtti] * armv8.1m.main_hard_fp_nomve_pacret_bti[_exn_rtti] * armv8.1m.main_hard_fpdp_nomve_pacret_bti[_exn_rtti] * armv8.1m.main_hard_nofp_mve_pacret_bti[_exn_rtti] Unfortunately, qemu currently doesn't provide support for PACBTI-M, which means these variants can't be fully tested at runtime level. Since the PACBTI-M instructions are NOP-copatible, we can use an arvm8.1m.main CPU as a workaround to ensure that the library code still works correctly regardless of branch protection.
* Added commands to set CC and CXX variables for current execution environment for example build commands list --------- Co-authored-by: Timur <[email protected]> Co-authored-by: timur.khasanshin <[email protected]>
…#473) This adds the `fenv` and `math_errhandling` tests to the set of disabled tests for the PACBTI-M library variants, introduced by ARM-software#467. These tests were already disabled for the previously existing variants, due to a known issue in compiler-rt where the floating point exceptions are not set correctly for computations on types implemented in software.
One of our xfailed tests has been fixed, and the patch adding the xfail comment was causing a conflict.
Now that the limits.h issue [^1] has been resolved in picolibc upstream [^2], we can go back to its latest HEAD. [^1]: Commit hash cdc4fc3 [^2]: picolibc/picolibc#810
The instruction set (ARM/Thumb) is included in the triple, but doesn't matter for library selection because the ISA can be switched on any function call. This adds match blocks to multilib.yaml to treat thumb triples as if they were arm triples for library selection. This will also be needed for AArch32 v8-A and v8-R, but we don't have any libraries for them yet.
This is a GCC name for v7-A with the virtualisation extension, so for library selection we can treat it the same as v7-A.
This patch failed to apply for me in a manual `git am`, because the upstream addition of `ADDITIONAL_COMPILE_FLAGS` was just close enough to our change to confuse git. Regenerated that hunk. Now it still makes the same change, just without complaining.
The only differece between v7-M and v7E-M is the DSP extension, which is not useful for the C/C++ libraries, so we can simplify things by just having v7-M builds. This fills in a gap of (v7-M, fpv4-sp-d16, hard-float), which we did not previously have a valid library for.
This will prevent C++ links from breaking when llvm/llvm-project#101259 lands, as that removes -lc++abi and -lunwind from the linker command-line.
…M-software#562) This patch restructures the top-level CMake script in order to split building the library variants into a separate CMake project. This better encapsulates the code to build the variants, and untethers it from the toolchain builds so that libraries can be developed and tested without having the rebuild the entire toolchain. Similarly, a new subproject has been created to build a multilib set of library variants, using a selected C library. A simple JSON format has been used to define the flags and variants, which are then built and collected into the appropriate layout.
…re#571) A number of corrections to the recent CMake changes: * Two scripts arguments were renamed during review, but the cmake steps calling the script weren't updated. * Our downstream test targets such as check-package-llvm-toolchain target require the path to an external llvm-lit in order to run the tests. * The variables to enable exceptions and RTTI are now cached and the names are capitalized, but CMake arguments needed updating to reflect this. * The PREBUILT_TARGET_LIBRARIES option is meant to disable the building of all libraries, on the assumption that pre-built libraries will be copied from another location. The functionality was accidentally removed, but has now been returned. * CMake automatically expands a string containing semi-colons to a list, but the ExternalProject argument must also be a semi-colon separated list. To get around this, convert the LLVM_TOOLCHAIN_LIBRARY_VARIANTS string to a comma separated list, since the LIST_SEPARATOR option will reinterpret this as a semi-colon list. * Previously, FVP testing was skipped if a FVP install was not available. However, this now throws a configuration error if testing relies on FVPs. But if you specifically want to test with FVPs, it is desirable to have the configuration error warn you with an if the install cannot be found. To resolve this, an additional option has been added to explicitly enable FVP testing, defaulting to OFF. This will override any settings from the variant JSON, so that the default case remains skipping FVP testing.
Now that our downstream picolibc changes are in the form of multiple patches in a subdirectory, it's better to apply them with 'git am' so they turn into separate commits, instead of 'git apply', the same way we do it for llvm-project.
…e#575) The suggested build commands left out the usual `cd` after `mkdir`, so that if you followed them literally, you'd create an `arm-runtimes` or `arm-multilib` directory and then ignore it completely and put all your build debris at the level above it. Also, trying to follow the instructions, I was confused by the semantics of `LLVM_BINARY_DIR`: given the name, I instinctively pointed it at the actual `bin` directory, but it turned out I should have aimed one level up. Added some clarifying text, with particular reference to using a full build of this toolchain for a standalone build of one library. While I'm here, removed some apparently accidental physical tabs, so that all the continuation lines in the long cmake commands line up.
This allows picolibc tests to mark themselves as "skipped" by returning
the special exit code 77. Previously that didn't work in an FVP test
run, so those tests are marked as failed rather than skipped.
In particular, test-sprintf-percent-n was affected, because we build
picolibc in a mode that doesn't support printf("%n"), and that test knew
it and was trying to return "skipped".
Background: Fast Models does support the SYS_EXIT_EXTENDED semihosting
request, which allows specifying an exit status. But it doesn't support
the ":semihosting-features" pseudo-file that advertises support for
non-default semihosting features including that one. So picolibc wrongly
believes that SYS_EXIT_EXTENDED doesn't work, and doesn't try it.
This commit works around that lack of support by simply creating a
_real_ file called ":semihosting-features" in the directory where the
model will run. Then picolibc does find it, and learns from it that it's
allowed to SYS_EXIT_EXTENDED.
This makes test-sprintf-percent-n stop failing. It also means we can
remove the semihost-exit-extended test from the list of tests specially
disabled on FVP runs, because now it works.
…est machine (ARM-software#574) The current picolibc changes can't handle fvp being set as the test machine for anything other than aarch64. If fvp isn't set, it defaults to qemu, which works for now because the ARM fvp doesn't require anything special. However, when adding new aarch64 big-endian variants, one of the picolibc patches includes test-machine as an option in the Meson build scripts. Consequently, the library's CMake needs to set this option using -Dtest-machine=${TEST_EXECUTOR}. This means that for the armv8.1m library variant,we are explicitly passing the FPU, which can cause the error fvp: requested test machine not found since fvps are only defined for the aarch64 case.
This patch add new aarch64 big endian library variants. 1. aarch64_be 2. aarch64_be_exn_rtti QEMU's doesn't have big-endian support in the release binaries. so need to use FVPs to test those libraries.
Before this patch, running get_fvps.sh --non-interactive would make a
weirdly named directory in the user's home, with a name like "". or "."
(depending on circumstances), with the crypto plugins installed in it.
Now --non-interactive installs the plugins under fvp/install in your git
checkout of this repository, which is where they should have been.
The problem was because of this construction in get_fvps.sh:
INSTALLER_FLAGS_CRYPTO="... --basepath \"$(dirname \"$0\")\""
which has multiple shell errors. Firstly, the \" around $0 are taken
literally, so dirname is handed a path beginning with a double quote,
which propagates into the path it returns. Secondly, the \" around
$(dirname) are also literal, but _not_ reprocessed when
$INSTALLER_FLAGS_CRYPTO is expanded on to the installer command line
(because quote processing happens before variable expansion). The net
effect is to give setup.bin a --basepath directory containing lots of
confusing double quotes. For extra confusion, setup.bin appears to
change directory to $HOME before interpreting that path, so you don't
even get a strangely named directory under the FVP install directory –
it appears in your home dir instead.
Since this script already commits to bash rather than plain POSIX sh,
the sensible fix for the quoting issues is to use a bash array variable
to accumulate the extra arguments, and expand it via
"${INSTALLER_FLAGS_CRYPTO[@]}".
Also, the --basepath will need to be absolute rather than relative, to
avoid confusion when setup.bin changes directory to $HOME. The easiest
approach to _that_ is to use the fact that we were already issuing a `cd
$(dirname "$0")` command to change into the fvp directory: do that
earlier, and then we can use `$PWD` to retrieve the absolute path.
This fixes --non-interactive. But another problem is that if you run
setup.bin interactively, you'll have to enter the pathname by hand, and
we don't give the user any guidance on what path to enter. So I've also
updated the docs to explain the interactive installation more fully.
This is all very error-prone, so I've also extended run_fvp.py so that
it actually finds and loads the crypto plugin. That way, if it hasn't
been installed in the right place, we find that out early, and can
figure out what went wrong.
(Finally, in this commit, I've also fixed a spelling error in one of the
script's internal variables: CORSTONE, not CORSONE.)
…#553) This adds the following library: - armv7a_soft_nofp_strictly_aligned - armv7a_soft_nofp_strictly_aligned_exn_rtti
This introduces new library variants for aarch64 to support targets where unaligned memory accesses are disabled. To enable these changes a patch to picolibc was required, ensuring the assembly implemenation that relies on unaligned accesses is not used when __ARM_FEATURE_UNALIGNED is not defined. Co-authored by: @pratlucas ( (ARM-software#567) --------- Co-authored-by: Lucas Prates <[email protected]>
…ftware#578) Rename the variant name from strictly_aligned to strictalign inorder to match with the corresponding aarch64 variant for -mno-unaligned-access.
Every `CMakeLists.txt` that includes `fetch_llvm.cmake` must also call `find_package(Python3)`, because `fetch_llvm.cmake` tries to run `patch_llvm.py` by prefixing it with the Python interpreter name. The CMakeLists in `arm-multilib` wasn't looking for Python at all, and the one in `arm-runtimes` was looking for it _after_ including `fetch_llvm.cmake`. Also, `fetch_picolibc.cmake` was not reliably looking in the correct directory for its patches: it was using a path relative to `CMAKE_CURRENT_SOURCE_DIR`, which varies depending which CMakeLists it's invoked by. It should be relative to `CMAKE_CURRENT_LIST_DIR`, as the corresponding llvm and newlib paths already are.
The cmake scripts to fetch content such as llvm-project and picolibc can be called by both the top-level project as well as the sub-projects. To prevent the content being checked out and patched repeatedly, the FETCHCONTENT_SOURCE_DIR_<PROJECT> variables should be defined as these will override the declared source on any subsequent calls. These variables are already passed down from top-level to sub-project, however the variables themselves are never defined unless configured by a user. This patch addresses this by setting the variables inside the fetch scripts as soon as the fetch content is declared.
…#581) The `test/multilib/armv7a.test` was looking for the incorrect multilib directory output when checking the behaviour for the strict align library variant. This patch fixes this issue.
Previously FVP testing was enabled or disabled implicitly based on whether an FVP install could be found. This has since been changed to a specific CMake option, however the documentation fails to mention this. The build instructions have now been updated with the new CMake option.
Since QEMU is needed to run tests with QEMU, if the program cannot be found the build should stop and an error generated.
) To simplify patching, the script used for the LLVM repo can be expanded for use on all repos. This also updates the newlib patch to use the same format as llvm-project and picolibc; using git's patch format means the .patch file is compatible with both git am and git apply, whereas a simple diff only works with git apply. This also adds an option to control the patch method used by the script, since using git am may be preferable to the default git apply. The script already supports the option, so all that is needed to pass down the selection from the CMake cache if present. The --3way option has been removed as a default when using --apply, since it can leave conflict markers in place which will not get detected by the script or CMake and will lead to a compilation error. Instead, an option to the script has been added to use --3way. Since --check --apply --3way does not return an error code when the patch is valid but contains conflicts, the --restore_on_fail is now marked as incompatible with that combination.
…re#587) The additional_cmake_args variable is used to override the defaults and disable testing so that it can be controlled by the higher-level CMake project. But the variable is currently set only when the tests should be disabled. If the tests do not need to be disabled, the value is not updated and the version from the previous iteration of the loop will be used, potentially incorrectly disabling the tests. To fix this, clear the value on every loop if not needed.
Testing with QEMU is implicitly enabled by default, but configuring the project to run with tests requires QEMU. As testing with QEMU is considered optional, it should be possible to disable tests when QEMU is not available or testing is not needed. Since there is already an option for controlling FVP testing, this patch adds a similar option for QEMU, ENABLE_QEMU_TESTING, which instead defaults to ON. This can then be turned off when QEMU is not installed, so that configurations where QEMU is expected can suitably detect and show an error when it is missing.
Build a soft float multilib variant for aarch64 supporting targets without an FPU. Authored by Keith Packard and Victor Campos.
- Make the variants strictly aligned - Create new variant for big endian - Create new variant for exceptions and RTTI
…e#590) This deletes most of the previous patch 0002 (add bootcode for AArch64 FVPs), because it's now upstream in picolibc itself. The remaining vestige, defining picocrt_machines in the AArch32 picolibc build configuration, remains necessary until we conditionalise our definition of `test-machine` in our own builds.
…#591) Those two variants have `-fno-exceptions -fno-rtti` in the configuration file that specifies options for building the library. None of the other library variants is built with this flags, not even the other variants intended for use in _applications_ without exceptions and RTTI. If you try to build libc++abi with those flags, it fails to build, because `private_typeinfo.cpp` is unconditionally included in the library and unconditionally uses `dynamic_cast`. (And I assume that it's OK to use `dynamic_cast` in that part of libc++abi if the application will never use RTTI, because then it won't include that object file in the first place.)
…re#592) LLVM-libc does not currently support no-neon soft float on AArch64 targets. To prevent building these until support is available, this patch adds an option to explicitly set which C libraries a variant can be built with. If the option is absent, the default is to build with all library options.
The libraries_supported property is a comma separated list, but CMake uses semi-colons to understand lists. Replacing the character will allow CMake to check which libraries are set within.
…-software#594) This patch fixes a few issues in the AArch64 nofp variants: - AArch64 nofp variants without exceptions and RTTI must have higher priority The presence of `-fno-exceptions` and `-fno-rtti` in the driver invocation causes matching with two variants: the one with RTTI/Exceptions and the one without. In the multilib system, the last match is the one that wins, therefore in the JSON file we must list the variant without RTTI/Exceptions *after* the one with. - Enable exceptions and RTTI in the build of aarch64 nofp variants supposed to have them
ARM-software#599) Update the patch file with upstream changes of test_demangle.pass.cpp
This patch fixes the multilib flags required to select the AArch64 soft nofp variants. The `+nofp` and `+nosimd` arch extensions are specified in the `-march` or `-mcpu` command line options. Before this patch, the multilib flags specified this part as `-march=armv8-a+nofp+nosimd`, however this does not work for any architecture greater than v8, including point releases, nor does it for any `-mcpu` value whose underlying architecture is greater than v8. In the former case because any point release would lead to a regular expression mismatch; and in the latter, `-mcpu` option processing introduces extra architecture extensions in the string. Hence the correct way is to match agains `-march=armvX+nofp` and `-march=armvX+nosimd`. These two multilib flags are inserted by our implementation of `multilib.yaml` when `+nofp` and `+nosimd` are present anywhere in the `-march=` or `-mcpu=` values, respectively.
This patch was merged in upstream picolibc: picolibc/picolibc#895
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
15 participants
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.
No description provided.