Update libcxx and libcxxabi to LLVM 19.1.4 #22994
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
This was referenced Nov 23, 2024
aheejin
force-pushed
the
update_libcxx_libcxxabi_19
branch
from
8936564 to
f619917
Compare
This is our emscripten-specific configuration file and was mistakenly deleted when I ran update_libcxx.py.
This file was added as a part of LLVM 18 update (emscripten-core#21638) in emscripten-core@8d51927 and mistakenly deleted when I ran update_libcxx.py. This file was copied from https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/vendor/llvm/default_assertion_handler.in, so this also updates the file with the newest `default_assertion_handler.in`.
These Emscripten-specific files were mistakenly deleted when I ran update_libcxx.py.
https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/src/expected.cpp was added in llvm/llvm-project#87390 and this file assumes C++23 to be compiled. Apparently libc++ sources are always built with C++23 so they don't guard things against it in `src/`: llvm/llvm-project#87390 (comment) This also bumps libc++abi to C++23 because... why not
We have excluded files in https://github.com/emscripten-core/emscripten/tree/main/system/lib/libcxx/src/support/win32. This is a new file added in this directory in llvm/llvm-project#83575.
We disabled C++20 time zone support in LLVM 18 update (emscripten-core#21638): emscripten-core@df9af64 The list of source files related to time zone support has changed in llvm/llvm-project#74928, so this commit reflects it.
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid specialization for `std::basic_string`: https://en.cppreference.com/w/cpp/string/basic_string But libc++ had a base template for `basic_string` that allows any type to be passed for a long time. It looks there have been several attempts to remove this but they restored it afterwards due to some complaints, in chronological order: llvm/llvm-project@aeecef0 llvm/llvm-project@08a0faf llvm/llvm-project@e30a148 llvm/llvm-project#66153 llvm/llvm-project#72694 The last one, llvm/llvm-project#72694, eventually removed it. So `std::basic_string<unsigned_char>` is not allowed anymore.
aheejin
force-pushed
the
update_libcxx_libcxxabi_19
branch
from
812cb80 to
643050f
Compare
Not sure why but some of them decreasd by ~3%. Increases don't seem to be meaningful; they are usually ~0.3%.
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid specialization for `std::basic_string`: https://en.cppreference.com/w/cpp/string/basic_string But libc++ had a base template for `basic_string` that allowed any type to be passed for a long time. It looks there have been several attempts to remove this after which they restored it due to complaints, in chronological order: llvm/llvm-project@aeecef0 llvm/llvm-project@08a0faf llvm/llvm-project@e30a148 llvm/llvm-project#66153 llvm/llvm-project#72694 The last one, llvm/llvm-project#72694, eventually removed it. So `std::basic_string<unsigned_char>` is not allowed anymore. This removes all uses of `std::basic_string<unsigned_char>` from embind. This needs to be done to update libc++ to LLVM 19 (emscripten-core#22994). I'm uploading this as a separate PR because this removes a functionality from embind.
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid specialization for `std::basic_string`: https://en.cppreference.com/w/cpp/string/basic_string But libc++ had a base template for `basic_string` that allowed any type to be passed for a long time. It looks there have been several attempts to remove this after which they restored it due to complaints, in chronological order: llvm/llvm-project@aeecef0 llvm/llvm-project@08a0faf llvm/llvm-project@e30a148 llvm/llvm-project#66153 llvm/llvm-project#72694 The last one, llvm/llvm-project#72694, eventually removed it. So `std::basic_string<unsigned_char>` is not allowed anymore. This removes all uses of `std::basic_string<unsigned_char>` from embind. This needs to be done to update libc++ to LLVM 19 (emscripten-core#22994). I'm uploading this as a separate PR because this removes a functionality from embind.
hedwigz
pushed a commit
to hedwigz/emscripten
that referenced
this pull request
Dec 18, 2024
Only `char`, `wchar`, `char8`, `char16`, and `char32` are valid specializations for `std::basic_string`: https://en.cppreference.com/w/cpp/string/basic_string But libc++ had a base template for `basic_string` that allowed any type to be passed for a long time. It looks there have been several attempts to remove this after which they restored it due to complaints, in chronological order: llvm/llvm-project@aeecef0 llvm/llvm-project@08a0faf llvm/llvm-project@e30a148 llvm/llvm-project#66153 llvm/llvm-project#72694 The last one, llvm/llvm-project#72694, eventually removed it. So `std::basic_string<unsigned char>` is not allowed anymore. This removes all uses of `std::basic_string<unsigned char>` from embind. This needs to be done to update libc++ to LLVM 19 (emscripten-core#22994). I'm uploading this as a separate PR because this removes a functionality from embind.
aheejin
added a commit
to aheejin/emscripten
that referenced
this pull request
Dec 19, 2024
When `deterministic_paths` is set, we are currently using `-ffile-prefix-map` to produce the same path in data and debug info. In the case of absolute paths, their emscripten path is replaced with a fake path `/emsdk/emscripten`, and in the case of relative paths, all path relative to the emscripten directory is removed, so `../../system/lib/somefile.c` becomes `system/lib/somefiles.c`. https://github.com/emscripten-core/emscripten/blob/f66b5d706e174d9e5cc6122c06ea29dcd2735cd0/tools/system_libs.py#L472-L477 https://github.com/emscripten-core/emscripten/blob/f66b5d706e174d9e5cc6122c06ea29dcd2735cd0/tools/system_libs.py#L495-L501 So this does not make relative paths and absolute paths the same, which can lead to different builds depending on whether the command line uses absolute paths vs. relative ones. Currently we use relative paths when `EMCC_BATCH_BUILD` is set. And Ninja builds cannot use relative paths. This is also what was suggested in emscripten-core#23195 (comment) while discussins `__FILE__` problem in #23915. This does not change any code size tests because no code size tests happens to contain `__FILE__` at the moment. (One will be added in emscripten-core#22994)
aheejin
added a commit
that referenced
this pull request
Dec 19, 2024
#23222) When `deterministic_paths` is set, we are currently using `-ffile-prefix-map` to produce the same path in data and debug info. In the case of absolute paths, their emscripten path is replaced with a fake path `/emsdk/emscripten`, and in the case of relative paths, all path relative to the emscripten directory is removed, so `../../system/lib/somefile.c` becomes `system/lib/somefiles.c`. https://github.com/emscripten-core/emscripten/blob/f66b5d706e174d9e5cc6122c06ea29dcd2735cd0/tools/system_libs.py#L472-L477 https://github.com/emscripten-core/emscripten/blob/f66b5d706e174d9e5cc6122c06ea29dcd2735cd0/tools/system_libs.py#L495-L501 So this does not make relative paths and absolute paths the same, which can lead to different builds depending on whether the command line uses absolute paths vs. relative ones. Currently we use relative paths when `EMCC_BATCH_BUILD` is set. And Ninja builds cannot use relative paths. This is also what was suggested in #23195 (comment) while discussins `__FILE__` problem in #23195. This does not change any code size tests because no code size tests happens to contain `__FILE__` at the moment. (One will be added in #22994)
This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (11) test expectation files were updated by running the tests with `--rebaseline`: ``` code_size/embind_hello_wasm.json: 18011 => 17910 [-101 bytes / -0.56%] code_size/embind_val_wasm.json: 17198 => 16938 [-260 bytes / -1.51%] other/codesize/test_codesize_cxx_ctors1.size: 128894 => 129570 [+676 bytes / +0.52%] other/codesize/test_codesize_cxx_ctors2.size: 128343 => 129019 [+676 bytes / +0.53%] other/codesize/test_codesize_cxx_except.size: 170950 => 171248 [+298 bytes / +0.17%] other/codesize/test_codesize_cxx_except_wasm.size: 142154 => 142510 [+356 bytes / +0.25%] other/codesize/test_codesize_cxx_except_wasm_exnref.size: 144741 => 145056 [+315 bytes / +0.22%] other/codesize/test_codesize_cxx_lto.size: 121844 => 122262 [+418 bytes / +0.34%] other/codesize/test_codesize_cxx_mangle.size: 232468 => 233088 [+620 bytes / +0.27%] other/codesize/test_codesize_cxx_noexcept.size: 131701 => 132146 [+445 bytes / +0.34%] other/codesize/test_codesize_cxx_wasmfs.size: 168848 => 169407 [+559 bytes / +0.33%] Average change: +0.08% (-1.51% - +0.53%) ```
sbc100
approved these changes
Jan 3, 2025
.circleci/config.yml
Outdated
| @@ -808,7 +808,8 @@ jobs: | |||
| - run-tests-linux: | |||
| # some native-dependent tests fail because of the lack of native | |||
| # headers on emsdk-bundled clang | |||
| test_targets: "other skip:other.test_native_link_error_message" | |||
| test_targets: " | |||
| other.test_minimal_runtime_code_size_hello_embind" | |||
There was a problem hiding this comment.
I guess you can revert this file?
There was a problem hiding this comment.
Oh I thought I reverted all CI testing changes but this remained... Reverted.
This reverts commit 76a4466.
I had omitted a few changes before, but re-added all additional changes to the PR description now. |
sbc100
approved these changes
Jan 3, 2025
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 updates libcxx and libcxxabi to LLVM 19.1.4:
https://github.com/llvm/llvm-project/releases/tag/llvmorg-19.1.4
The initial update was done using
update_libcxx.pyandupdate_libcxxabi.py, and subsequent fixes were made in indidual commits. The commit history here is kind of messy because of CI testing so not all individual commits are noteworthy.Additional changes:
Build libcxx and libcxxabi with C++23: 8b0bfdf
https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/src/expected.cpp
was added in [libc++] Use availability to rely on key functions for bad_expected_access and bad_function_call llvm/llvm-project#87390 and this file assumes C++23 to be compiled. Apparently libc++ sources are always built with C++23 so they don't guard things against it in
src/: [libc++] Use availability to rely on key functions for bad_expected_access and bad_function_call llvm/llvm-project#87390 (comment)This commit also builds libc++abi with C++23 because it doesn't seem there's any downside to it.
Exclude newly added
compiler_rt_shims.cpp: 5bbcbf0We have excluded files in https://github.com/emscripten-core/emscripten/tree/main/system/lib/libcxx/src/support/win32.
This is a new file added in this directory in [libc++] Use _Complex for multiplication and division of complex floating point types llvm/llvm-project#83575.
Disable time zone support: a5f2cbe
We disabled C++20 time zone support in LLVM 18 update (Update libcxx and libcxxabi to LLVM 18.1.2 #21638): df9af64
The list of source files related to time zone support has changed in [libc++][chrono] Loads tzdata.zi in tzdb. llvm/llvm-project#74928, so this commit reflects it.
Re-add + update
__assertion_handlerfromdefault_assertion_handler.in: 41f8037This file was added as a part of LLVM 18 update (Update libcxx and libcxxabi to LLVM 18.1.2 #21638) in 8d51927 and mistakenly deleted when I ran
update_libcxx.py. This file was copied from https://github.com/llvm/llvm-project/blob/aadaa00de76ed0c4987b97450dd638f63a385bed/libcxx/vendor/llvm/default_assertion_handler.in, so this also updates the file with the newestdefault_assertion_handler.in._LIBCPP_PSTL_CPU_BACKEND_SERIAL->_LIBCPP_PSTL_BACKEND_SERIAL: 4b969c3The name changed in this update, so reflecting it on our
__config_site.Directly include
pthread.hfromemscripten/val.h: a5a76c3Due to file rearrangements happened, this was necessary.
Other things to note:
std::basic_string<unsigned_char>is not supported anymoreThe support for
std::basic_string<unsigned_char>, which was being used by embind, is removed in this version.Remove basic_string<unsigned char> from embind #23070 removes the uses from embind.libcxxabi uses
__FILE__in more places[libc++abi] Replace usage of raw assert by _LIBCXXABI_ASSERT llvm/llvm-project#80689 started to use
_LIBCXXABI_ASSERT, which uses__FILE__, inprivate_typeinfo.cpp.__FILE__macro produces different paths depending on the local directory names and how the file is given to clang in the command line, and this file is included in the result of one of our code size tests,other.test_minimal_runtime_code_size_hello_embind, which caused the result of the test to vary depending on the CI bots and how the library is built (e.g., whether by embuilder, ninja, or neither).Even though this was brought to surface during this LLVM 19 update,
__FILE__macro could be a problem for producing reproducible builds anyway. We discussed this problem in Handling __FILE__ in code size tests in CircleCI #23195, and the fixes landed in Use consistent prefix map when building deterministic system libraries #23222, Add deterministic prefix in Library.get_flags() (NFC) #23225, and Always enable deterministic_paths #23256.