Move more layouting logic to `rustc_abi` #138158

moulins · 2025-03-07T10:59:49Z

Move all LayoutData-constructing code to rustc_abi:

Infaillible operations get a new LayoutData constructor method;
Faillible ones get a new method on LayoutCalculator.

rustbot · 2025-03-07T10:59:53Z

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

rustbot · 2025-03-07T10:59:54Z

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

jieyouxu · 2025-03-07T11:28:05Z

r? @workingjubilee since you've been trying to clean this up

compiler/rustc_abi/src/layout.rs

workingjubilee · 2025-03-08T00:38:56Z

I walked through the commits checking to see if they passed x.py check and while that's not a strict requirement, there was a small change that a later commit added that fixed things, and it built after I moved that back, so I did.

workingjubilee

I like things overall but I'm a bit perplexed by a few changes.

compiler/rustc_abi/src/layout.rs

compiler/rustc_abi/src/lib.rs

compiler/rustc_abi/src/layout.rs

...either as: - methods on LayoutCalculator, for faillible operations; - constructors on LayoutData, for infaillible ones.

Also properly attaches spans on layouts of non-promoted coroutine locals, which slightly improves the error messages for some coroutine tests.

workingjubilee

Thanks!

workingjubilee · 2025-03-09T02:32:32Z

@bors r+

bors · 2025-03-09T02:32:35Z

📌 Commit 08530d3 has been approved by workingjubilee

It is now in the queue for this repository.

…r=workingjubilee Move more layouting logic to `rustc_abi` Move all `LayoutData`-constructing code to `rustc_abi`: - Infaillible operations get a new `LayoutData` constructor method; - Faillible ones get a new method on `LayoutCalculator`.

Rollup of 16 pull requests Successful merges: - rust-lang#122790 (Apply dllimport in ThinLTO) - rust-lang#136127 (Allow `*const W<dyn A> -> *const dyn A` ptr cast) - rust-lang#136968 (Turn order dependent trait objects future incompat warning into a hard error) - rust-lang#137147 (Add exclude to config.toml) - rust-lang#137319 (Stabilize `const_vec_string_slice`) - rust-lang#137885 (tidy: add triagebot checks) - rust-lang#138040 (compiler: Use `size_of` from the prelude instead of imported) - rust-lang#138052 (strip `-Wlinker-messages` wrappers from `rust-lld` rmake test) - rust-lang#138084 (Use workspace lints for crates in `compiler/`) - rust-lang#138158 (Move more layouting logic to `rustc_abi`) - rust-lang#138160 (depend more on attr_data_structures and move find_attr! there) - rust-lang#138192 (crashes: couple more tests) - rust-lang#138216 (bootstrap: Fix stack printing when a step cycle is detected) - rust-lang#138232 (Reduce verbosity of GCC build log) - rust-lang#138233 (Windows: Don't link std (and run-make) against advapi32, except on win7) - rust-lang#138242 (Revert "Don't test new error messages with the stage 0 compiler") r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#136127 (Allow `*const W<dyn A> -> *const dyn A` ptr cast) - rust-lang#136968 (Turn order dependent trait objects future incompat warning into a hard error) - rust-lang#137319 (Stabilize `const_vec_string_slice`) - rust-lang#137885 (tidy: add triagebot checks) - rust-lang#138040 (compiler: Use `size_of` from the prelude instead of imported) - rust-lang#138084 (Use workspace lints for crates in `compiler/`) - rust-lang#138158 (Move more layouting logic to `rustc_abi`) - rust-lang#138160 (depend more on attr_data_structures and move find_attr! there) - rust-lang#138192 (crashes: couple more tests) - rust-lang#138216 (bootstrap: Fix stack printing when a step cycle is detected) - rust-lang#138232 (Reduce verbosity of GCC build log) - rust-lang#138242 (Revert "Don't test new error messages with the stage 0 compiler") r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#138158 - moulins:move-layout-to-rustc_abi, r=workingjubilee Move more layouting logic to `rustc_abi` Move all `LayoutData`-constructing code to `rustc_abi`: - Infaillible operations get a new `LayoutData` constructor method; - Faillible ones get a new method on `LayoutCalculator`.

Kobzol · 2025-03-09T20:28:50Z

@rust-timer build ffbe058

rust-timer · 2025-03-10T02:43:11Z

Finished benchmarking commit (ffbe058): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 4.4%, secondary 3.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	4.4%	[4.3%, 4.5%]	2
Regressions ❌ (secondary)	3.2%	[0.9%, 5.1%]	28
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.4%	[4.3%, 4.5%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 765.417s -> 765.492s (0.01%)
Artifact size: 362.05 MiB -> 362.03 MiB (-0.00%)

rustbot assigned wesleywiser Mar 7, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 7, 2025

rustbot assigned workingjubilee and unassigned wesleywiser Mar 7, 2025

workingjubilee reviewed Mar 7, 2025

View reviewed changes

compiler/rustc_abi/src/layout.rs Outdated Show resolved Hide resolved

moulins force-pushed the move-layout-to-rustc_abi branch from 790d71f to dcb26d0 Compare March 7, 2025 21:14

workingjubilee force-pushed the move-layout-to-rustc_abi branch from dcb26d0 to d02af3b Compare March 7, 2025 23:44

workingjubilee requested changes Mar 8, 2025

View reviewed changes

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 Mar 8, 2025

moulins added 4 commits March 8, 2025 12:27

Remove most manual LayoutData creations and move them to rustc_abi

9917173

...either as: - methods on LayoutCalculator, for faillible operations; - constructors on LayoutData, for infaillible ones.

Move SIMD layout logic to rustc_abi

e69491a

Use rustc_abi code for SIMD layout in rust-analyzer

f79f3d3

Refactor coroutine layout logic to precompute all sublayouts

b8a2170

Also properly attaches spans on layouts of non-promoted coroutine locals, which slightly improves the error messages for some coroutine tests.

moulins force-pushed the move-layout-to-rustc_abi branch from d02af3b to 578f3b9 Compare March 8, 2025 11:49

This comment has been minimized.

Sign in to view

Move coroutine layout logic to rustc_abi

08530d3

moulins force-pushed the move-layout-to-rustc_abi branch from 578f3b9 to 08530d3 Compare March 8, 2025 15:17

workingjubilee approved these changes Mar 9, 2025

View reviewed changes

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 Mar 9, 2025

jieyouxu mentioned this pull request Mar 9, 2025

Rollup of 16 pull requests #138255

Closed

matthiaskrgr mentioned this pull request Mar 9, 2025

Rollup of 12 pull requests #138267

Merged

bors merged commit bfa1a62 into rust-lang:master Mar 9, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 9, 2025

moulins deleted the move-layout-to-rustc_abi branch March 9, 2025 15:45

This comment has been minimized.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move more layouting logic to `rustc_abi` #138158

Move more layouting logic to `rustc_abi` #138158

moulins commented Mar 7, 2025 •

edited

Loading

rustbot commented Mar 7, 2025

rustbot commented Mar 7, 2025

jieyouxu commented Mar 7, 2025

workingjubilee commented Mar 8, 2025

workingjubilee left a comment

This comment has been minimized.

workingjubilee left a comment

workingjubilee commented Mar 9, 2025

bors commented Mar 9, 2025

Kobzol commented Mar 9, 2025

This comment has been minimized.

rust-timer commented Mar 10, 2025

Move more layouting logic to rustc_abi #138158

Move more layouting logic to rustc_abi #138158

Conversation

moulins commented Mar 7, 2025 • edited Loading

rustbot commented Mar 7, 2025

rustbot commented Mar 7, 2025

jieyouxu commented Mar 7, 2025

workingjubilee commented Mar 8, 2025

workingjubilee left a comment

Choose a reason for hiding this comment

This comment has been minimized.

workingjubilee left a comment

Choose a reason for hiding this comment

workingjubilee commented Mar 9, 2025

bors commented Mar 9, 2025

Kobzol commented Mar 9, 2025

This comment has been minimized.

rust-timer commented Mar 10, 2025

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Move more layouting logic to `rustc_abi` #138158

Move more layouting logic to `rustc_abi` #138158

moulins commented Mar 7, 2025 •

edited

Loading