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

Split record_header_indices loop to work around rustc/LLVM bug, on relevant targets #1671

Merged
merged 1 commit into from
Oct 16, 2018

Conversation

SimonSapin
Copy link
Contributor

rustc issue: rust-lang/rust#55105

Steps to reproduce:

rustup target add armv7-linux-androideabi
RUSTFLAGS="-Ctarget-feature=+neon" cargo build --target armv7-linux-androideabi --release

Output without this change:

   Compiling hyper v0.12.11 (/home/simon/projects/servo-deps/hyper)
LLVM ERROR: ran out of registers during register allocation
error: Could not compile `hyper`.

@seanmonstar
Copy link
Member

Yikes!

Since this is only an issue for Android ARM, what if looping twice included some cfg? That way any other platform doesn't need to see slowdown.

…levant targets

rustc issue: rust-lang/rust#55105

Steps to reproduce:

```
rustup target add armv7-linux-androideabi
RUSTFLAGS="-Ctarget-feature=+neon" cargo build --target armv7-linux-androideabi --release
```

Output without this change:

```
   Compiling hyper v0.12.11 (/home/simon/projects/servo-deps/hyper)
LLVM ERROR: ran out of registers during register allocation
error: Could not compile `hyper`.
```
@SimonSapin SimonSapin changed the title Split record_header_indices loop to work around rustc/LLVM bug Split record_header_indices loop to work around rustc/LLVM bug, on relevant targets Oct 16, 2018
@SimonSapin
Copy link
Contributor Author

I’ve also reproduced the error with armv7-unknown-linux-gnueabihf but not: aarch64-linux-android, aarch64-unknown-linux-gnu, arm-unknown-linux-gnueabihf, or arm-linux-androideabi. So this looks specific to ARMv7 but not Android.

I’ve added corresponding cfgs, with a macro to avoid repeating stuff.

This PR works for me when building hyper by itself, let’s try it in Servo before merging.

@seanmonstar seanmonstar merged commit 30a4f23 into hyperium:master Oct 16, 2018
@seanmonstar
Copy link
Member

Nice, the macro is pretty clever, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants