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.

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

Makefile: Add flags for cross-compiler LLVM target #685

Closed

Conversation

mariasfiraiala
Copy link
Contributor

@mariasfiraiala mariasfiraiala commented Dec 9, 2022

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.pl on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): AArch64
  • Platform(s): kvm
  • Application(s): N/A

Additional configuration

make CC=clang CROSS_COMPILE=~/toolchains/gcc-arm-11.2-2022.02-x86_64-aarch64-none-elf/bin/aarch64-none-elf-

Description of changes

If one wishes to compile an app using clang, for instance, they would have to choose the Custom cross-compiler LLVM target option from the make build system, and manually set the flags.

This commit adds flags needed for cross-compiling with clang for AArch64.
Instead of the extra step of filling in the cross-compiler in the Custom cross-compiler LLVM target field, we harcode it to be aarch64-none-elf, which with clang, works like a charm (it doesn't get a trap due to the emutls issue - unikraft/docs#153).

Fixes: #921

Signed-off-by: Maria Sfiraiala maria.sfiraiala@gmail.com

@mariasfiraiala mariasfiraiala requested a review from a team as a code owner December 9, 2022 21:18
@mariasfiraiala mariasfiraiala added kind/enhancement New feature or request arch/arm64 labels Dec 9, 2022
@unikraft-bot unikraft-bot added the area/makefile Part of the Unikraft Makefile build system label Dec 9, 2022
@nderjung nderjung added this to the v0.12.0 (Epimetheus) milestone Dec 13, 2022
Copy link
Member

@michpappas michpappas left a comment

Choose a reason for hiding this comment

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

@mariasfiraiala I think we should be striving for better clang integration, if possible one that does not depend on a GCC toolchain, but rather on clang, ldd, and binutils. If that is not possible, we should still update the build system so that the user is not required to manually set so many env vars.

If you decide to pursue this, here are some useful resources:

@razvand razvand requested review from razvand and removed request for a team January 13, 2023 12:53
@razvand razvand assigned skuenzer and unassigned razvand Jan 13, 2023
@razvand razvand changed the title Makfile: Add flags for cross-compiler LLVM target Makefile: Add flags for cross-compiler LLVM target May 1, 2023
@StefanJum
Copy link
Member

@mariasfiraiala @michpappas @razvand, should we go ahead with some variation of this PR until we get a better clang integration? It would be nice to have clang support on arm64 without needing to pull PRs, and I think better clang integration will take quite some time.

I think we can do something very transparent to the user, like check if the user selected any LLVM target in the menuconfig, if not (and the used CC is clang) just add a default one (I've been testing everything with --targtet=aarch64-linux-none-gnu so far).

The only env variable that should be set is the CROSS_COMPILE that should point to the toolchain path, everything else is set based on that, but adding the --target flag works for me without even setting the CROSS_COMPILE variable.

@StefanJum
Copy link
Member

@mariasfiraiala following the discussion here we should update this PR to just add the aarch64-none-elf target to CFLAGS, CXXFLAGS, ASFLAGS etc if arch/arm64 is selected in the configuration menu and CC==clang.

We should eventually just remove the whole CONFIG_LLVM_TARGET_ARCH, but it looks like Rust support might need it.

Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

Thank you @mariasfiraiala. Besides the comment, I would replace flags with something more specific (like target flags?) in the commit message / description.

Makefile Outdated Show resolved Hide resolved
@unikraft-bot
Copy link
Member

Checkpatch passed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request and it all looks good!

SHA commit checkpatch
2c2b28f Makfile: Add flags for `clang` with `AArch64`

This commit adds target flags needed for cross-compiling with `clang`
for`AArch64`.
They are guarded by a condition which checks if these flags
are really needed (the compiler is `clang` and the architecture is
`AArch64`).

Signed-off-by: Maria Sfiraiala <maria.sfiraiala@gmail.com>
@mariasfiraiala
Copy link
Contributor Author

Done @StefanJum, please take another look!

@mariasfiraiala
Copy link
Contributor Author

Just to have it here for documentation purposes, there is a small issue with the TLS phdrs, that rarely shows up, a better description of it being on the discord channel.

Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

Thank you @mariasfiraiala, all good.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

@michpappas
Copy link
Member

Thanks @mariasfiraiala 🚀

Approved-by: Michalis Pappas michalis@unikraft.io

@unikraft-bot unikraft-bot added the ci/merged Merged by CI label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch/arm64 area/makefile Part of the Unikraft Makefile build system ci/merged Merged by CI kind/enhancement New feature or request
Projects
Status: Done
Status: Done!
Development

Successfully merging this pull request may close these issues.

Support cross-compiling with clang
7 participants