Move loop_hint_attrs from Terminator into TerminatorKind::Goto

[joshtriplett]

Only the Goto variant uses loop_hint_attrs, so other variants shouldn't pay the cost. This shrinks Terminator and thus BasicBlockData by 8 bytes, and also simplifies many users of Terminator.

Every byte saved from BasicBlockData makes a big difference in max-rss. For aws-sdk-ec2 full check, this saves 34.1 MiB (~0.62% of its peak RSS)

Add a TerminatorKind::goto helper function for convenient construction with a target and no hints. With that helper available, most of this commit is simple, straightforward, mechanical translations of construction or pattern matching. The only notable changes are:

• compiler/rustc_codegen_ssa/src/mir/block.rs actually uses the moved hints.
• compiler/rustc_middle/src/mir/mod.rs updates the size assertions.
• compiler/rustc_mir_build/src/builder/cfg.rs adds/updates helpers.
• compiler/rustc_mir_build/src/builder/expr/into.rs has the one place that creates a Goto with non-empty hints.

---

I have several more potential improvements to basic block sizes, but this one was an easy starting point.

[rustbot]

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @vakaras

Some changes occurred in coverage instrumentation.

cc @Zalathar

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

cc @bjorn3

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

This PR changes rustc_public

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

[rustbot]

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler, mir, mir-opt
• compiler, mir, mir-opt expanded to 77 candidates
• Random selection from 19 candidates

[joshtriplett]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: e5d77f0 (e5d77f0e45fa793b37386951f1a9c0ab5a245a07)
Base parent: 8d1a764 (8d1a76430406c877b35d0b627e7f796dcf0dfeca)

[rust-timer]

Finished benchmarking commit (e5d77f0): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary -1.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.5%	[-1.5%, -1.5%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (primary -1.5%, secondary -0.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.3%	[1.3%, 1.3%]	1
Regressions ❌ (secondary)	9.2%	[6.6%, 11.9%]	2
Improvements ✅ (primary)	-2.5%	[-2.7%, -2.3%]	3
Improvements ✅ (secondary)	-4.2%	[-6.7%, -2.2%]	6
All ❌✅ (primary)	-1.5%	[-2.7%, 1.3%]	4

Binary size

Results (primary -0.1%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.2%, -0.0%]	58
Improvements ✅ (secondary)	-0.1%	[-0.2%, -0.0%]	52
All ❌✅ (primary)	-0.1%	[-0.2%, -0.0%]	58

Bootstrap: 489.893s -> 486.175s (-0.76%)
Artifact size: 408.60 MiB -> 406.23 MiB (-0.58%)

[mejrs]

@saethlin has talked about wanting to add a loop vectorization attribute, I think it's intended to live in Terminator as well.

I guess my question is: if that change would require undoing basically everything here there's not much point in doing this.

[panstromek]

I was looking into this before as well and left it alone because I expected there's an intent to use it for other attributes later. I really don't like that the array is there, though, I'd merge this even if just for the metadata size reductions. It feels very wasteful to keep a field that is almost always empty for a feature that's almost never used (and actually doesn't even exist atm 😄)

[joshtriplett]

I saw one mention of adding loop vectorization hints, but I would expect those to go on Goto as well. When that happens, I expect the second commit to be largely unchanged, and the first to change from UnrollAttr to LoopHintAttr, an enum containing variants for Unroll and Vectorize.

@saethlin, does that sound right?

[saethlin]

Oh yeah I should really get around to adding vectorization hints...

Given the difficulty that we have had with name collisions with #[unroll] I don't think it make sense to deny people a perf improvement while we untangle how to expose such attributes. There's also clearly some good cleanups in this PR, though of course I dislike how difficult it is to make changes like this to the codebase.

r? saethlin
@bors r+

[rust-bors]

📌 Commit ec45ecf has been approved by saethlin

It is now in the queue for this repository.