rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.89k stars 12.78k forks source link

How should cfg(target-feature) behave around forbidden target features? #132351

Open RalfJung opened 3 weeks ago

RalfJung commented 3 weeks ago

See https://github.com/rust-lang/rust/pull/129884 and https://github.com/rust-lang/rust/issues/116344 for context explaining what a forbidden target feature is.

The plan for target features like fpreg on ARM is that they will be forbidden unless the target has soft-float in its base feature set. This means our hardfloat ARM targets require fpregs, but on our softfloat ARM targets users can opt-in to having the FPU avialable, it just won't be used for the ABI. See https://github.com/rust-lang/rust/pull/124404 for why we can't just forbid fpregs outright.

What should the behavior of cfg(target-feature = "fpregs") be, then? For now, #129884 makes it so that forbidden features are never set in cfg. But for this one it seems like when we unstable add fpregs for softfloat targets, then we'll want to unstably expose the ability to do cfg(target-feature = "fpregs") on all ARM targets. So we'll need a concept of target features that are forbidden to set, but can be unstably (and eventually, stably) queried. At the same time, other target features like soft-float likely should never be queried.

So seems like for these more complicated features, the question whether a feature is forbidden to set is somewhat orthogonal to its stability. I have some plans for a follow-up PR to https://github.com/rust-lang/rust/pull/129884 to be able to represent that, but still need to figure out the details.

RalfJung commented 2 weeks ago

My current plan is to have a representation like this:

pub enum Stability {
    /// This target feature is stable. It can be used in `#[cfg(target_feature)]` and
    /// `allow_toggle` controls when it can be used in `#[target_feature]` or
    /// `-Ctarget-feature`.
    Stable {
        allow_toggle: fn(&Session) -> Result<(), &'static str>,
    },
    /// This target feature is unstable. It can be used in `#[cfg(target_feature)]` on nightly,
    /// and `allow_toggle` controls when it can be used in `#[target_feature]` or
    /// `-Ctarget-feature`; the former additionally requires enabling the given nightly feature.
    Unstable {
        feature: Symbol,
        allow_toggle: fn(&Session) -> Result<(), &'static str>,
    },
    /// This feature can not be set via `-Ctarget-feature` or `#[target_feature]`, it can only be set in the basic
    /// target definition. Used in particular for features that change the floating-point ABI.
    Forbidden { reason: &'static str },
}

For instance, fpreg would be Unstable (and later Stable) with an allow_toggle function that ensures that the target base definition enables the soft-float feature.

What I am not sure about is whether a query can return function pointers. Also it may be good to evaluate those functions only once... but we actually do access this target feature stuff without going through the query in a bunch of early codegen setup code. We'll just have to benchmark this.