Summary
Currently, calling a function annotated with #[target_feature(enable = "foo")] requires the function making the call also to be annotated with #[target_feature(enable = "foo")] in order for the call not to require unsafe even if "foo" is enabled for the whole compilation. This RFC removes the requirement for unsafe to call a function annotated with #[target_feature(enable = "foo")] when "foo" is enabled for the whole compilation (by any mechanism: --target, -C target-cpu, -C target-feature).
Motivation
Consider the following code that uses u16x8 and u8x16 from core::simd:
cfg_if! {
if #[cfg(all(target_feature = "sse2", target_arch = "x86_64"))] {
use core::arch::x86_64::_mm_packus_epi16;
} else if #[cfg(all(target_feature = "sse2", target_arch = "x86"))] {
use core::arch::x86::_mm_packus_epi16;
} else {
}
}
// We don't care what the output is for lanes whose value is above
// `0xFFu16` as long as it's not UB and doesn't cross-contaminate
// lanes whose value is between `0u16` and `0uFFu16` (inclusive).
//
// Direct use of `_mm_packus_epi16` avoids the cost of zeroing the
// high halves.
cfg_if! {
if #[cfg(target_feature = "sse2")] {
#[inline(always)]
pub fn simd_pack(a: u16x8, b: u16x8) -> u8x16 {
unsafe {
// Safety: We have cfg()d the correct platform
_mm_packus_epi16(a.into(), b.into()).into()
}
}
} else {
#[inline(always)]
pub fn simd_pack(a: u16x8, b: u16x8) -> u8x16 {
let first: u8x16 = a.to_le_bytes();
let second: u8x16 = b.to_le_bytes();
let (ret, _) = first.deinterleave(second);
ret
}
}
}
The code requires unsafe even though the compiler could logically be made to figure out on its own that the call to _mm_packus_epi16 is safe when sse2 is enabled for the whole compilation.
The point of unsafe is to let the programmer assert to the compiler that something that the compiler can't itself prove to be safe is indeed safe.
Requiring unsafe when the compiler could be made to figure out on its own that the code is safe is bad, because it dilutes the review value of unsafe.
Most trivially, this makes crates look bad when quickly assessed simply by counting the instances of unsafe. Less trivially, logically unnecessary unsafe pollutes unsafe reviews. In the example above, it looks like it's no big deal. However, things add up: Prior to RFC 2396, pretty much everything using core::arch was unsafe to the point of approaching the C and C++ situation where actually-safe and actually-unsafe were not usefully distinguished as spots to review. RFC 2396 greatly improved the situation; this is the logical next step.
Note that simd_pack itself is deliberately not and should not be annotated with #[target_feature(enable = "foo")]: When programming with core::simd, either no function is annotated with #[target_feature(enable = "foo")] when relying on relevant features being enabled for the whole compilation or only a function at multiversioning point is annotated with #[target_feature(enable = "foo")] and the rest are inlined into it with #[inline(always)], so the relevant set of target_features in effect for the #[inline(always)] takes effect after inlining and the given #[inline(always)] function can get compiled with differet target_features within the same compilation.
Guide-level explanation
In addition for a safe function annotated with #[target_feature(enable = "foo")] not requiring unsafe to call when the caller function also has #[target_feature(enable = "foo")], unsafe is not required when calling a safe function annotated with #[target_feature(enable = "foo")] when "foo" is enabled for the whole compilation.
Reference-level explanation
See the previous section.
Drawbacks
Whether a function call requires unsafe now depends on the target configuration (--target, -C target-cpu, -C target-feature), so a crate tested with one target configuration may fail to compile with an error complaining about missing unsafe when compiled with another target configuration. On the flip side, compiling with warnings as errors may cause a crate not to compile when compiling with a configuration that enables more target_features compilation-wide than the configuration the crate has been tested with.
Rationale and alternatives
The above drawback is in practice much less of a drawback than it theoretically appears.
It's been argued that in the status quo, unsafe is a signal about a portability hazard. In general, that's not what unsafe means: unsafe means that the programmer asserts to the compiler that something that the compiler can't prove to be safe is actually safe. It's not generally a portability concern.
Ever since 1.0 Rust hasn't required unsafe to signal portability hazards when using std::os: the portability hazard signal is the module path (std::os), not unsafe. In this case, the signal of portability hazard is core::arch (or cfg). (The core::arch signal might not be visible for functions defined by crates rather than defined by the standard library, but neither is std::os.)
In practice, code that reaches for core::arch is likely to to use conditional compilation with cfg in one way or another. It is much more likely that a crate developer makes a mistake such that the different conditional pieces don't fit together with all possible target configurations and compilation fails for that reason than for compilation to fail because the crate developer only tested a scenario where unsafe isn't required due to the target configuration and then someone else compiles a crate with a target
configuration that requires unsafe. For a user of a crate, the significant thing is the failure the compile the crate, not the specific reason.
Notably, this concern is moot for the code pattern seen as the motivating example. Furthermore, the most mainstream targets where varying target configurations are relevant and variations of x86 and x86_64, and, as also, seen in the example, due to the way core::arch is organized for these architectures, they already pose the portability hazard that it's easy to cause a crate not to compile on x86 due to forgetting to make the import of every function under core::arch conditional to import it either from core::arch::x86_64 or core::arch::x86. Thus, there is already a more likely hazard resulting in failure to compile a crate where not tested compared to the compilability hazard introduced here.
There's an argument to be made that this proposal does not fully remove unsafe, because unsafe is still needed at the run-time dispatch point. unsafe indeed belongs there, but that point does not need to be present in code that uses core::arch. Code that relies statically on cfg, like the motivating example, does not have a run-time dispatch point at all. This is particularly relevant to sse2 on x86_64 and i686 targets and neon on aarch64 targets. Even in code that does use run-time dispatch can delegate the dispatch point to another crate, such as multiversion.
Unresolved questions
Unsolved if the case where unsafe is used but is unnecessary due to (and only due to) the target configuration should be special-cased in the compiler not to emit a warning.
Future possibilities
In the future, add a higher-level way than mere constant propagation to attach target_feature_available_at_call_site to a block and remove the requirement for unsafe to call functions with the checked target_feature from within such a block.