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

Link LLVM tools dynamically #43577

Merged
merged 3 commits into from
Aug 4, 2017
Merged

Link LLVM tools dynamically #43577

merged 3 commits into from
Aug 4, 2017

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Jul 31, 2017

Set LLVM_LINK_LLVM_DYLIB=ON -- "If enabled, tools will be linked with
the libLLVM shared library." Rust doesn't ship any of the LLVM tools,
and only needs a few at all for some test cases, so statically linking
the tools is just a waste of space. I've also had memory issues on
slower machines with LLVM debuginfo enabled, when several tools start
linking in parallel consuming several GBs each.

With the default configuration, build/x86_64-unknown-linux-gnu/llvm
was 1.5GB before, now down to 731MB. The difference is more drastic
with --enable-llvm-release-debuginfo, from 28GB to "only" 13GB.

This does not change the linking behavior of rustc_llvm.

Set `LLVM_LINK_LLVM_DYLIB=ON` -- "If enabled, tools will be linked with
the libLLVM shared library."  Rust doesn't ship any of the LLVM tools,
and only needs a few at all for some test cases, so statically linking
the tools is just a waste of space.  I've also had memory issues on
slower machines with LLVM debuginfo enabled, when several tools start
linking in parallel consuming several GBs each.

With the default configuration, `build/x86_64-unknown-linux-gnu/llvm`
was 1.5GB before, now down to 731MB.  The difference is more drastic
with `--enable-llvm-release-debuginfo`, from 28GB to "only" 13GB.

This does not change the linking behavior of `rustc_llvm`.
@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@ishitatsuyuki
Copy link
Contributor

I'm curious what's the use case for this. There's already an option to set llvm-root.

The rustup binaries needs to be linked statically. Shared libraries can cause path conflicts too.

@cuviper
Copy link
Member Author

cuviper commented Aug 1, 2017

The use case is to reduce the resource requirements for building rustc's own LLVM. When several of the LLVM tools link in parallel, especially with debuginfo enabled, it takes a LOT of memory. The resulting binaries also take a lot of disk space. The LLVM tools are not shipped with rustc at all, so it's fine for them to just link to a locally-built libLLVM.so instead.

The rustup binaries needs to be linked statically.

I assume you mean the binaries distributed through rustup. They still are static -- this is why I mentioned that there's no change for rustc_llvm. This is where LLVM gets linked into rustc, and it will still use the same linkage as before.

@sanxiyn
Copy link
Member

sanxiyn commented Aug 3, 2017

This seems to be a clear win. @bors r+

@bors
Copy link
Contributor

bors commented Aug 3, 2017

📌 Commit 1636a2c has been approved by sanxiyn

@bors
Copy link
Contributor

bors commented Aug 3, 2017

⌛ Testing commit 1636a2c with merge a04aa70b0ddb81f12cc46946aa406a706cbff007...

@bors
Copy link
Contributor

bors commented Aug 3, 2017

💔 Test failed - status-appveyor

@kennytm
Copy link
Member

kennytm commented Aug 3, 2017

Failed to link LLVM products on MSVC. Legit

Exhibit A:

[1700/1912] Linking CXX executable bin\llvm-cxxfilt.exe
FAILED: bin/llvm-cxxfilt.exe 
cmd.exe /C "cd . && "C:\Program Files (x86)\CMake\bin\cmake.exe" -E vs_link_exe --intdir=tools\llvm-cxxfilt\CMakeFiles\llvm-cxxfilt.dir --manifests  -- C:\PROGRA~2\MI0E91~1.0\VC\bin\amd64\link.exe /nologo tools\llvm-cxxfilt\CMakeFiles\llvm-cxxfilt.dir\llvm-cxxfilt.cpp.obj tools\llvm-cxxfilt\CMakeFiles\llvm-cxxfilt.dir\__\__\resources\windows_version_resource.rc.res  /out:bin\llvm-cxxfilt.exe /implib:lib\llvm-cxxfilt.lib /pdb:bin\llvm-cxxfilt.pdb /version:0.0  /machine:x64 /STACK:10000000 /INCREMENTAL:NO /subsystem:console  lib\LLVM.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."
LINK : fatal error LNK1181: cannot open input file 'lib\LLVM.lib'
LINK failed. with 1181

Exhibit B:

[1698/1912] Linking CXX shared library bin\LTO.dll
FAILED: bin/LTO.dll lib/LTO.lib 
cmd.exe /C "cd . && "C:\Program Files (x86)\CMake\bin\cmake.exe" -E vs_link_dll --intdir=tools\lto\CMakeFiles\LTO.dir --manifests  -- C:\PROGRA~2\MI0E91~1.0\VC\bin\amd64\link.exe /nologo tools\lto\CMakeFiles\LTO.dir\LTODisassembler.cpp.obj tools\lto\CMakeFiles\LTO.dir\lto.cpp.obj tools\lto\CMakeFiles\LTO.dir\__\__\resources\windows_version_resource.rc.res  /out:bin\LTO.dll /implib:lib\LTO.lib /pdb:bin\LTO.pdb /dll /version:0.0 /machine:x64 /INCREMENTAL:NO  /DEF:"C:/projects/rust/build/x86_64-pc-windows-msvc/llvm/build/tools/lto/LTO.def"  lib\LLVM.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib  && cd ."
LINK : fatal error LNK1181: cannot open input file 'lib\LLVM.lib'
LINK failed. with 1181

@cuviper
Copy link
Member Author

cuviper commented Aug 3, 2017

That's annoying... it failed within the LLVM build. I'm tempted to just skip this flag on Windows, leaving it to the status quo, but I'll do a little research first.

@aidanhs aidanhs added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 3, 2017
@cuviper
Copy link
Member Author

cuviper commented Aug 3, 2017

Alright, I think that setting is just buggy on Windows, so I've excluded it from this change for now. Let's see if the rest of the platforms can handle it, otherwise I'll set it on Linux only.

@sanxiyn
Copy link
Member

sanxiyn commented Aug 4, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Aug 4, 2017

📌 Commit ced1fda has been approved by sanxiyn

@bors
Copy link
Contributor

bors commented Aug 4, 2017

⌛ Testing commit ced1fda with merge 157267418be3397b0627227a6f35675a6224fd67...

@bors
Copy link
Contributor

bors commented Aug 4, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Aug 4, 2017

dist-i686-freebsd failed to link LLVM products on distribution. Legit.

[00:49:28] [ 91%] Linking CXX executable ../../bin/llvm-ar
[00:49:28] CMakeFiles/llvm-ar.dir/llvm-ar.cpp.o: In function `llvm::cl::opt<(anonymous namespace)::Format, false, llvm::cl::parser<(anonymous namespace)::Format> >::printOptionInfo(unsigned int) const':
[00:49:28] llvm-ar.cpp:(.text._ZNK4llvm2cl3optIN12_GLOBAL__N_16FormatELb0ENS0_6parserIS3_EEE15printOptionInfoEj+0x1d): undefined reference to `llvm::cl::generic_parser_base::printOptionInfo(llvm::cl::Option const&, unsigned int) const'
[00:49:28] CMakeFiles/llvm-ar.dir/llvm-ar.cpp.o: In function `llvm::cl::opt<(anonymous namespace)::Format, false, llvm::cl::parser<(anonymous namespace)::Format> >::getOptionWidth() const':
[00:49:28] llvm-ar.cpp:(.text._ZNK4llvm2cl3optIN12_GLOBAL__N_16FormatELb0ENS0_6parserIS3_EEE14getOptionWidthEv+0x19): undefined reference to `llvm::cl::generic_parser_base::getOptionWidth(llvm::cl::Option const&) const'
...
[00:49:30] llvm-config.cpp:(.text.startup.main+0x3049): undefined reference to `llvm::raw_ostream::write(char const*, unsigned int)'
[00:49:30] llvm-config.cpp:(.text.startup.main+0x305a): undefined reference to `llvm::raw_ostream::write(char const*, unsigned int)'
[00:49:30] llvm-config.cpp:(.text.startup.main+0x3077): undefined reference to `llvm::raw_ostream::write(char const*, unsigned int)'
[00:49:30] llvm-config.cpp:(.text.startup.main+0x3179): undefined reference to `llvm::raw_ostream::write(char const*, unsigned int)'
[00:49:30] collect2: error: ld returned 1 exit status
[00:49:30] tools/llvm-config/CMakeFiles/llvm-config.dir/build.make:96: recipe for target 'bin/llvm-config' failed
[00:49:30] make[2]: *** [bin/llvm-config] Error 1
[00:49:30] CMakeFiles/Makefile2:11805: recipe for target 'tools/llvm-config/CMakeFiles/llvm-config.dir/all' failed
[00:49:30] make[1]: *** [tools/llvm-config/CMakeFiles/llvm-config.dir/all] Error 2
[00:49:30] Makefile:149: recipe for target 'all' failed
[00:49:30] make: *** [all] Error 2
[00:49:30] thread 'main' panicked at '
[00:49:30] command did not execute successfully, got: exit code: 2

@cuviper
Copy link
Member Author

cuviper commented Aug 4, 2017

Sheesh. OK, I have experience with linux-gnu, and it looks like apple-darwin was OK, so let's just try those.

@sanxiyn
Copy link
Member

sanxiyn commented Aug 4, 2017

The third time is the charm! @bors r+

@bors
Copy link
Contributor

bors commented Aug 4, 2017

📌 Commit 6c46f4f has been approved by sanxiyn

@bors
Copy link
Contributor

bors commented Aug 4, 2017

⌛ Testing commit 6c46f4f with merge 08f0680f3c414c9b54502a7776c77239a4bb26cd...

@bors
Copy link
Contributor

bors commented Aug 4, 2017

💔 Test failed - status-appveyor

@kennytm
Copy link
Member

kennytm commented Aug 4, 2017

@bors retry #42117x86_64-pc-windows-msvc failed to clone iron, spurious.

testing https://github.com/iron/iron
Initialized empty Git repository in /c/projects/rust/build/ct/iron/.git/
fatal: Could not parse object '21c7dae29c3c214c08533c2a55ac649b418f2fe3'.
From https://github.com/iron/iron
 * branch            master     -> FETCH_HEAD
fatal: Could not parse object '21c7dae29c3c214c08533c2a55ac649b418f2fe3'.
error: RPC failed; HTTP 500 curl 22 The requested URL returned error: 500 Internal Server Error
fatal: The remote end hung up unexpectedly
thread 'main' panicked at 'assertion failed: status.success()', src\tools\cargotest\main.rs:106:12
note: Run with `RUST_BACKTRACE=1` for a backtrace.

@bors
Copy link
Contributor

bors commented Aug 4, 2017

⌛ Testing commit 6c46f4f with merge ff1135b...

bors added a commit that referenced this pull request Aug 4, 2017
Link LLVM tools dynamically

Set `LLVM_LINK_LLVM_DYLIB=ON` -- "If enabled, tools will be linked with
the libLLVM shared library."  Rust doesn't ship any of the LLVM tools,
and only needs a few at all for some test cases, so statically linking
the tools is just a waste of space.  I've also had memory issues on
slower machines with LLVM debuginfo enabled, when several tools start
linking in parallel consuming several GBs each.

With the default configuration, `build/x86_64-unknown-linux-gnu/llvm`
was 1.5GB before, now down to 731MB.  The difference is more drastic
with `--enable-llvm-release-debuginfo`, from 28GB to "only" 13GB.

This does not change the linking behavior of `rustc_llvm`.
@bors
Copy link
Contributor

bors commented Aug 4, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: sanxiyn
Pushing ff1135b to master...

@bors bors merged commit 6c46f4f into rust-lang:master Aug 4, 2017
@cuviper
Copy link
Member Author

cuviper commented Aug 5, 2017

Whew, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants