Skip to content

rustc: Try again to disable NEON on armv7 linux #36933

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Oct 4, 2016

Conversation

alexcrichton
Copy link
Member

This is a follow-up to #35814 which apparently didn't disable it hard enough. It
looks like LLVM's default armv7 target enables NEON so we'd otherwise have to
pass -neon, but we're already enabling armv7 with +v7 supposedly, so let's
try just telling LLVM that the armv7 target is arm and then enable features
selectively.

Closes #36913

@rust-highfive
Copy link
Contributor

r? @eddyb

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

@eddyb
Copy link
Member

eddyb commented Oct 3, 2016

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 3, 2016

📌 Commit 56a7db2 has been approved by eddyb

@@ -13,7 +13,7 @@ use target::{Target, TargetOptions, TargetResult};
pub fn target() -> TargetResult {
let base = super::linux_base::opts();
Ok(Target {
llvm_target: "armv7-unknown-linux-gnueabihf".to_string(),
llvm_target: "arm-unknown-linux-gnueabihf".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it matters, but you lose the Tag_CPU_arch_profile: Application attribute this way, but you could add it back with +aclass.
IMO armv7-unknown-linux-gnueabihf with -neon,+vfp3,+d16 is all that is needed here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also lose +dsp.

@@ -13,7 +13,7 @@ use target::{Target, TargetOptions, TargetResult};
pub fn target() -> TargetResult {
let base = super::linux_base::opts();
Ok(Target {
llvm_target: "armv7-unknown-linux-gnueabihf".to_string(),
llvm_target: "arm-unknown-linux-gnueabihf".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also lose +dsp.

@alexcrichton
Copy link
Member Author

@bors: r-

Ah thanks for the info @parched! I'll update.

This is a follow-up to rust-lang#35814 which apparently didn't disable it hard enough. It
looks like LLVM's default armv7 target enables NEON so we'd otherwise have to
pass `-neon`, but we're already enabling armv7 with `+v7` supposedly, so let's
try just telling LLVM that the armv7 target is arm and then enable features
selectively.

Closes rust-lang#36913
@alexcrichton
Copy link
Member Author

@bors: r=eddyb

@bors
Copy link
Collaborator

bors commented Oct 4, 2016

📌 Commit 4625642 has been approved by eddyb

Copy link
Contributor

@parched parched left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw the v7 and thumb2 are redundant when using the armv7 triple.

@bors
Copy link
Collaborator

bors commented Oct 4, 2016

⌛ Testing commit 4625642 with merge 1a41928...

bors added a commit that referenced this pull request Oct 4, 2016
rustc: Try again to disable NEON on armv7 linux

This is a follow-up to #35814 which apparently didn't disable it hard enough. It
looks like LLVM's default armv7 target enables NEON so we'd otherwise have to
pass `-neon`, but we're already enabling armv7 with `+v7` supposedly, so let's
try just telling LLVM that the armv7 target is arm and then enable features
selectively.

Closes #36913
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants