Respect verify-llvm-ir option in the backend

[nikic]

We are currently unconditionally verifying the LLVM IR in the backend (twice), ignoring the value of the verify-llvm-ir option. This has substantial compile-time impact for debug builds.

[nikic]

@bors try @rust-timer queue

[bors]

⌛ Trying commit d3ad000 with merge 203d6bf...

[bors]

☀️ Try build successful - checks-actions
Build commit: 203d6bf (203d6bf2665654d024ab811c9adfbecfba5e1f21)

[rust-timer]

Finished benchmarking commit (203d6bf): comparison URL.

Overall result: ✅ improvements - 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 is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.4%	[0.3%, 0.4%]	3
Improvements ✅ (primary)	-2.3%	[-8.2%, -0.2%]	88
Improvements ✅ (secondary)	-3.0%	[-9.0%, -0.3%]	39
All ❌✅ (primary)	-2.3%	[-8.2%, -0.2%]	88

Max RSS (memory usage)

Results (primary -3.4%, secondary -1.1%)

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)	-	-	0
Regressions ❌ (secondary)	1.7%	[1.7%, 1.7%]	2
Improvements ✅ (primary)	-3.4%	[-5.1%, -1.6%]	13
Improvements ✅ (secondary)	-4.0%	[-4.9%, -3.1%]	2
All ❌✅ (primary)	-3.4%	[-5.1%, -1.6%]	13

Cycles

Results (primary -3.5%, secondary -4.8%)

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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.5%	[-7.5%, -0.7%]	46
Improvements ✅ (secondary)	-4.8%	[-9.8%, -1.8%]	22
All ❌✅ (primary)	-3.5%	[-7.5%, -0.7%]	46

Binary size

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

Bootstrap: 795.777s -> 789.433s (-0.80%)
Artifact size: 336.33 MiB -> 336.34 MiB (0.01%)

[nikic]

r? compiler

[Mark-Simulacrum]

@bors r+

r? @Mark-Simulacrum

[bors]

📌 Commit d3ad000 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[bors]

⌛ Testing commit d3ad000 with merge 4b635bd...

[bors]

💔 Test failed - checks-actions

[nikic]

Updated test to explicitly pass -Z verify-llvm-ir. (Most CI jobs that run tests enable this in config.toml...)

@bors try

[bors]

⌛ Trying commit 3275488 with merge fe76bd2...

[nikic]

Fingers crossed that there aren't more tests lurking...

@bors r=Mark-Simulacrum

[bors]

📌 Commit fc9f727 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[bors]

⌛ Testing commit fc9f727 with merge c43d4f5...

[rust-log-analyzer]

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

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

[bors]

💔 Test failed - checks-actions

[nikic]

Huh:

---- pgo::pgo_works stdout ----
running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo build --release`
thread 'pgo::pgo_works' panicked at tests/testsuite/pgo.rs:77:10:

test failed running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/cargo build --release`
error: process exited with code 101 (expected 0)
--- stdout

--- stderr
   Compiling foo v0.0.0 (/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/cit/t2375/foo)
error[E0463]: can't find crate for `profiler_builtins`
  |
  = note: the compiler may have been built without the profiler runtime

For more information about this error, try `rustc --explain E0463`.
error: could not compile `foo` (bin "foo") due to 1 previous error

Doesn't look related to this change. It looks like this test was pulled in by a cargo update in the directly preceding merge #133654. cc @weihanglo Looks like the test does not account for Rust being built without profiling support? Though I wonder why this didn't fail in the bors run that merged the cargo update...

[nikic]

I think I know what happened here. #133654 as a cargo-only change used download-rustc, which appears to have downloaded a rustc with profiling support and the test worked. Then this PR changed the compiler, so we don't use download-rustc and build our own, with profiling support disabled.

So I think there are multiple issues here:

• The cargo test does not support rust without profiling support.
• We should not be using download-rustc when updating cargo -- can't the cargo update potentially impact the rust build?
• We should make sure that the downloaded rustc has the same profiler/sanitizer/etc configuration as the local configuration.

cc @onur-ozkan

[onur-ozkan]

We should not be using download-rustc when updating cargo

Sounds right to me.

We should make sure that the downloaded rustc has the same profiler/sanitizer/etc configuration as the local configuration.

We do check if CI rustc has the same configuration with the local one and if they don't match we build the in-tree compiler.

[nikic]

We should not be using download-rustc when updating cargo

Sounds right to me.

We should make sure that the downloaded rustc has the same profiler/sanitizer/etc configuration as the local configuration.

We do check if CI rustc has the same configuration with the local one and if they don't match we build the in-tree compiler.

As far as I can tell, the profiler option is currently not taken into account (it's not listed in

rust/src/bootstrap/src/core/config/config.rs

Line 3132 in 76f3ff6

let Rust {

in either of the lists).

[RalfJung]

We do check if CI rustc has the same configuration with the local one and if they don't match we build the in-tree compiler.

Evidently that does not work, or else #133654 would have failed.

EDIT: opened #133675 to track this.

[onur-ozkan]

We do check if CI rustc has the same configuration with the local one and if they don't match we build the in-tree compiler.

Evidently that does not work, or else #133654 would have failed.

EDIT: opened #133675 to track this.

That PR doesn't change any contig at all? I guess we are on different points, I am talking about config.toml here.

[nikic]

@bors retry

[weihanglo]

Or some tests should only run on Cargo's CI, to make everybody happier?

[RalfJung]

Cargo might not be the only component sensitive to whether rustc is built with profiling or not. So I don't think we should paper over this issue by running the test only in cargo CI.

[bors]

⌛ Testing commit fc9f727 with merge 8ac313b...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 8ac313b to master...

[rust-timer]

Finished benchmarking commit (8ac313b): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.3%	[-8.1%, -0.2%]	89
Improvements ✅ (secondary)	-2.9%	[-9.8%, -0.3%]	41
All ❌✅ (primary)	-2.3%	[-8.1%, -0.2%]	89

Max RSS (memory usage)

Results (primary -3.9%, secondary 0.0%)

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)	-	-	0
Regressions ❌ (secondary)	1.4%	[1.3%, 1.5%]	3
Improvements ✅ (primary)	-3.9%	[-7.3%, -1.6%]	12
Improvements ✅ (secondary)	-4.1%	[-4.1%, -4.1%]	1
All ❌✅ (primary)	-3.9%	[-7.3%, -1.6%]	12

Cycles

Results (primary -3.1%, secondary -4.6%)

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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.1%	[-7.4%, -0.7%]	54
Improvements ✅ (secondary)	-4.6%	[-9.6%, -1.0%]	22
All ❌✅ (primary)	-3.1%	[-7.4%, -0.7%]	54

Binary size

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

Bootstrap: 774.422s -> 769.103s (-0.69%)
Artifact size: 332.10 MiB -> 332.14 MiB (0.01%)