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

Upgrade to LLVM 16 & C++17, and bundle libc++ on Linux #87190

Merged
merged 74 commits into from
Jul 3, 2023

Conversation

directhex
Copy link
Member

@directhex directhex commented Jun 6, 2023

This includes building against, linking, and shipping our own libc++ in lieu of system libstdc++, for LLVM builds. This is because LLVM 16 has a hard requirement on C++17 - our existing code assumes C++11

@directhex directhex added blocked Issue/PR is blocked on something - see comments area-Codegen-LLVM-mono labels Jun 6, 2023
@ghost ghost assigned directhex Jun 6, 2023
@directhex directhex requested a review from vargaz as a code owner June 9, 2023 14:26
@directhex directhex changed the title [DO NOT MERGE] Upgrade to LLVM 16 Upgrade to LLVM 16 & C++17, and bundle libc++ on Linux Jun 30, 2023
Looks like someone made LLVM too good, and broke the build
@filipnavara
Copy link
Member

Shrink the min Dwarf warning limit.

Sorry. I plead guilty.

@directhex
Copy link
Member Author

Shrink the min Dwarf warning limit.

Sorry. I plead guilty.

Plz stop fixing bugs, those were load-bearing bugs

@directhex
Copy link
Member Author

This affects folks on the NativeAOT side too. Maybe @agocke, @jkotas are the right people to loop in?

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

:shipit:

@jkotas
Copy link
Member

jkotas commented Jul 1, 2023

I am wondering whether it would be better to link libc++.so and libc++abi.so.1 statically into libobjwriter.so, so there is no risk of it colliding with system libraries that may depend on C++ runtime.

@agocke
Copy link
Member

agocke commented Jul 2, 2023

Agreed, statically linking libc++ would be fine with me.

Also, the warning shrink looks good to me, that may also have been impacted by dotnet/llvm-project#444

@filipnavara
Copy link
Member

Also, the warning shrink looks good to me, that may also have been impacted by dotnet/llvm-project#444

Didn't mean to steal credit from you. 😅 Anyway, we are (back) at zero warnings with llvm-dwarfdump.

@agocke
Copy link
Member

agocke commented Jul 2, 2023

Nope, no worries, you've done tons in this space and deserve the credit. I was just pointing out that, if that change hadn't yet been integrated, it might cause us to lower the count once again. I have one more change that will lower the count again on Linux (for some reason Linux still seems to have errors, but Mac doesn't).

@filipnavara
Copy link
Member

for some reason Linux still seems to have errors, but Mac doesn't

They ship different dwarfdump. macOS ships the LLVM one, which reports less errors but supports proprietary Apple sections (which were later replaced by DWARF5 standard).

@directhex
Copy link
Member Author

If someone wants to run experiments on static linking vs dynamic, they're welcome to - though the place to start that work is in the llvm-project repo where objwriter is generated. In my testing, via use of rpath, dynamic linking is working at least well enough for CI.

I don't think static linking is a realistic option for Mono - the bloat on 20-30 clang/llvm binaries would push us past the Nupkg size limit again

@directhex directhex merged commit 087651e into dotnet:main Jul 3, 2023
175 checks passed
@directhex directhex removed the blocked Issue/PR is blocked on something - see comments label Jul 3, 2023
@jkotas
Copy link
Member

jkotas commented Jul 3, 2023

This introduced build break in Android extra-platforms Mono legs:

https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/328385/logs/223

2023-07-03T16:59:17.6848371Z   CMake Error at /usr/share/cmake-3.21/Modules/CMakeTestCXXCompiler.cmake:62 (message):
2023-07-03T16:59:17.6856690Z     The C++ compiler
2023-07-03T16:59:17.6856932Z   
2023-07-03T16:59:17.6857259Z       "/usr/bin/c++"
2023-07-03T16:59:17.6857646Z   
2023-07-03T16:59:17.6857894Z     is not able to compile a simple test program.
2023-07-03T16:59:17.6858043Z   
2023-07-03T16:59:17.6858208Z     It fails with the following output:
2023-07-03T16:59:17.6858340Z   
2023-07-03T16:59:17.6858549Z       Change Dir: /__w/1/s/artifacts/obj/mono/android.arm64.Release/cross/CMakeFiles/CMakeTmp
2023-07-03T16:59:17.6858948Z       
2023-07-03T16:59:17.6859436Z       Run Build Command(s):/usr/bin/make -f Makefile cmTC_e98b0/fast && /usr/bin/make  -f CMakeFiles/cmTC_e98b0.dir/build.make CMakeFiles/cmTC_e98b0.dir/build
2023-07-03T16:59:17.6859987Z       make[1]: Entering directory '/__w/1/s/artifacts/obj/mono/android.arm64.Release/cross/CMakeFiles/CMakeTmp'
2023-07-03T16:59:17.6860286Z       Building CXX object CMakeFiles/cmTC_e98b0.dir/testCXXCompiler.cxx.o
2023-07-03T16:59:17.6861105Z       /usr/bin/c++   -I/__w/1/s/artifacts/obj/mono/android.arm64.Release/llvm//x64/include/c++/v1 -L/__w/1/s/artifacts/obj/mono/android.arm64.Release/llvm//x64/lib -stdlib=libc++  -o CMakeFiles/cmTC_e98b0.dir/testCXXCompiler.cxx.o -c /__w/1/s/artifacts/obj/mono/android.arm64.Release/cross/CMakeFiles/CMakeTmp/testCXXCompiler.cxx
2023-07-03T16:59:17.6862084Z       c++: error: unrecognized command-line option ‘-stdlib=libc++’
2023-07-03T16:59:17.6862397Z       make[1]: *** [CMakeFiles/cmTC_e98b0.dir/build.make:78: CMakeFiles/cmTC_e98b0.dir/testCXXCompiler.cxx.o] Error 1
2023-07-03T16:59:17.6862993Z       make[1]: Leaving directory '/__w/1/s/artifacts/obj/mono/android.arm64.Release/cross/CMakeFiles/CMakeTmp'
2023-07-03T16:59:17.6863231Z       make: *** [Makefile:127: cmTC_e98b0/fast] Error 2
2023-07-03T16:59:17.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants