-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Detect and optimize SIMD-like struct #146049
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
Conversation
r? @wesleywiser
rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r?
to explicitly pick a reviewer
The job tidy
failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
[TIMING:end] tool::ToolBuild { build_compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu, tool: "tidy", path: "src/tools/tidy", mode: ToolBootstrap, source_type: InTree, extra_features: [], allow_features: "", cargo_args: [], artifact_kind: Binary } -- 9.892
[TIMING:end] tool::Tidy { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu } -- 0.000
fmt check
Diff in /checkout/compiler/rustc_ty_utils/src/layout.rs:569:
// SIMD-like struct detection and optimization with conservative safety constraints
// Check if this is a user-defined struct like Vector<T, const N: usize>([T; N])
// that should be treated as SIMD-friendly even without #[repr(simd)]
- if def.is_struct()
- && def.variants().len() == 1
+ if def.is_struct()
+ && def.variants().len() == 1
&& def.variant(FIRST_VARIANT).fields.len() == 1
- && def.did().is_local() // Only user crate types
+ && def.did().is_local()
+ // Only user crate types
{
// Safety: exclude system crates by checking crate name
let crate_name = tcx.crate_name(def.did().krate);
Diff in /checkout/compiler/rustc_ty_utils/src/layout.rs:579:
let crate_name_str = crate_name.as_str();
-
+
// Exclude known system/compiler crates
- if !matches!(crate_name_str,
- "core" | "std" | "alloc" | "proc_macro" | "test" |
- "rustc_std_workspace_core" | "rustc_std_workspace_alloc" |
- "compiler_builtins" | "libc" | "unwind" | "panic_abort" |
- "panic_unwind" | "adler2" | "object" | "memchr"
+ if !matches!(
+ crate_name_str,
+ "core"
+ | "std"
+ | "alloc"
+ | "proc_macro"
+ | "test"
+ | "rustc_std_workspace_core"
+ | "rustc_std_workspace_alloc"
+ | "compiler_builtins"
+ | "libc"
+ | "unwind"
+ | "panic_abort"
+ | "panic_unwind"
+ | "adler2"
+ | "object"
+ | "memchr"
) {
let field_ty = def.variant(FIRST_VARIANT).fields[FieldIdx::ZERO].ty(tcx, args);
-
+
// Check if the single field is an array of SIMD-friendly types
if let ty::Array(element_ty, array_len) = field_ty.kind() {
if is_simd_friendly_element_type(*element_ty) {
Diff in /checkout/compiler/rustc_ty_utils/src/layout.rs:593:
- if let Some(len) = extract_const_value(cx, ty, *array_len)?
- .try_to_target_usize(tcx)
+ if let Some(len) =
+ extract_const_value(cx, ty, *array_len)?.try_to_target_usize(tcx)
{
// Common SIMD sizes: 4, 8, 16 (conservative range)
if matches!(len, 4 | 8 | 16) {
Diff in /checkout/compiler/rustc_ty_utils/src/layout.rs:598:
// REQUIRE explicit alignment hint - this is the key safety measure
- let has_simd_alignment = def.repr().align
- .map_or(false, |align| {
+ let has_simd_alignment =
+ def.repr().align.map_or(false, |align| {
let align_bytes = align.bytes();
// Must have 16+ byte alignment to suggest SIMD intent
align_bytes >= 16 && align_bytes.is_power_of_two()
Diff in /checkout/compiler/rustc_ty_utils/src/layout.rs:604:
});
-
+
// Only apply if user explicitly requested SIMD-like alignment
if has_simd_alignment {
let e_ly = cx.layout_of(*element_ty)?;
Diff in /checkout/compiler/rustc_ty_utils/src/layout.rs:609:
- return Ok(map_layout(cx.calc.simd_type(e_ly, len, false))?);
+ return Ok(map_layout(
+ cx.calc.simd_type(e_ly, len, false),
+ )?);
}
}
}
Diff in /checkout/compiler/rustc_ty_utils/src/layout.rs:614:
}
---
// Floating-point types - most common SIMD use case
ty::Float(_) => true,
- // Integer types - common for SIMD
+ // Integer types - common for SIMD
ty::Int(_) | ty::Uint(_) => true,
// Bool can be vectorized in some contexts
ty::Bool => true,
fmt: checked 6344 files
Bootstrap failed while executing `test --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck`
Build completed unsuccessfully in 0:00:45
local time: Sun Aug 31 04:11:39 UTC 2025
Please make the PR description a brief summary of what is fixed, rather than just crosslinking an issue (so we don't need to follow it for context, GH doesn't even link it). Also the commit message only says "Update layout.rs", ideally it should be the same as the description.
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.
This could really use a regression test to demonstrate that the fix indeed fixes the reported issue
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.
Why do these get special treatment?
Name matching specific ecosystem crates to give them special treatment is very hazardous. Occasionally we need to match std crates, for which we have the list at
rust/compiler/rustc_span/src/symbol.rs
Line 2444 in 64a99db
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.
Use a let
chain to flatten this nesting.
If I am understanding this correctly, it looks like it is changing arrays to SIMD vectors if they match a reasonable size/align? This probably makes sense in some cases, but doing it in layout seems way too early: things like ABI calculation rely on the computed layout, saying we have a SIMD type when we don't is probably going to cause confusion somewhere.
Tbh I'm not even certain this is an optimization we should be trying to do on the Rust side or if it's something LLVM has enough information for, what does the IR look like?
Uh oh!
There was an error while loading. Please reload this page.
Fixes: #145248