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.

Sign up for GitHub

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

Jump to bottom

Use dynamic linking and download LLVM from CI for rustc across platforms #77084

Closed
jyn514 opened this issue Sep 22, 2020 · 4 comments · Fixed by #80932
Closed

Use dynamic linking and download LLVM from CI for rustc across platforms #77084

jyn514 opened this issue Sep 22, 2020 · 4 comments · Fixed by #80932
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-linkage Area: linking into static, shared libraries and binaries A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-feature-request Category: A feature request, i.e: not implemented / a PR. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Sep 22, 2020

From #76810:

Longer-term I do think it's best to get back to a point where we're consistently doing the same thing for LLVM across all our platforms. It's been the case for quite some time that LLVM is built with ThinLTO on Linux but nowhere else. Differing in linkage is also unfortunate across the major platforms too.

I've given up on trying to make link-shared work well across the board -- see PR description and the last few CI failures -- it seems link LLVM is just not ready for a shared link on macOS or when cross-compiling, which excludes almost all of our builders.

Switching to static linking would preclude any of the improvements from #76349, so it would be better to use dynamic linking instead which would allow any contributor to get started without building LLVM from source or using system LLVM. That said, it sounds like dynamic linking will be very hard to do on other platforms.

cc @Mark-Simulacrum
@jyn514 jyn514 added A-linkage Area: linking into static, shared libraries and binaries E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-feature-request Category: A feature request, i.e: not implemented / a PR. A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Sep 22, 2020
@Mark-Simulacrum
Copy link
Member

Cc @cuviper @nikic @nagisa - do we know what the right way to bring the issues (bugs?) I found to LLVM developers? Are there active maintainers of LLVMs build system?

@jyn514 jyn514 added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Sep 23, 2020
@nagisa
Copy link
Member

nagisa commented Sep 23, 2020

LLVM’s bugzilla is pretty much the only place for bug reports. If its something less defined than a precise bug report, talking it out on the mailing lists is the way to go.

@est31
Copy link
Member

est31 commented Sep 23, 2020

Switching to static linking would preclude any of the improvements from #76349, so it would be better to use dynamic linking instead

In which way is static linking making downloaded LLVM impossible?

@Mark-Simulacrum
Copy link
Member

I don't think it's impossible to download/ship the statically linked LLVM, it's something I want to explore. That said, we want to move to dynamically linking LLVM because we ideally want to be applying ThinLTO to LLVM everywhere, not just on x86_64-unknown-linux-gnu, and since LTO happens at link-time, we want that to happen once, not on the multiple LLVM links occurring during CI.

This was referenced Jan 7, 2021
Merged
Merged
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 16, 2021
@m-ou-se
Rollup merge of rust-lang#80932 - jyn514:download-windows-llvm, r=Mar…
b2d8614 
…k-Simulacrum

Allow downloading LLVM on Windows and MacOS

- Don't ignore packaging `llvm/lib/` for `rust-dev` when LLVM is linked
statically
- Add `link-type.txt` so bootstrap knows whether llvm was linked
  statically or dynamically
- Don't assume CI LLVM is linked dynamically in `bootstrap::config`
- Fall back to dynamic linking if `link-type.txt` doesn't exist
- Fix existing bug that split the output of `llvm-config` on lines, not spaces
- Only special case MacOS when dynamic linking. Static linking works fine.
- Enable building LLVM tests

  This works around the following llvm bug:

  ```
  llvm-config: error: component libraries and shared library

  llvm-config: error: missing: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/llvm/build/lib/libgtest.a
  llvm-config: error: missing: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/llvm/build/lib/libgtest_main.a
  llvm-config: error: missing: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/llvm/build/lib/libLLVMTestingSupport.a
  thread 'main' panicked at 'command did not execute successfully: "/home/joshua/rustc2/build/x86_64-unknown-linux-gnu/llvm/build/bin/llvm-config" "--libfiles"
  ```

  I'm not sure why llvm-config thinks these are required, but to avoid
  the error, this builds them anyway.

- Bump version of `download-ci-llvm-stamp`

  `src/llvm-project` hasn't changed, but the generated tarball has.

Fixes rust-lang#77084.

# Current Status

This works on both MacOS and Windows! 🎉 🎉 Thanks to `@nagisa,` `@halkcyon,` `@Lokathor,` `@jryans,` and `@poliorcetics` for helping me test!

The `if-available` check now supports all tier 1 platforms. Although only x64 apple and x64 msvc have been tested, none of the changes here are Windows or Mac specific, and I expect this to work anywhere that LLVM artifacts are uploaded to CI (i.e. the `rust-dev` component exists).

## Windows

Note that if you have an old version of MSVC build tools you'll need to update them. VS Build Tools 2019 14.28 and later are known to work. With old tools, you may see an error like the following:

```
error LNK2001: unresolved external symbol __imp___std_init_once_complete
```
m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 17, 2021
@m-ou-se
Rollup merge of rust-lang#80932 - jyn514:download-windows-llvm, r=Mar…
98cc8ae 
…k-Simulacrum

Allow downloading LLVM on Windows and MacOS

- Don't ignore packaging `llvm/lib/` for `rust-dev` when LLVM is linked
statically
- Add `link-type.txt` so bootstrap knows whether llvm was linked
  statically or dynamically
- Don't assume CI LLVM is linked dynamically in `bootstrap::config`
- Fall back to dynamic linking if `link-type.txt` doesn't exist
- Fix existing bug that split the output of `llvm-config` on lines, not spaces
- Only special case MacOS when dynamic linking. Static linking works fine.
- Enable building LLVM tests

  This works around the following llvm bug:

  ```
  llvm-config: error: component libraries and shared library

  llvm-config: error: missing: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/llvm/build/lib/libgtest.a
  llvm-config: error: missing: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/llvm/build/lib/libgtest_main.a
  llvm-config: error: missing: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/llvm/build/lib/libLLVMTestingSupport.a
  thread 'main' panicked at 'command did not execute successfully: "/home/joshua/rustc2/build/x86_64-unknown-linux-gnu/llvm/build/bin/llvm-config" "--libfiles"
  ```

  I'm not sure why llvm-config thinks these are required, but to avoid
  the error, this builds them anyway.

- Bump version of `download-ci-llvm-stamp`

  `src/llvm-project` hasn't changed, but the generated tarball has.

Fixes rust-lang#77084.

# Current Status

This works on both MacOS and Windows! 🎉 🎉 Thanks to ``@nagisa,`` ``@halkcyon,`` ``@Lokathor,`` ``@jryans,`` and ``@poliorcetics`` for helping me test!

The `if-available` check now supports all tier 1 platforms. Although only x64 apple and x64 msvc have been tested, none of the changes here are Windows or Mac specific, and I expect this to work anywhere that LLVM artifacts are uploaded to CI (i.e. the `rust-dev` component exists).

## Windows

Note that if you have an old version of MSVC build tools you'll need to update them. VS Build Tools 2019 14.28 and later are known to work. With old tools, you may see an error like the following:

```
error LNK2001: unresolved external symbol __imp___std_init_once_complete
```
@bors bors closed this as completed in cfe2253 Jan 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-linkage Area: linking into static, shared libraries and binaries A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-feature-request Category: A feature request, i.e: not implemented / a PR. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow downloading LLVM on Windows and MacOS jyn514/rust
4 participants
@nagisa @Mark-Simulacrum @est31 @jyn514