-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix native triple for Raspberry Pi 5 to fix other.test_cmake_compile_features #25028
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
Conversation
3407b15
to
d06104b
Compare
test/clang_native.py
Outdated
|
||
def get_native_triple(): | ||
# On Raspberry Pi 5, the target triple for native compilation must exactly | ||
# match this. E.g. "arm64-linux" will not work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you say is doesn't work, what does that mean exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The system fails to build a test program: http://clbri.com:8010/#/builders/9/builds/77
-- Build files have been written to: /tmp/emtest_asq9fsuv/emscripten_test_other__qesubir
['cmake', '-DCMAKE_C_COMPILER=/home/pi64/emscripten_buildbot/worker_root/rpi-5b-8gb-debian12-bookworm/emscripten_linux_arm64/build/llvm/git/build_main_64/bin/clang', '-DCMAKE_C_FLAGS=--target=arm64-linux', '-DCMAKE_CXX_COMPILER=/home/pi64/emscripten_buildbot/worker_root/rpi-5b-8gb-debian12-bookworm/emscripten_linux_arm64/build/llvm/git/build_main_64/bin/clang++', '-DCMAKE_CXX_FLAGS=--target=arm64-linux', '/home/pi64/emscripten_buildbot/worker_root/rpi-5b-8gb-debian12-bookworm/emscripten_linux_arm64/build/emscripten/main/test/cmake/stdproperty']
test_cmake_emscripten_version (test_other.other.test_cmake_emscripten_version) ... ok (8.15s)
test_cmake_compile_features (test_other.other.test_cmake_compile_features) ... skipped 'requested to be skipped'
configure: cmake /home/pi64/emscripten_buildbot/worker_root/rpi-5b-8gb-debian12-bookworm/emscripten_linux_arm64/build/emscripten/main/test/cmake/static_lib -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DEMSCRIPTEN_FORCE_COMPILERS=OFF -DCMAKE_TOOLCHAIN_FILE=/home/pi64/emscripten_buildbot/worker_root/rpi-5b-8gb-debian12-bookworm/emscripten_linux_arm64/build/emscripten/main/cmake/Modules/Platform/Emscripten.cmake -DCMAKE_CROSSCOMPILING_EMULATOR=/home/pi64/emscripten_buildbot/worker_root/rpi-5b-8gb-debian12-bookworm/emscripten_linux_arm64/build/node/22.16.0_64bit/bin/node
CMake Error at /usr/share/cmake-3.25/Modules/CMakeTestCCompiler.cmake:70 (message):
The C compiler
"/home/pi64/emscripten_buildbot/worker_root/rpi-5b-8gb-debian12-bookworm/emscripten_linux_arm64/build/llvm/git/build_main_64/bin/clang"
is not able to compile a simple test program.
It fails with the following output:
Change Dir: /tmp/emtest_8xglffu9/emscripten_test_other_cr05mw78/build_native/CMakeFiles/CMakeScratch/TryCompile-7aMe3V
Run Build Command(s):/usr/bin/gmake -f Makefile cmTC_ba609/fast && /usr/bin/gmake -f CMakeFiles/cmTC_ba609.dir/build.make CMakeFiles/cmTC_ba609.dir/build
gmake[1]: Entering directory '/tmp/emtest_8xglffu9/emscripten_test_other_cr05mw78/build_native/CMakeFiles/CMakeScratch/TryCompile-7aMe3V'
Building C object CMakeFiles/cmTC_ba609.dir/testCCompiler.c.o
/home/pi64/emscripten_buildbot/worker_root/rpi-5b-8gb-debian12-bookworm/emscripten_linux_arm64/build/llvm/git/build_main_64/bin/clang --target=arm64-linux -MD -MT CMakeFiles/cmTC_ba609.dir/testCCompiler.c.o -MF CMakeFiles/cmTC_ba609.dir/testCCompiler.c.o.d -o CMakeFiles/cmTC_ba609.dir/testCCompiler.c.o -c /tmp/emtest_8xglffu9/emscripten_test_other_cr05mw78/build_native/CMakeFiles/CMakeScratch/TryCompile-7aMe3V/testCCompiler.c
Linking C executable cmTC_ba609
/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_ba609.dir/link.txt --verbose=1
/home/pi64/emscripten_buildbot/worker_root/rpi-5b-8gb-debian12-bookworm/emscripten_linux_arm64/build/llvm/git/build_main_64/bin/clang --target=arm64-linux CMakeFiles/cmTC_ba609.dir/testCCompiler.c.o -o cmTC_ba609
/usr/bin/ld: cannot find crtbeginS.o: No such file or directory
/usr/bin/ld: cannot find -lgcc: No such file or directory
/usr/bin/ld: cannot find -lgcc_s: No such file or directory
clang: error: linker command failed with exit code 1 (use -v to see invocation)
when it doesn't find those three libraries.
test/clang_native.py
Outdated
def get_native_triple(): | ||
# On Raspberry Pi 5, the target triple for native compilation must exactly | ||
# match this. E.g. "arm64-linux" will not work. | ||
if os.path.isdir('/usr/lib/aarch64-linux-gnu'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existence of this directory alone, I fear, does not mean the how is arm64. This directory could exist an an x86 host and contain libraries for cross compiling for aarch64.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't cross compilation toolchains typically use their own --sysroot= directories under the installation of the toolchain, instead of "merging" the cross compilation toolchains into the system ones?
Nevertheless, even if that is true, it is still a bit of a hypothetical. If we think of the pool of users on Raspberry Pi's vs the pool of users that could have such a specific installation, the native Raspi users are probably two orders of magnitude more common?
I spent quite a while with Google and GPT trying to come up with a "correct" way to identify the target triple when building on Raspberry Pi, but couldn't find one that didn't involve grepping grep /proc/cpuinfo, and that has the same issue that other systems could use Broadcom CPUs as well. If you have a better recommendation, I can definitely adapt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debian multi-arch support works this way: https://wiki.debian.org/Multiarch/HOWTO
Packges for different architectures appear in /usr/lib/<target>-linux-gnu
. You can see this on most x86_64 systems where you will see both /usr/lib/i386-linux-gnu
and /usr/lib/x86_64-linux-gnu
, although you can install any cross architecture you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I would suggest that you add EMTEST_LACKS_NATIVE_CLANG=1
to your raspberry Pi CI, since this only effect a tiny subset of tests.
In the mean time I think we should try to completely move away from using this "native clang" feature of the test suite since it causes a lot of headaches and only benefits a tiny number of tests. Hopefully we can find alternative ways to implement these tests or mark them as linux-only (where the host toolchain config is trivial).
Another reason to do this is that we are have long considered dropping the x86/host support from the version of clang we ship with emsdk, which would mean that emsdk toolchain clang could not run these tests.
CC @dschuff who has been looking at remove the x86 backend from the emsdk binaries, or at least changing the default to wasm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I would suggest that you add EMTEST_LACKS_NATIVE_CLANG=1 to your raspberry Pi CI, since this only effect a tiny subset of tests.
I think we should try to completely move away from using this "native clang" feature
You are saying that because there might somewhere exist a hypothetical user out there who might be using this Debian multi-arch feature, and run Emscripten native compilation unit tests, then I, as a concrete user with this need today, should instead settle to skip running these tests.
Why do I come second to a fictional user?
How about I make an env. var. EMCC_NATIVE_COMPILATION_TRIPLE=/usr/lib/aarch64-linux-gnu
that I can set on my Raspberry Pi test box to override the returned triple here. Would that be a good middle ground?
Skipping running these tests would be disappointing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure EMCC_NATIVE_COMPILATION_TRIPLE
seems fine.
Can you call it EMTEST_XX
though..since it doesn't effect emscripten itself, only these few tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, updated to EMTEST_NATIVE_COMPILATION_TRIPLE
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, I
My preference among the ideas Sam and I we discussed the other day would be
TBH I would suggest that you add EMTEST_LACKS_NATIVE_CLANG=1 to your raspberry Pi CI, since this only effect a tiny subset of tests.
I think we should try to completely move away from using this "native clang" feature
You are saying that because there might somewhere exist a hypothetical user out there who might be using this Debian multi-arch feature, and run Emscripten native compilation unit tests, then I, as a concrete user with this need today, should instead settle to skip running these tests.
This is not about the users who want to run emscripten tests on various styles of systems; removing native architecture support from the emsdk binaries will make them smaller, benefitting all emsdk users (who surely vastly outnumber the developers trying to run emscripten tests themselves). Having good support for wasm-only clang binaries (in emscripten testsuite, and also the LLVM upstream test suite, which needs some work) will make that possible, and I think we are fairly close to it.
How about I make an env. var.
EMCC_NATIVE_COMPILATION_TRIPLE=/usr/lib/aarch64-linux-gnu
that I can set on my Raspberry Pi test box to override the returned triple here. Would that be a good middle ground?
Skipping running these tests would be disappointing.
I'm OK with this generally but it doesn't solve all the issues. For example the Intel SIMD tests require an x86 clang (not even a native one). My general preference would be to find a way to keep our test coverage without needing a clang that supports both wasm and other architectures. Maybe that means being able to pass a native compile command (rather than just flags for the primary clang under test), or something along those lines.
Also, it's already the case that not every test will run on every architecture or OS (e.g. the aforementioned SIMD tests, various nodeFS tests across architectures, as well as the ones that use this native triple feature). I don't think perfect coverage of every OS x architecture combination is necessarily needed. For example, the Intel SIMD tests don't necessarily need to run on every system; testing against a native compiler on e.g. x86-64-linux verifies that the generated code gets the correct results when run, and LLVM codegen tests that do run on every architecture verify that the generated code is the same everywhere. I think the number of tests that use native clang are few enough that we can evaluate on case-by-case basis what kind of coverage we need to make sure we are testing what we want to test.
For the 2 cases that use get_native_triple()
. I think test_native_link_error_message()
doesn't need to run on Windows (where the likelihood of accidentally mixing native and wasm objects seems very low), and an object file generated by any native compiler will do just fine. test_cmake_compile_features()
seems harder, since you need a clang with the same supported C++ features as Emscripten's, and the whole point is to make sure we update our CMake files when clang gets new C standards. Maybe we could write a test that accomplishes the same thing another way.
This is an automatic change generated by tools/maint/rebaseline_tests.py. The following (1) test expectation files were updated by running the tests with `--rebaseline`: ``` code_size/test_codesize_hello_dylink_all.json: 844375 => 844360 [-15 bytes / -0.00%] Average change: -0.00% (-0.00% - -0.00%) ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the expectations updates, the quickest / best way to rebaseline when main itsefl is out-of-date is to using "Actions" menu in github and run the "Rebaseline tests" action on the main branch.
This runs automatically each night but can also be run manually.
Thanks, hmhm, it is not enough if I rebase/merge the PR branch against main, and then run rebaseline on top of that? |
Yes, that should work, but having github do it for you means it take your local setup output of the equation (i.e. you cannot have an out-of-date emsdk) |
In this case main itself seems to have gotten out-of-sync. I normally do separate rebaseline using the Actions menu before landing any other changes land. |
…triple # Conflicts: # test/code_size/test_codesize_hello_dylink_all.json
No description provided.