Repository navigation
Add documentation for the target_feature attribute
#160218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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. | ||
| /// | ||
| /// 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Comment on lines
+890
to
+893
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| /// | ||
| /// The value of `enable` is one or more comma-separated feature names, and the names are | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably show what this looks like. |
||
| /// architecture-specific: `avx2` belongs to x86, `neon` to ARM. The `-C target-feature` flag | ||
|
Comment on lines
+895
to
+896
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| /// 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| /// | ||
| /// 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 _: () = (); | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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#L398It 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:
Unfortunately
crt-staticis 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