-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
cfg_target_feature
and target_feature
don't interact properly
#42515
Comments
It does panic. #![feature(cfg_target_feature, target_feature)]
#[target_feature = "+avx"]
fn should_panic() {
#[cfg(target_feature = "avx")]
panic!("have_avx");
}
fn main() {
should_panic();
}
If you don't compile with the appropriate target-feature settings, then the |
But |
What do gcc and clang do? Can you explain more about your use case and why you expect this to happen? |
Well gcc and c only have the preprocessor macros so that would behave like cfg_target_feature currently does. However, I assumed rust attributes are more context aware. |
My use case is for this procedural macro attribute I've written https://github.com/parched/runtime-target-feature-rs which generates 2 copies of the function, one with |
@parched gcc and Clang have the equivalent of
To give you some context, the
But |
More explicitly the macro I've written,
|
@BurntSushi I mean they only have macros for detecting the target features so the couldn't possibly change inside a function with |
@parched Sorry, but I still don't understand your use case. What problem are you trying to solve at a high level? For example, having a |
And I still don't understand what's fundamentally different between Rust's system and gcc's/Clang's. I think you need to back way up and help me try to understand what specific problems you're facing. |
@BurntSushi: I think the use case here is a matter of convenience: Write a function in which some parts are marked The difference to gcc/Clang is, as I understand it, that since the preprocessor runs before any c parsing happens it cannot know if it is inside a function marked |
@TimNN Thanks for explaining that. It makes a little more sense, although I don't understand why the macro can't just strip out the cfg'd code that's never going to execute. Popping up a level, I'm not particularly familiar with this aspect of the compiler (can arbitrary attributes impact the resolution of |
Yes just what TimNN says. @BurntSushi I was actually just thinking that on my bike ride home and that would definitely work. It would mean the macro would need to know all the target features and their hierarchy. I.e., if the feature enabled was sse3 the macro would have to know to strip out any cfg not sse2. |
Does the following help? #[inline(always)]
fn foo_impl() {
// ...generic implementation here...
#[cfg(traget_feature = "+avx")] {
// some avx specific optimizaiton here
}
#[cfg(not(target_feature = "+avx"))] {
// some non-avx generic alternative here
}
// ... more generic implementation ...
}
#[target_feature = "+avx"] fn foo_avx() { foo_impl() }
#[target_feature = "+sse3"] fn foo_sse3() { foo_impl() } That is, could we propagate the target features of The trivial case of this is what @parched proposed. |
Propagating through function calls as @gnzlbg described is entirely incompatible with how Edit: Sorry, the following doesn't make sense
|
Yes, but IMO worth it. An alternative would be to make |
From the discussion here, it seems like the code-gen-time context-aware behaviour of |
I wouldn't necessarily say there is no design that can make this work, but it is definitely is going to require a large amount of work to make it happen, and would most likely need to happen across an edition boundary. The most appropriate way to get a similar, but not equivalent, behaviour would be to use the |
This should panic but it doesn`t
I would like for this to work because I have a macro that generates 2 copys of a function, one with
#[target_feature = "+feat"]
and one without and I want to conditionally use some assembly when the feature is available.CC #29717
The text was updated successfully, but these errors were encountered: