Pre-RFC: stabilization of target_feature

compiler
1

This pre-RFC proposes a path to stabilize cfg_target_feature and target_feature. It proposes the minimal set of functionality required to allow other RFCs to progress, and sketches some implementation work that needs to happen before that.

Recall: target features

Some CPUs of the x86 architecture family support bit manipulation algorithms directly as single hardware instructions (e.g. via BMI/BMI2/TBM/ABM instruction sets). Many CPUs of the ARM architecture family support SIMD algorithms via the NEON vector extensions.

While different architectures cannot be easily mixed in the same binary, a binary can still contain code for target features not supported for the CPU it runs on, as long as this code is not executed on that CPU (otherwise a SIGILL exception is typically thrown).

The unstable “target feature” functionality in rustc supports this via two mechanisms:

  • cfg_target_feature allows conditional compilation based on which target features are passed to rustc via cli arguments; this allows providing different implementations of some algorithms, depending on whether a target feature is enabled or not.
  • [target_feature] allows rustc to emit code that uses some target features; this allows rustc to emit code for target features not supported by the current target.

Stabilization proposal

1. White-listed target features

This RFC proposes to not stabilize any target feature names.

The stabilization of the target feature names themselves is left for follow-up RFCs (on SIMD, bit manipulation, …). However, stabilizing target features without stabilizing any target feature names necessarily introduces a split between stable and unstable target features. This requires changes to the current white-listing solution (see here for x86). Also, in the current white-listing solution, trying to use an unknown target feature results in an LLVM error/warning. This is not acceptable for a stable feature.

This RFC specifies the behavior of rustc for stabilized, unstable, and unknown target features with the objective of:

  1. eliminate all LLVM errors related to unknown/unavailable target features
  2. allow the stabilization of unstable crates relying on target features.

To achieve this, the current per-architecture target feature white-list should be split into two list for stabilized and unstable target features (per architecture). Rustc should implement the following semantics:

    1. Stable target features can be used on stable, beta, and nightly.
    1. Unstable target features can only be used on nightly.
    1. Unknown target features must be rejected by rustc (and not LLVM like it is currently the case).
    1. Unstable target features on stable must be rejected by rustc.
  • 5.Suppressing these hard errors results in the feature being disabled.

The mechanism to suppress these hard errors can follow that of warnings:

  • #[allow(unstable_target_feature = "feature")]
  • #[allow(unknown_target_feature = "feature")]

And the semantics should be that on stable for unstable or unknown target features explicitly suppressed:

  • conditional compilation returns false (the feature is disabled),
  • the function attribute does nothing.

This should happen prior to stabilization.

2. Conditional compilation: cfg_target_feature

The cfg_target_feature allows using target_feature configuration option via cfg to query specific hardware features of the target at compile-time. This information can be used for conditional compilation or via the cfg! macro like this:

if cfg!(target_feature = "bmi2") {
    // if target has the BMI2 instruction set, use a BMI2 instruction:
    unsafe { intrinsic::bmi2::bzhi(x, bit_position) }
} else {
    // otherwise call an algorithm that emulates the instruction:
    software_fallback::bzhi(x, bit_position)
}

The features supported by the target can be either manually passed to rustc using -C target-feature=+bmi2 or automatically detected by LLVM using, e.g., RUSTFLAGS="-C target-cpu=native".

3. Generating code for target features: the target_feature attribute

The target_feature function attribute allows the compiler to emit target specific code independently of the target features supported by the current target:

#[target_feature = "+sse4.2"]
fn foo() { ... }  // might contain SSE4.2 instructions even on targets that do not support SSE4.2

4. Minimal stabilization proposal

The discussion of stabilization was kickstarted by @alexcrichton on the tracking issue. This is an extended version of his proposal.

It proposes to stabilize the following:

  1. Stabilize passing -C target-feature="..." and -C target-cpu="...". to rustc where:
  • -C target-feature="" has the current semantics of enabling/disabling target features
  • -C target-cpu="" has the semantics of enabling all features supported by the CPU architecture menthioned (CPU architecture names are not stabilized)
  1. Stabilize conditional compilation: #[cfg(target_feature = "...")], cfg!(target_feature = "..."), …
  2. Stabilize function attribute: [target_feature = "..."]
  3. Stabilize the semantics proposed in section 1. white-listing w.r.t. stable, unstable, and unknown target features and how to disable them in stable
  4. Stabilize --print cfg.
  5. Stabilize the reference documentation for all of the above.

It does not propose to stabilize any target feature names or target CPU names. This should be done in follow-up RFCs (e.g. for SIMD, bit manipulation, cpu architectures, …).

5. Remaining work to be done prior to stabilization

  • Implement the stable, unstable, unknown semantics for target features
  • Document these semantics, cfg_target_feature, and [target_feature] in the language reference

Pinging: @burntsushi , @alexcrichton

1 Like
2

Thanks for writing this up @gnzlbg! Looks pretty great to me, I’ve only a few small nits:

  • Typically access to unstable features is gated via #![feature] rather than #[allow] (as proposed), would you be ok gating access on nightly to unstable target features with #[feature(unstable_target_feature)] and unknown features with #[feature(unknown_target_feature)]?
  • The -C target-feature and -C target-cpu flags are effectively stable today, so I think we need to be careful about breaking those. For example -C target-feature=+unknown-feature is accepted today (and warned about in LLVM as you’ve found). I think that we just need to be sure to not require nightly for such an invocation and instead simply print a warning. Access in code, however (via attributes) should certainly be gated.
3

Unstable/unknown "target features" (in contrast to language features) in stable produce an error. When we stabilize cfg_target_feature/target_feature, we want unstable code that uses those to compile on stable on the next rust version. Since we are not stabilizing any "target features", it makes sense to allow users to easily disable them on stable, so that their code that uses them still does compile. The semantics then become that the "target features" are disabled: cfg!(target_feature = "unstable_feature") returns always false, and [target_feature = "unstable_feature"] does not enable unstable_feature for a particular function. Since newer features will be unknown in older compiler versions, it makes sense to allow this for unknown features as well, to allow improving code in a more backwards compatible way.

However, some users might not want to just silently disable "target features" on stable, but emit a warning to their users conveying the information that they might be missing something by using the stable rust version. That is, they might want to turn the hard error into a warning.

The main rationale behind using allow was to allow users to also use #[warn(target_feature = "avx")] to produce a warning on stable, but still allow the code to compile.

The second argument behind using allow and not feature is because feature is used to enable "language features" and AFAIK can only be used in a nightly compiler, but the point is to be able to silently disable "target features" on a stable compiler.

Does #[feature(unstable_target_feature = "avx")] read like "the unstable target feature avx is silently disabled on stable rust when used" ?

It does not read like that to me. If the majority think that this is fine (and I am willing to bend on this), and if we can use feature on stable rust without problems, and if we don't need the possibility of turning the hard error into a warning, then I would be fine with this change.

Also, if we use allow, we can add the possibility to warn later in a backwards compatible change, so we don't need to implement and stabilize that right now. I was kind of think that the checking against the white-lists should be implemented as a warning that errors by default, and then allow and warn "just work" as expected. If the feature is not in the white-lists, code generation ignores it and doesn't pass it to LLVM.

The -C target-feature and -C target-cpu flags are effectively stable today, so I think we need to be careful about breaking those. For example -C target-feature=+unknown-feature is accepted today (and warned about in LLVM as you've found). I think that we just need to be sure to not require nightly for such an invocation and instead simply print a warning. Access in code, however (via attributes) should certainly be gated.

Indeed, I did not knew that one could use them on stable. This is bad, but not horrible, since -C target-feature=+unknown-feature will not do anything useful, and if rustc rejects it, the fix is as easy as removing that. This is a backwards incompatible change, but at the same time, it might fix bugs where the target feature is misspelled. I don't know whats the policy on this, but doing a crater run sounds like something we might want to do anyways to asses breakage.

The main problem will be that -C target-feature=+unstable_feature will break on stable if this RFC is implemented, but then I don't see a solution that does not involve stabilizing all currently used stable features along this change to avoid breakage, and from this point on make sure that new target features are unstable first.

4

Sorry I’m not really following how we’re reaching these conclusions. Maybe it’d be good to talk about some concrete examples?

Let’s say we’ve got a stable target feature stable and an unstable target feature unstable. I would expect this program to compile on all channels:

// works on all channels
fn foo() {
    if cfg!(target_feature = "stable") {
        bar();
    }
}

#[target_feature = "stable"]
fn bar() {}

I’d expect this to fail to compile on all channels:

// fails to compile on all channels.
fn foo() {
    if cfg!(target_feature = "unstable") {
        bar();
    }
}

#[target_feature = "unstable"]
fn bar() {}

And I’d expect this to only compile on nightly

// only compiles on nightly

#![feature(unstable_target_features)]

fn foo() {
    if cfg!(target_feature = "unstable") {
        bar();
    }
}

#[target_feature = "unstable"]
fn bar() {}

I would, however, expect rustc -C target-feature=+unstable to work across all three channels at all times, although I think it’s reasonable to print a warning on all three channels that -Z unstable-options will be required to enable the feature in the future.

So put another way, I would expect:

  • Unstable features of Rust (be it unknown or unstable target features) are unusable on all channels without an opt-in
  • Unstable target features are the wild west once you enable the corresponding feature in your crate (e.g. cfg! and #[target_feature])

I would personally avoid #[warn(foo = "bar")] or #[feature(foo = "bar")] as we don’t really have any support for this in the compiler today and it seems somewhat alien in terms of a new feature for lints or features themselves. I don’t think we’d get that much benefit from being so fine-grained as well, so I think it’d be ok to lump all unstable/unknown target features into the same “rust feature bucket”

Does that make sense? Or does it differ from what you’re thinking?

5

I expect this (straw man syntax) to compile on stable as well, with the semantics that the target feature "unstable" is disabled:

// only compiles on nightly

#![feature(unstable_target_features = "unstable")]

fn foo() {
    if cfg!(target_feature = "unstable") {  // always false on stable rust
        bar();
    }
}

#[target_feature = "unstable"]  // does nothing on stable rust
fn bar() {}

rustc -C target-feature=+unstable to work across all three channels at all times,

I would expect this to error on stable, since the target feature "unstable" is not stable (unless one disables it like above). Otherwise we are defacto stabilizing all unstable target features.

I also think that opting into all unstable features at once is too coarse grained and brittle even for unstable, one should opt into each unstable feature independently. In particular, since we plan to stabilize some, but not all of them, and some of the unstable ones are actually probably going to change in semantics (e.g. @burntsushi added popcnt but whether that should be a feature of its own might change during stabilization).

I would personally avoid #[warn(foo = "bar")] or #[feature(foo = "bar")] as we don't really have any support for this in the compiler today and it seems somewhat alien in terms of a new feature for lints or features themselves.

Fair enough, we can have a flag for each target feature, following the example above: unstable_target_feature_${feature_name}$, that is, for the target feature "unstable" we obtain: unstable_target_feature_unstable.

1 Like
6

Oh so typically today we forbid #![feature] (e.g. unstable language features) on the stable channel entirely, and unknown/unstable target features fall into the bucket of "unstable language fatures", right? (sure is fun talking about "language features" and "target features" as "feature" is ambiguous!)

I also definitely agree that rustc -C target-feature=+unstable should return an error on the stable channel, but it's accepted today by accident :(. In that sense I'd just want to prevent breaking anyone's build by accident.

I'd be fine with this!

7

Agreed, and to reiterate what @gnzlbg said, if we consider this stable, then we have already stabilised all the feature names. IMO we should implement an error for an unknown feature as @gnzlbg proposes but have it a warning for a few releases first.

8

Oh sorry yeah to be clear I’m totally fine moving toward making rustc -C target-feature=+unstable an error! We’ll just have to make it a slow transition I think with warnings on stable for awhile.

1 Like
9

and unknown/unstable target features fall into the bucket of "unstable language fatures", right?

Right know, enabling an unknown feature doesn't do anything (it raises a LLVM warning). I think that unknown/unstable target features should just be disabled on stable (with some warning/error options/defaults available). This would allow unstable crates to migrate to rust stable.

Consider the answer to the question "Is this target feature enabled?" Should just be "no" on stable if the target feature is unknown or unstable; "no, this target feature is not enabled" is always a valid answer. Also, the target feature function attribute declares that: "the compiler is allowed to use this target features when emitting code for this function" - not emitting code that uses those target features is also a valid option.

So I don't think this type of behavior has any downsides, and the main upside is that crates that only work on nightly because of target features would work on stable.

Authors that do not want their crates to work when a feature is not enabled are already preventing compilation on nightly rust (e.g. by making compilation fail if the target feature is not enabled). They can keep doing that on stable.

Do we agree on this? If we do not agree, why? I haven't found any downsides but that doesn't mean they aren't there.

I also definitely agree that rustc -C target-feature=+unstable should return an error on the stable channel, but it's accepted today by accident :(. In that sense I'd just want to prevent breaking anyone's build by accident.

I think we can make using an unknown/unstable target feature on stable a warning during a full release cycle, and then switch that to an error in the next cycle. This would just be the default behavior though. DIsabling the error/warning to make these features "do nothing/return false" should be made as easy as possible for both ends: the author of a library using cfg_target_feature/target_feature with an unstable feature, and the user of such a library.

I think it would be better to focus on agreeing on the semantics this should have on stable first, and worry about how to implement them later (that is, postpone the allow/deny(...) vs feature(...) issue).

10

Oh sure yeah and to be clear I agree! I'm just advocating not doing this all at once. This runs a risk of breaking builds in the ecosystem, so I think we should just take a more conservative route with warnings first, errors later.

I've basically just been coming at this with the mindset of how unstable features work today elsewhere in the compiler. For example let's say we add an unstable method Option::foo to the standard library. You can actually to option.foo() on all the Rust channels, it's just an error on stable/beta. In that sense the compiler isn't doing anything special here, everything is always available at all times. The purpose of feature gating is to just explicitly disallow some features on stable/beta, while allowing everything on nightly with an opt-in.

One surprising interaction I could imagine is that LLVM target features are not totally isolated. Let's say, for example, you've got cfg!(target_feature = "ssse3") checks in your code, and then you compile with -C target-cpu=native. You may be surprised that without #![feature(unstable_target_feature)] this check returns false! However with that feature enabled it returns true.

But in general I'm sorry I didn't really intend on this turning into a discussion point. I'm mostly just explaining how almost all feature stabilizations/gatings have gone in the past and why I think this shouldn't really be that much different. Do you disagree with the semantics I'm proposing? Getting consensus with the broader community will be much easier I think if this sticks to the "tried and true" or, e.g., familiar solutions.

11

I disagree, as that assumes a feature being disabled is a subset of the feature being enabled. Enabled and disabled could be mutually exclusive, or even being enabled could be a subset of disabled, like the 'd16' llvm feature on arm for example.

12

Could you elaborate?

Either the feature is enabled, or code that doesn’t offer an alternative path will just loudly fail to compile, but it will do so on both nightly and stable. Maybe a complete example would asses the part that I am missing.

13

@gnzlbg do you see a downside to following the unstable feature pattern today? I think it’s definitely a little less ergonomic, but beyond that are there other downsides you see?

14

think it's definitely a little less ergonomic, but beyond that are there other downsides you see?

No, the only downsides I see is that:

  • unstable crates that rely on target feature still cannot be "easily" made to work on stable,
  • unstable crates that rely on target feature will need to be updated on nightly to require the "unstable_target_feature_xxx" they are using.

But those downsides will disappear as the nightly crates migrate to the new unstable feature flags, and as those start to get stabilized, so doing what you propose is still a step forward (although maybe not as big as I had hoped).

I wish @burntsushi would chime in to give his opinion due to his work with the SIMD rfc.

15

I might be misunderstanding what you meaning, but as understand it you are saying the fallback code path when the feature is disabled should be fine (but possibly not optimal) even if the feature is actually enabled but is unknown to rustc. That is true for probably most target features but not all. If the feature is like d16 it reduces the number of FP registers so the fallback path may use more of them. Hopefully this would be caught by LLVM (I tested and this case is), however, I don't think generally there is a guarantee of that so you could end up with SIGILL at runtime.

16

Oh no, sorry, this is not what I meant. What I meant is that at least today on nightly, if some code needs (as in, cannot work properly without it) a target feature to work, the right thing to do is to prevent compilation from succeeding if the feature is disabled. The way to achieve this is to just use conditional compilation with cfg_target_feature to not provide an alternative code path when the feature isn't there (but provide a loud, self explanatory compilation error instead). This is typically done at the top-level of a crate +- build.rs and works, but feels a bit ad-hoc.

Your point stands, implicitly disabling all target features in stable won't make all of the nightly code just work though.

17

@alexcrichton another alternative could be to keep allowing any target features control via -C target-feature=... but only allow the detection via the cfg macros of the target features that are stabilized. That is, a user that writes -C target-feature=+d16 in stable today would still be able to do so without a warning / error. But what they won’t be able to do on stable is perform conditional compilation or code generation based on that until unstable_target_feature_d16 is stabilized.

This might be the path of less breakage, since basically we would only need to:

  • step 0. warn on usage of target features without the corresponding feature(unstable_target_feature_xxx) feature flag on nightly
    • step 0.5. at some point make this an error on nightly
  • step 1. stabilize cfg_target_feature/target_feature as is, without stabilizing any features
  • step 2…N: stabilize those unstable_target_feature_xxx that we want to have on stable
18

Sounds reasonable to me!

19

I will change the pre-RFC to reflect that and submit it as an RFC next.

2 Likes
20

Awesome and thanks again for pushing on this @gnzlbg!