Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions library/core/src/attribute_docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -850,3 +850,64 @@ const _: () = ();
/// [`cfg`]: ./attribute.cfg.html
/// [the `cfg_attr` attribute]: ../reference/conditional-compilation.html#the-cfg_attr-attribute
const _: () = ();

#[doc(attribute = "target_feature")]
//
/// Enables extra CPU instructions for a single function.

@workingjubilee workingjubilee Oct 30, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. It's an ISA extension... it doesn't necessarily correspond exactly to extra instructions. The actual behavior is thus effectively to specify to the code generator that a behavior of the CPU is different (with the baseline being, yes, "more instructions are valid encodings instead of being undefined or only-trapping").

A good example of a non-instruction feature would be ermsb, AKA "rep movs is good": https://github.com/llvm/llvm-project/blob/4f7ca43759f63102f3f73677a5008d0a5576ed12/llvm/lib/Target/X86/X86.td#L398

It changes nothing about what the CPU can do, but encourages the compiler to use a simpler code pattern in some cases.

And yes, it is on the list of officially supported features:

$ rustc --print target-features
Features supported by rustc for this target:
    ...
    ermsb                           - REP MOVS/STOS are fast.

Unfortunately crt-static is also a target feature, but I don't think we need to even think about it as IMO it should not have been. It is a far more annoying counterexample and it doesn't interact with a number of things "real" features do.

View changes since the review

///
/// Newer CPUs support instructions that older ones may lack. AVX2, for example, processes 32
/// bytes in a single instruction, but a program using it everywhere cannot run on older machines.
Comment on lines +858 to +859

@workingjubilee workingjubilee Oct 30, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm. The CPUs that shipped with AVX2 were in 2011. They are quite old by now.

The difference is that Intel kept shipping new lines of non-AVX2 CPUs also until... oh, into the 2020s. Tremont misses AVX2. https://en.wikipedia.org/wiki/Tremont_(microarchitecture)

Gracemont has it, but missed AVX512, and that's why Alder Lake wound up not supporting AVX512 except for a few early CPUs that hadn't had it fused off yet. The motherboard is still supposed to disable their AVX512 though, so there's no per-core variance in instruction set.

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably be better to say something like "early or low-power x86-64 CPUs", I think.

/// By default the compiler stays within a baseline that the compilation target defines.
/// `#[target_feature(enable = "...")]` enables the named features for one function, so a portable
/// binary can use faster instructions on CPUs that support them, and fall back otherwise:
///
/// ```
/// # #[cfg(target_arch = "x86_64")] {
/// #[target_feature(enable = "avx2")]
/// fn increment_avx2(data: &mut [u8]) {
/// // The compiler may use AVX2 instructions to process many bytes at once.
/// for byte in data {
/// *byte = byte.wrapping_add(1);
/// }
/// }
///
/// fn increment(data: &mut [u8]) {
/// if is_x86_feature_detected!("avx2") {
/// // SAFETY: the CPU was just confirmed to support AVX2.
/// unsafe { increment_avx2(data) }
/// } else {
/// // Fallback that runs on every x86-64 CPU.
/// for byte in data {
/// *byte = byte.wrapping_add(1);
/// }
/// }
/// }
/// # }
/// ```
///
/// Calling a `#[target_feature]` function on a CPU that lacks the features is undefined
/// behavior, unless the platform explicitly documents it to be safe. That is why the call
/// needs `unsafe`: the caller must verify that the features are present. Even in a crate built
/// with `-C target-feature=+avx2`, calling `increment_avx2` still needs `unsafe`; only a
/// caller that enables the same features through its own `#[target_feature]` attribute may
/// call it without.
Comment on lines +888 to +893

@workingjubilee workingjubilee Oct 30, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not "documents" here, but "defines". It needs to be a foundational promise. If it was okay today but may not be okay in the future, we couldn't rely on it, and vice versa, it needs to have been okay since the very start of the platform defined by the target. This is why wasm is allowed to be different, because invalid instructions must have a reliable behavior of being rejected, from the beginning of wasm onward.

Further, the feature doesn't need to just be physically available on the CPU. People must perform runtime detection instead of assuming a feature is enabled because e.g. the behavior is equally arbitrary if the OS refuses to enable the feature, from our perspective, though more typically it at least incurs an invalid instruction trap since the instruction does get parsed correctly.

...However, as noted by the Gracemont comment, this is not guaranteed, as a feature being disabled despite being physically available can be due to kernel software detecting a per-core ISA difference and thus refusing to enable it, and so using those instructions can still reach a hardware UB case even if some cores parse it correctly.

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would invert this segment and start at the conclusion: Describe what is needed to be sound and correct, and then add the recursive exemption, and then add the wasm exemption (as that is the only platform that has such an exemption). Then -Ctarget-feature doesn't even come up because we are only describing what you must do. It's a red herring.

Comment on lines +890 to +893

@workingjubilee workingjubilee Oct 30, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this part is a fairly arbitrary decision by us so that whether a crate passes unsafety-checking doesn't depend on your specific -Ctarget-feature flags, it has less to do with soundness per se and more to do with making the compiler easy to wield.

View changes since the review

///
/// The value of `enable` is one or more comma-separated feature names, and the names are

@workingjubilee workingjubilee Oct 30, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably show what this looks like.

View changes since the review

/// architecture-specific: `avx2` belongs to x86, `neon` to ARM. The `-C target-feature` flag
Comment on lines +895 to +896

@workingjubilee workingjubilee Oct 30, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels vague to me between "each architecture has a set of names and those names are not assigned to other architectures" and "each name has a specific meaning for each architecture". It's the latter. They are not required to match to a single feature on a single architecture. Many platforms have aes, for example, which indicates the same computation but may have some variance in its implementation details. Also, we can have the same name mean fully different things based on architecture.

View changes since the review

/// accepts the same names crate-wide, and run-time detection has per-architecture macros such
/// as [`is_x86_feature_detected`] and [`is_aarch64_feature_detected`], which callers use to
/// confirm the running CPU has a feature before the call.
Comment on lines +895 to +899

@workingjubilee workingjubilee Oct 30, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like this leaves slightly vague something that it should probably explicate: the same strings work for both.

In fact, we should probably move this way up, to near the start, and maybe reference rustc --print target-features?

View changes since the review

///
/// Inlining would copy the function's extra instructions into code that runs on any CPU, so
/// the function is never inlined into callers that lack its features and does not combine with
/// [`inline(always)`]. Every dispatched call is a real call, so put a whole loop behind it,
/// not a few instructions. It also does not implement the `Fn` traits, so wrap the checked call
/// in a closure instead.
///
/// For more information, see the Reference on [the `target_feature` attribute].
///
/// [`is_x86_feature_detected`]: ../std/arch/macro.is_x86_feature_detected.html
/// [`is_aarch64_feature_detected`]: ../std/arch/macro.is_aarch64_feature_detected.html
/// [`inline(always)`]: ./attribute.inline.html
/// [the `target_feature` attribute]: ../reference/attributes/codegen.html#the-target_feature-attribute
const _: () = ();
Loading