[perf] Revert FastISel patch

[nikic]

View all comments

This caused a significant compile-time regression for debug builds.

There is another change (llvm/llvm-project#186723) that mitigates that regression, but not fully. Revert it for now.

[nikic]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: af18a79 (af18a79249ebb7cdcf8e745d19085bf1b3c46242, parent: 7e28c7438a7b0c79a09724ba799d32ed461beb72)

[rust-timer]

Finished benchmarking commit (af18a79): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@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)	-1.3%	[-3.2%, -0.2%]	27
Improvements ✅ (secondary)	-1.2%	[-5.0%, -0.2%]	14
All ❌✅ (primary)	-1.3%	[-3.2%, -0.2%]	27

Max RSS (memory usage)

Results (primary -1.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)	-1.1%	[-1.1%, -1.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.1%	[-1.1%, -1.1%]	1

Cycles

Results (primary -2.5%, secondary -3.2%)

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)	-2.5%	[-3.4%, -2.2%]	4
Improvements ✅ (secondary)	-3.2%	[-4.4%, -2.4%]	3
All ❌✅ (primary)	-2.5%	[-3.4%, -2.2%]	4

Binary size

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

Bootstrap: 484.417s -> 484.146s (-0.06%)
Artifact size: 394.97 MiB -> 396.98 MiB (0.51%)

[rustbot]

r? @cuviper

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

[rustbot]

⚠️ Warning ⚠️

• Some commits in this PR modify submodules.

  If this was not intentional, see I changed a submodule on accident in the rustc dev guide.

[nikic]

r? @dianqk

[dianqk]

@bors r+

[rust-bors]

📌 Commit 1a32061 has been approved by dianqk

It is now in the queue for this repository.

[rust-log-analyzer]

The job armhf-gnu failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[rust-bors]

💔 Test for 1481366 failed: CI. Failed job:

• auto - armhf-gnu (web logs, enhanced plaintext logs)

[mati865]

Spurious?

@bors retry

[rust-bors]

☀️ Test successful - CI
Approved by: dianqk
Duration: 3h 20m 48s
Pushing 116458d to main...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 80ad557 (parent) -> 116458d (this PR)

Test differences

Show 3 test diffs

Stage 2

• [run-make] tests/run-make/compressed-debuginfo-zstd: pass -> ignore (ignored if LLVM wasn't build with zstd for ELF section compression or LLVM is not the default codegen backend) (J0)

Additionally, 2 doctest diffs were found. These are ignored, as they are noisy.

Job group index

• J0: x86_64-gnu-nopt

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 116458d0a5ae01cd517cabd2d1aee7f5457018ab --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. x86_64-rust-for-linux: 53m 54s -> 1h 8m (+28.0%)
2. pr-check-1: 35m 51s -> 45m 32s (+27.0%)
3. test-various: 2h 2m -> 2h 32m (+23.7%)
4. x86_64-gnu-tools: 1h 6m -> 1h 20m (+19.9%)
5. x86_64-gnu-miri: 1h 32m -> 1h 51m (+19.7%)
6. armhf-gnu: 1h 33m -> 1h 52m (+19.7%)
7. optional-x86_64-gnu-parallel-frontend: 2h 43m -> 3h 14m (+18.9%)
8. x86_64-gnu-distcheck: 2h 9m -> 2h 31m (+16.9%)
9. pr-check-2: 46m 45s -> 54m 2s (+15.6%)
10. x86_64-gnu: 2h 26m -> 2h 48m (+15.2%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (116458d): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -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)	-1.3%	[-3.2%, -0.1%]	26
Improvements ✅ (secondary)	-1.2%	[-5.1%, -0.2%]	13
All ❌✅ (primary)	-1.3%	[-3.2%, -0.1%]	26

Max RSS (memory usage)

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

Cycles

Results (primary -2.9%, secondary -1.2%)

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)	4.9%	[4.9%, 4.9%]	1
Improvements ✅ (primary)	-2.9%	[-3.4%, -2.3%]	2
Improvements ✅ (secondary)	-4.2%	[-4.4%, -4.1%]	2
All ❌✅ (primary)	-2.9%	[-3.4%, -2.3%]	2

Binary size

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

Bootstrap: 490.149s -> 496.876s (1.37%)
Artifact size: 394.89 MiB -> 394.82 MiB (-0.02%)

[cuviper]

Nominating as a perf fix to go along with the accepted #154344.

@rustbot label beta-nominated