Enable MIR inlining for generators too

[cjgillot]

This is a tentative to enable MIR inlining on generators too.

This PR proceeds in 2 steps:

1. the generator information is separated from the MIR body itself, to be computed using two separate queries: mir_generator_lowered and mir_generator_info;
2. we perform inlining on the generator body (generator_resume function).

This PR took the opportunity to simplify the generated generator_drop code, since it won't be optimized on its own.

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[rust-highfive]

r? @fee1-dead

(rust-highfive has picked a reviewer for you, use r? to override)

[cjgillot]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit a4a8252e576a22eae530dc638bf98102f566f4d8 with merge ac9b18299e23be0ac59586be05a93e8e9e0f54d8...

[bors]

☀️ Try build successful - checks-actions
Build commit: ac9b18299e23be0ac59586be05a93e8e9e0f54d8 (ac9b18299e23be0ac59586be05a93e8e9e0f54d8)

[rust-timer]

Queued ac9b18299e23be0ac59586be05a93e8e9e0f54d8 with parent c11207e, future comparison URL.

[rust-timer]

Finished benchmarking commit (ac9b18299e23be0ac59586be05a93e8e9e0f54d8): comparison url.

Instruction count

• Primary benchmarks: 😿 relevant regressions found
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions 😿 (primary)	0.5%	0.9%	41
Regressions 😿 (secondary)	0.9%	4.1%	14
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-1.7%	-2.1%	5
All 😿🎉 (primary)	0.5%	0.9%	41

Max RSS (memory usage)

Results

• Primary benchmarks: mixed results
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions 😿 (primary)	1.8%	2.2%	4
Regressions 😿 (secondary)	2.9%	3.1%	2
Improvements 🎉 (primary)	-1.8%	-2.2%	10
Improvements 🎉 (secondary)	-3.3%	-5.9%	4
All 😿🎉 (primary)	-0.8%	-2.2%	14

Cycles

Results

• Primary benchmarks: no relevant changes found
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	5.3%	5.3%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-3.1%	-3.9%	5
All 😿🎉 (primary)	N/A	N/A	0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.
Warning ⚠: The following benchmark(s) failed to build:

• deeply-nested-multi

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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Footnotes

1. the arithmetic mean of the percent change ↩ ↩² ↩³

2. number of relevant changes ↩ ↩² ↩³

[fee1-dead]

r? rust-lang/compiler

[cjgillot]

The issue with deeply-nested-multi benchmark is only a recursion limit issue.

[bors]

☔ The latest upstream changes (presumably #99780) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #100089) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #99908) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #99946) made this pull request unmergeable. Please resolve the merge conflicts.

[cjgillot]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 24722aac770190c699585342078635d9e478696b with merge f33592845aa45f6bdcf5e797c6209813fe179704...

[cjgillot]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 9b0c4be7988c3d7906f99e86af021fd9fe6199dd with merge 22ba33181fb0c3e6588779a40a43dae8865e3993...

[bors]

☀️ Try build successful - checks-actions
Build commit: 22ba33181fb0c3e6588779a40a43dae8865e3993 (22ba33181fb0c3e6588779a40a43dae8865e3993)

[rust-timer]

Queued 22ba33181fb0c3e6588779a40a43dae8865e3993 with parent d0e1491, future comparison URL.

[rust-timer]

Finished benchmarking commit (22ba33181fb0c3e6588779a40a43dae8865e3993): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Warning ⚠: The following benchmark(s) failed to build:

• deeply-nested-multi

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	0.5%	[0.2%, 0.9%]	57
Regressions ❌ (secondary)	0.9%	[0.2%, 4.1%]	22
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.0%	[-2.5%, -1.3%]	5
All ❌✅ (primary)	0.5%	[0.2%, 0.9%]	57

Max RSS (memory usage)

Results

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.

	mean1	range	count2
Regressions ❌ (primary)	1.8%	[0.5%, 2.5%]	5
Regressions ❌ (secondary)	2.7%	[1.4%, 5.0%]	7
Improvements ✅ (primary)	-3.6%	[-4.1%, -3.0%]	2
Improvements ✅ (secondary)	-2.2%	[-2.8%, -1.7%]	3
All ❌✅ (primary)	0.2%	[-4.1%, 2.5%]	7

Cycles

Results

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.

	mean1	range	count2
Regressions ❌ (primary)	2.0%	[2.0%, 2.0%]	1
Regressions ❌ (secondary)	3.3%	[3.3%, 3.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.0%	[2.0%, 2.0%]	1

Footnotes

1. the arithmetic mean of the percent change ↩ ↩² ↩³

2. number of relevant changes ↩ ↩² ↩³

[rustbot]

This PR changes MIR

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

[bors]

☔ The latest upstream changes (presumably #101086) made this pull request unmergeable. Please resolve the merge conflicts.

[cjgillot]

From perf results, this does not seem to be worth it.

[JakobDegen]

I agree that we should maybe not be turning this on right now, but I'd also caution against just using perf results here. Compile time perf is not the same as runtime perf, and also in general this will enable other optimizations that we may take advantage of in the future. I expect that we will want to revive this PR at some point.

[wesleywiser]

Agreed, I think there might be code quality reasons to enable this in the future. I do wonder though if inlining into generators is what we want. It seems plausible to me that keeping the generator code as small as possible might enable better inlining of the generator into its callsite which could also be beneficial to runtime performance.

[JakobDegen]

Hmm, what makes you think that generators are more affected by this than any other function?