Allow #[cfg(target_feature = "-feature")]

1

Right now in library code we can only query whether a target feature is enabled during compilation or not using #[cfg(target_feature = "feature")]. In other words, we can write code for two cases: "target feature is enabled" and "target feature is not enabled, but may be available during runtime". In the second case crates could use runtime target feature detection (e.g. based on CPUID) between two code branches: normal and target feature specific wrapped in #[target_feature(enable = "feature")].

In other words, in library code we have access to two states +feature and ?feature. But there is the third one: -feature. If a target feature was explicitly disabled using compilation flags or by target configuration, then libraries should not generate code branches dependent on the disabled feature. Unfortunately, currently there is no way for libraries to detect explicitly disabled target features. It causes issues for libraries which rely on runtime target feature detection. For example, in RustCrypto we have several reported issues like this. UEFI targets do not have access to FPU and SIMD, i.e. when you compile for x86_64-unknown-uefi compiler uses the following flags: target_feature = "-mmx,-sse,-sse2,-sse3,-ssse3,-sse4.1,-sse4.2,-3dnow,-3dnowa,-avx,-avx2,+soft-float" (arguably, +soft-float should not be a "positive" target feature and instead it should be something like -hard-float). But in crates like sha2 and aes we can not see the negative target features, thus we generate hardware-accelerated branches, which LLVM can not compile because of +soft-float.

I propose to allow target_feature = "-feature", target_feature = "?feature", and target_feature = "+feature". The current target_feature = "feature" will be a simple alias for the latter. Not only it will resolve the issues around "bare" x86 targets, but also will allow users to trim unnecessary branches dependent on target features (e.g. if a binary compiled with target-cpu=native, we could disable all unavailable on the CPU target features).

cc @gnzlbg

2

Do you have a link to an example? It's hard for me to imagine how you can use hardware acceleration (I assume that's manual assembly, because otherwise LLVM should be able to compile Rust code with imposed restrictions) in case where a feature isn't guaranteed.

3

See how aes and sha2 crates mentioned in the OP are implemented. Virtually all modern x86 CPU have AES-NI available, but it's not enabled by default like SSE2 when we compile for "full" x86 targets (i.e. it's not +aes). But it's quite likely that binary will be executed on CPU with AES-NI (i.e. we have ?aes). This is why we use runtime target feature detection, which looks roughly like this:

#[target_feature(enable = "aes")]
unsafe fn aesni_enc(...) {
    // we can use AES-NI intrinsics like _mm_aesenc_si128 here
    ...
}

fn aes_soft_enc(...) {
    // unaccelerated AES implementation
    ...
}

fn aes_enc(...) {
    if is_x86_feature_detected!("aes") {
        unsafe { aesni_enc(...) }
    } else {
        aes_soft_enc(...)
    }
}

In practice, for a number of reasons, in RustCrypto we use the cpufeatures crate instead of is_x86_feature_detected!. But the point is the same, if a project dependent on the aes crate is compiled with target-feature=-aes, then the AES-NI-dependent branch must be eliminated. Roughly, I want to be able to write the following code:

#[cfg(target_feature = "+aes")]
fn aes_enc(...) {
    aesni_enc(...)
}

#[cfg(target_feature = "-aes")]
fn aes_enc(...) {
    aes_soft_enc(...)
}

#[cfg(target_feature = "?aes")]
fn aes_enc(...) {
    if is_aesni_available() {
        unsafe { aesni_enc(...) }
    } else {
        aes_soft_enc(...)
    }
}

Of course, in practice the cfgs will be handled by is_x86_feature_detected! or by a crate like cpufeatures.

2 Likes
4

So, you're looking for a more standardized way to do this rather than having to define feature flags for it?

5

Compilation flags defined by crates (such as aes_force_soft) is nothing more than a subpar unergonomic workaround. Users who compile a project should not search through their dependency tree for various compilation flags defined by crates deep in it, when compiler already has all necessary information, but simply does not provide access to it for libraries.

2 Likes
6

I don't think -feature is the right indicator for disallowing features. There are a lot of times that this is used to just tweak the defaults. For example, a lot of the ARM targets have -neon in their target spec, but they should absolutely be open to runtime feature detection for that.

Maybe something more explicit like !feature could be used?

7

I am not familiar with ARM and Neon, but are you sure that -neon is not simply a way to overwrite defaults used by LLVM? My proposal is mostly about Rust's frontend. ?neon on the Rust side could be translated to -neon when code without target_feature(enable = "neon") is passed to LLVM. Either way, I do not care much about the exact syntax used for the feature, so I am fine with !feature, if people think it will be less confusing.

8

It is changing the default, yes, that's what I mean. Currently -Ctarget-feature is passed unfiltered to LLVM -- I think we could reasonably extend that with syntax like !feature, but it's a stable option so we should not change any existing meaning.

9

It's not quite true, even today we remap some target features. AFAIK Rust's target features are not documented to be tied to LLVM's in any way, which is to be expected as LLVM should be one of supported backends, not the only one. Since +/?/-feature in cfgs will be a new addition, the existing code will not be affected in any way. The only potentially visible change is that is_x86_feature_detected! will become -feature aware.

10

Ah! The CLI features didn't used to be mapped, but it looks like that changed in 1.61:

11

I'm unfamiliar with this space, but what happens if the user mistypes one of these stringly-typed sigils +, -, ?? E.g., if the user means ?foo but types /foo, will it be interpreted as +/foo, gating code on the existence of a feature /foo? Or are feature names required to be valid Rust identifiers, allowing this mistake to be caught?

12

Unfortunately, right now Rust accepts any target feature string in cfgs:

// This gets accepted
#[cfg(target_feature = "foo")]
fn f() {}

// But this results in a compilation error
#[target_feature(enable = "foo")]
unsafe fn f() {}

So target_feature = "/foo" is accepted even today, but always results in false. I don't know why and would prefer for it to be a compilation error (maybe in the next edition?).

13

In that case, would it be better to use a syntax that would allow more errors to be caught? Maybe syntax like this:

  • cfg(target_feature(has "aes"))
  • cfg(target_feature(lacks "aes"))
  • cfg(target_feature(maybe "aes"))

Never having written target-feature-dependent code, I assure you all that I defer to your experience! :slight_smile:

1 Like
14

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.