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

Misc fixes for configure #37482

Merged
merged 2 commits into from
Nov 3, 2016
Merged

Misc fixes for configure #37482

merged 2 commits into from
Nov 3, 2016

Conversation

matwey
Copy link
Contributor

@matwey matwey commented Oct 30, 2016

Currently,
./configure at armv6 machines ends up with

configure: error: unknown CPU type: armv6l

./configure at armv7 machines silently produces build for armv6 (compatible, but suboptimal)

configure: CFG_BUILD            := arm-unknown-linux-gnueabihf

armv7l is armv7 architecture and CFG_CPUTYPE should be armv7 in order to end up
with armv7-unknown-linux-gnueabihf.mk rather than
arm-unknown-linux-gnueabihf.mk
Use arm-unknown-linux-gnueabihf for hardware floating point armv6 variant
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

Thanks for the PR! Right now rustup has a similar block but also has this:

@alexcrichton
Copy link
Member

Er, hit comment too soon, but rustup also has this:

# Detect armv7 but without the CPU features Rust needs in that build,
# and fall back to arm.
# See https://github.com/rust-lang-nursery/rustup.rs/issues/587.
if [ $_ostype = "unknown-linux-gnueabihf" -a $_cputype = armv7 ]; then
    if ensure grep '^Features' /proc/cpuinfo | grep -q -v neon; then
        # At least one processor does not have NEON.
        local _cputype=arm
    fi
fi

I wonder if that also needs to be included here? Could you also elaborate on the purpose of the PR in the description as well?

cc @brson

@matwey
Copy link
Contributor Author

matwey commented Nov 2, 2016

@alexcrichton I am not sure that your code snippet is necessary.

@alexcrichton
Copy link
Member

Ah I erroneously thought that the armv7 Linux target enabled NEON, which we needed to specifically check for lack of support and go back to arm. Due to a recent PR, though, we no longer assume NEON on armv7, so this is fine.

@bors: r+

@bors
Copy link
Contributor

bors commented Nov 2, 2016

📌 Commit 9b81f3c has been approved by alexcrichton

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Nov 2, 2016
Misc fixes for configure

Currently,
`./configure` at armv6 machines ends up with

```
configure: error: unknown CPU type: armv6l
```

`./configure` at armv7 machines **silently** produces build for armv6 (compatible, but suboptimal)

```
configure: CFG_BUILD            := arm-unknown-linux-gnueabihf
```
bors added a commit that referenced this pull request Nov 2, 2016
Rollup of 10 pull requests

- Successful merges: #37351, #37405, #37473, #37482, #37488, #37498, #37502, #37513, #37517, #37523
- Failed merges: #37521
@bors bors merged commit 9b81f3c into rust-lang:master Nov 3, 2016
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.

5 participants