Page MenuHomePhabricator

[WebAssembly] Use the new crt1-command.o if present.
ClosedPublic

Authored by sunfish on Oct 12 2020, 2:57 PM.

Details

Summary

If crt1-command.o exists in the sysroot, the libc has new-style command
support, so use it.

Diff Detail

Unit TestsFailed

TimeTest
4,200 mswindows > Clang-Unit.DirectoryWatcher/_/DirectoryWatcherTests_exe::DirectoryWatcherTest.AddFiles
Note: Google Test filter = DirectoryWatcherTest.AddFiles [==========] Running 1 test from 1 test case.
3,990 mswindows > Clang-Unit.DirectoryWatcher/_/DirectoryWatcherTests_exe::DirectoryWatcherTest.DeleteFile
Note: Google Test filter = DirectoryWatcherTest.DeleteFile [==========] Running 1 test from 1 test case.
4,120 mswindows > Clang-Unit.DirectoryWatcher/_/DirectoryWatcherTests_exe::DirectoryWatcherTest.ModifyFile
Note: Google Test filter = DirectoryWatcherTest.ModifyFile [==========] Running 1 test from 1 test case.

Event Timeline

sunfish created this revision.Oct 12 2020, 2:57 PM
sunfish requested review of this revision.Oct 12 2020, 2:57 PM

Why not just use crt1.o in both cases? If you are going to prefer crt1-command.o in all cases then a toolchain would have no reason to ever ship crt1.o would it? (since it would always be ignored?)

It's to ensure that older LLVM works with newer WASI libc, and newer clang works with older WASI libc. New-style commands require [lld support]. We can assume that if clang is updated, lld has the requisite support.

That said, I'm open to other ideas here.

[lld support]: https://github.com/llvm/llvm-project/commit/6cd8511e5932e4a53b2bb7780f69489355fc7783

It's to ensure that older LLVM works with newer WASI libc, and newer clang works with older WASI libc. New-style commands require [lld support]. We can assume that if clang is updated, lld has the requisite support.

That said, I'm open to other ideas here.

[lld support]: https://github.com/llvm/llvm-project/commit/6cd8511e5932e4a53b2bb7780f69489355fc7783

Certainly seems like its would be better if would find a way to keep the crt1.o name if possible.

I cant't remember now what the old and new crt1.o looks like... but maybe we can find way for new WASI libc to detect if the new linker is being used? Perhaps using weak symbol references in crt1.o?

If that isn't possible then perhaps some kind of install script for wasi-libc that can install one or the other?

I don't see a way to do this with weak symbols, and an install script would be yet-another moving part that we'd have to make on end-user systems.

How about this: once we reach a point where we don't support the old LLVM anymore, libc can make crt1.o be the same as crt1-command.o, and then once we're sure we don't support libc old than that, we can switch LLVM back to crt1.o :-).

sbc100 accepted this revision.EditedFeb 11 2021, 10:52 AM

Ok so we can see this as an interim thing. I think I'm OK with that. Could you add a comment about that, or at least say why we want to support both the old crt1 and the new crt1-command at the same time (i.e. so that wasi-libc can easily suppot both old and new llvm right?)

This revision is now accepted and ready to land.Feb 11 2021, 10:52 AM
This revision was landed with ongoing or failed builds.Feb 11 2021, 2:44 PM
This revision was automatically updated to reflect the committed changes.