compiler: untangle SIMD alignment assumptions by workingjubilee · Pull Request #137256 · rust-lang/rust

workingjubilee · 2025-02-19T04:56:31Z

There were a number of puzzling assumptions being made about SIMD types and their layout that I have corrected in this diff. These are mostly no-op edits in actual fact, but they do subtly alter a pair of checks in our invariant-checking and union layout computation that rested on those peculiar assumptions. Those unfortunately stand in the way of any further actual fixes. I submit this for review, even though it's not clearly motivated without its followups, because it should still be possible to independently conclude whether this is correct.

One of these checks is yours, I believe?
r? RalfJung

rustbot · 2025-02-19T04:56:40Z

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

RalfJung · 2025-02-19T07:59:58Z

Sorry, I don't have any knowledge of the vast majority of the code being touched here. I won't be able to review this. I'm not even sure who our experts here are, maybe @bjorn3 or @nikic ?

r? compiler

RalfJung · 2025-02-19T09:04:27Z

Looking at #130402, repr(align) was also implicated in the issues around Scalar/ScalarPair that I mentioned above. For those types, the fix is to use BackendRepr::Aggregate instead when repr(align) is present. Maybe that is also the right fix for SIMD types? Or maybe those can have their alignment overridden in Layout without causing havoc in the rest of the compiler?

RalfJung · 2025-02-19T09:05:04Z

(Unrelated, maybe we should rename BackendRepr::Vector to BackendRepr::SimdVector, since for most Rustaceans "vector" will refer to Vec.)

workingjubilee · 2025-02-19T09:11:15Z

Looking at #130402, repr(align) was also implicated in the issues around Scalar/ScalarPair that I mentioned above. For those types, the fix is to use BackendRepr::Aggregate instead when repr(align) is present. Maybe that is also the right fix for SIMD types? Or maybe those can have their alignment overridden in Layout without causing havoc in the rest of the compiler?

What exactly is "causing havoc in the rest of the compiler"? The code that I am adjusting is the layout code.

workingjubilee · 2025-02-19T09:43:09Z

Ah, I see what I missed now, GitHub's UI improvements are "even better" when high latency is involved!

RalfJung · 2025-02-19T09:51:57Z

Yeah sorry I was referring to the issues mentioned here.

workingjubilee · 2025-02-19T11:37:25Z

will revise accordingly

@rustbot author

workingjubilee · 2025-02-19T23:16:11Z

@rustbot ready

workingjubilee · 2025-02-19T23:17:21Z

Will conflict with #136985 so should not be put in the queue yet but is otherwise ready for review

workingjubilee · 2025-02-20T18:36:57Z

@bors r=bjorn3,RalfJung

bors · 2025-02-20T18:37:00Z

📌 Commit f964368 has been approved by bjorn3,RalfJung

It is now in the queue for this repository.

workingjubilee · 2025-02-20T18:37:30Z

I swear I have incorporated that diff like five times.

rustbot · 2025-02-20T20:11:45Z

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

workingjubilee · 2025-02-20T20:13:12Z

huh, unexpected. 🫠

oh well, rebased on #136985 since it will break this anyways

No functional changes.

This pair of fn was introduced to perform invariant checks for scalars. Their current behavior doesn't mesh as well with checking SIMD types, so change the name of the fn to reflect their actual use-case and refactor the corresponding checks. Also simplify the returns from Option<AbiAndPrefAlign> to Option<Align>, because every site was mapping away the "preferred" alignment anyways.

Introduce a pair of functions that actually describe what they do, because it wasn't clear the alignment is sometimes a forgery.

This is only to fix the build.

workingjubilee · 2025-02-21T03:56:01Z

@bors r=bjorn3,RalfJung

bors · 2025-02-21T03:56:04Z

📌 Commit 46f7a7d has been approved by bjorn3,RalfJung

It is now in the queue for this repository.

…-assumptions, r=bjorn3,RalfJung compiler: untangle SIMD alignment assumptions There were a number of puzzling assumptions being made about SIMD types and their layout that I have corrected in this diff. These are mostly no-op edits in actual fact, but they do subtly alter a pair of checks in our invariant-checking and union layout computation that rested on those peculiar assumptions. Those unfortunately stand in the way of any further actual fixes. I submit this for review, even though it's not clearly motivated without its followups, because it should still be possible to independently conclude whether this is correct.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#135501 (Inject `compiler_builtins` during postprocessing and ensure it is made private) - rust-lang#136543 (intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic) - rust-lang#137121 (stabilize `(const_)ptr_sub_ptr`) - rust-lang#137180 (Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes) - rust-lang#137256 (compiler: untangle SIMD alignment assumptions) - rust-lang#137298 (Check signature WF when lowering MIR body) - rust-lang#137415 (Remove invalid suggestion of into_iter for extern macro) r? `@ghost` `@rustbot` modify labels: rollup

…-assumptions, r=bjorn3,RalfJung compiler: untangle SIMD alignment assumptions There were a number of puzzling assumptions being made about SIMD types and their layout that I have corrected in this diff. These are mostly no-op edits in actual fact, but they do subtly alter a pair of checks in our invariant-checking and union layout computation that rested on those peculiar assumptions. Those unfortunately stand in the way of any further actual fixes. I submit this for review, even though it's not clearly motivated without its followups, because it should still be possible to independently conclude whether this is correct.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#135501 (Inject `compiler_builtins` during postprocessing and ensure it is made private) - rust-lang#136543 (intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic) - rust-lang#137121 (stabilize `(const_)ptr_sub_ptr`) - rust-lang#137180 (Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes) - rust-lang#137256 (compiler: untangle SIMD alignment assumptions) - rust-lang#137383 (stabilize `unsigned_is_multiple_of`) - rust-lang#137415 (Remove invalid suggestion of into_iter for extern macro) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#135501 (Inject `compiler_builtins` during postprocessing and ensure it is made private) - rust-lang#137121 (stabilize `(const_)ptr_sub_ptr`) - rust-lang#137180 (Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes) - rust-lang#137256 (compiler: untangle SIMD alignment assumptions) - rust-lang#137383 (stabilize `unsigned_is_multiple_of`) - rust-lang#137415 (Remove invalid suggestion of into_iter for extern macro) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#137256 - workingjubilee:untangle-vector-abi-assumptions, r=bjorn3,RalfJung compiler: untangle SIMD alignment assumptions There were a number of puzzling assumptions being made about SIMD types and their layout that I have corrected in this diff. These are mostly no-op edits in actual fact, but they do subtly alter a pair of checks in our invariant-checking and union layout computation that rested on those peculiar assumptions. Those unfortunately stand in the way of any further actual fixes. I submit this for review, even though it's not clearly motivated without its followups, because it should still be possible to independently conclude whether this is correct.

rustbot assigned RalfJung Feb 19, 2025

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 19, 2025

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 19, 2025

This was referenced Feb 19, 2025
Internal SIMD layout specification is not flexible enough for SPIR-V #130405
Open
When a type is #[repr(simd)], #[repr(align(N))] annotations are ignored. #130402
Open

RalfJung reviewed Feb 19, 2025
View reviewed changes

Comment threadcompiler/rustc_ty_utils/src/layout/invariant.rs Outdated

rustbot assigned oli-obk and unassigned RalfJung Feb 19, 2025

RalfJung reviewed Feb 19, 2025
View reviewed changes

Comment threadcompiler/rustc_abi/src/lib.rs

RalfJung reviewed Feb 19, 2025
View reviewed changes

Comment threadcompiler/rustc_abi/src/lib.rs Outdated

RalfJung reviewed Feb 19, 2025
View reviewed changes

Comment threadcompiler/rustc_abi/src/lib.rs Outdated

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 19, 2025

workingjubilee force-pushed the untangle-vector-abi-assumptions branch from 3a5fe77 to a96ec2aCompare February 19, 2025 23:14

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 19, 2025

This comment has been minimized.
Sign in to view

workingjubilee force-pushed the untangle-vector-abi-assumptions branch from a96ec2a to efb89c9Compare February 19, 2025 23:24

This comment has been minimized.
Sign in to view

workingjubilee force-pushed the untangle-vector-abi-assumptions branch from efb89c9 to 1f1bd7cCompare February 20, 2025 02:33

workingjubilee force-pushed the untangle-vector-abi-assumptions branch from b611749 to f964368Compare February 20, 2025 18:36

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 20, 2025

workingjubilee force-pushed the untangle-vector-abi-assumptions branch from f964368 to 7173d84Compare February 20, 2025 20:11

workingjubilee added 4 commits February 20, 2025 19:55

compiler: Align::max_for_offset -> Align::max_aligned_factor
efff15a
No functional changes.

compiler: split vector_align into cabi and llvmlike
79f41c7
Introduce a pair of functions that actually describe what they do, because it wasn't clear the alignment is sometimes a forgery.

rust-analyzer: use new function in (incorrect) layout computation
46f7a7d
This is only to fix the build.

workingjubilee force-pushed the untangle-vector-abi-assumptions branch from 7173d84 to 46f7a7dCompare February 21, 2025 03:55

matthiaskrgr mentioned this pull request Feb 22, 2025
Rollup of 8 pull requests #137431
Closed

matthiaskrgr mentioned this pull request Feb 22, 2025
Rollup of 7 pull requests #137433
Closed

matthiaskrgr mentioned this pull request Feb 22, 2025
Rollup of 7 pull requests #137436
Closed

matthiaskrgr mentioned this pull request Feb 22, 2025
Rollup of 6 pull requests #137446
Merged

bors merged commit 86008ea into rust-lang:masterFeb 23, 2025

rustbot added this to the 1.87.0 milestone Feb 23, 2025

workingjubilee deleted the untangle-vector-abi-assumptions branch March 5, 2025 04:31

Uh oh!

Uh oh!

Conversation

workingjubilee commented Feb 19, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 19, 2025

Uh oh!

Uh oh!

RalfJung commented Feb 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RalfJung commented Feb 19, 2025

Uh oh!

RalfJung commented Feb 19, 2025

Uh oh!

workingjubilee commented Feb 19, 2025

Uh oh!

workingjubilee commented Feb 19, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Feb 19, 2025

Uh oh!

workingjubilee commented Feb 19, 2025

Uh oh!

workingjubilee commented Feb 19, 2025

Uh oh!

workingjubilee commented Feb 19, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

workingjubilee commented Feb 20, 2025

Uh oh!

bors commented Feb 20, 2025

Uh oh!

workingjubilee commented Feb 20, 2025

Uh oh!

rustbot commented Feb 20, 2025

Uh oh!

workingjubilee commented Feb 20, 2025

Uh oh!

workingjubilee commented Feb 21, 2025

Uh oh!

bors commented Feb 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

workingjubilee commented Feb 19, 2025•
edited
Loading

workingjubilee commented Feb 19, 2025•
edited
Loading