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

target_feature cfg only obeys `-C target-feature="+feature"` #31662

Closed
rkruppe opened this Issue Feb 14, 2016 · 5 comments

Comments

Projects
None yet
4 participants
@rkruppe
Copy link
Member

rkruppe commented Feb 14, 2016

This program on a 32 bit Linux outputs no SSE :( even though our default target does enable SSE and SSE2:

#![feature(cfg_target_feature)]

#[cfg(target_feature="sse")]
fn main() {
    println!("yay, SSE!");
}

#[cfg(not(target_feature="sse"))]
fn main() {
    println!("no SSE :(");
}

I'll save myself the hassle of copying rustc -vV from the VM I tested this on, because the relevant code hasn't been touched since cfg_target_feature was introduced in 4f44258 (July 2015).

This is because the feature detection is really naive, neither asking LLVM for details nor even looking at rustc's own target definitions. @huonw mentioned this in #27731 (comment) but IMHO this deserves an issue of its own.

@huonw huonw changed the title cfg_target_feature is pretty terrible target_feature cfg only obeys `-C target-feature="+feature"` Feb 15, 2016

@huonw

This comment has been minimized.

Copy link
Member

huonw commented Feb 15, 2016

Yeah, this should be looking at the target spec in use (i.e. even if it is a custom one, not only knowing about the built-in ones), and it should also correctly turn off features when requested (e.g. -C target-feature="-sse").

neither asking LLVM for details

(I tried to do this, but couldn't find a way to actually get it to work. This would be far-and-away the nicest way if someone could make it work.)

@Aatch

This comment has been minimized.

Copy link
Contributor

Aatch commented Feb 15, 2016

Argh, LLVM apparently doesn't want anybody to get/query CPU features. This means it'll require a patch to LLVM to do properly, not a big one, since the functionality we need is basically already there (is feature X enabled), but it's not exposed at present.

@rkruppe

This comment has been minimized.

Copy link
Member Author

rkruppe commented Feb 15, 2016

Querying LLVM would be perfect, but since we're committed to supporting some official releases (not sure which ones exactly) this won't be a solution in the short term. Still, if someone wrote that patch and got it upstream now, this issue could be solved properly in the future (whenever we bump our minimum requirement to, let's say, 3.9).

@ranma42

This comment has been minimized.

Copy link
Contributor

ranma42 commented Feb 15, 2016

I think it might be possible to extract the features from LLVM without any changes to its code base.
Given an llvm::TargetMachine they are made available through its getMCSubtargetInfo() method. The MCSubtargetInfo objects exposes the raw features through the getFeatureBits () method and (given the CPU, which is available from the same object though getCPU()), they can be associated to the corresponding strings using llvm::SubtargetFeatures.

bors added a commit that referenced this issue Mar 30, 2016

Auto merge of #31709 - ranma42:target_feature-from-llvm, r=alexcrichton
Compute `target_feature` from LLVM

This is a work-in-progress fix for #31662.

The logic that computes the target features from the command line has been replaced with queries to the `TargetMachine`.

bors added a commit that referenced this issue Mar 30, 2016

Auto merge of #31709 - ranma42:target_feature-from-llvm, r=alexcrichton
Compute `target_feature` from LLVM

This is a work-in-progress fix for #31662.

The logic that computes the target features from the command line has been replaced with queries to the `TargetMachine`.

bors added a commit that referenced this issue Mar 30, 2016

Auto merge of #31709 - ranma42:target_feature-from-llvm, r=alexcrichton
Compute `target_feature` from LLVM

This is a work-in-progress fix for #31662.

The logic that computes the target features from the command line has been replaced with queries to the `TargetMachine`.

bors added a commit that referenced this issue Mar 30, 2016

Auto merge of #31709 - ranma42:target_feature-from-llvm, r=alexcrichton
Compute `target_feature` from LLVM

This is a work-in-progress fix for #31662.

The logic that computes the target features from the command line has been replaced with queries to the `TargetMachine`.

bors added a commit that referenced this issue Mar 31, 2016

Auto merge of #31709 - ranma42:target_feature-from-llvm, r=alexcrichton
Compute `target_feature` from LLVM

This is a work-in-progress fix for #31662.

The logic that computes the target features from the command line has been replaced with queries to the `TargetMachine`.

bors added a commit that referenced this issue Mar 31, 2016

Auto merge of #31709 - ranma42:target_feature-from-llvm, r=alexcrichton
Compute `target_feature` from LLVM

This is a work-in-progress fix for #31662.

The logic that computes the target features from the command line has been replaced with queries to the `TargetMachine`.

bors added a commit that referenced this issue Apr 19, 2016

Auto merge of #31709 - ranma42:target_feature-from-llvm, r=alexcrichton
Compute `target_feature` from LLVM

This is a work-in-progress fix for #31662.

The logic that computes the target features from the command line has been replaced with queries to the `TargetMachine`.

bors added a commit that referenced this issue Apr 20, 2016

Auto merge of #31709 - ranma42:target_feature-from-llvm, r=alexcrichton
Compute `target_feature` from LLVM

This is a work-in-progress fix for #31662.

The logic that computes the target features from the command line has been replaced with queries to the `TargetMachine`.

bors added a commit that referenced this issue Apr 20, 2016

Auto merge of #31709 - ranma42:target_feature-from-llvm, r=alexcrichton
Compute `target_feature` from LLVM

This is a work-in-progress fix for #31662.

The logic that computes the target features from the command line has been replaced with queries to the `TargetMachine`.
@huonw

This comment has been minimized.

Copy link
Member

huonw commented Apr 21, 2016

Fixed by #31709, I believe. Thanks @ranma42!

@huonw huonw closed this Apr 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.