Rollup of 7 pull requests #125824

matthiaskrgr · 2024-05-31T15:05:28Z

Successful merges:

Revert propagation of drop-live information from Polonius #125652 (Revert propagation of drop-live information from Polonius)
Apply x clippy --fix and x fmt on Rustc #125730 (Apply x clippy --fix and x fmt on Rustc)
coverage: Optionally instrument the RHS of lazy logical operators #125756 (coverage: Optionally instrument the RHS of lazy logical operators)
Stop using translate_args in the new solver #125776 (Stop using translate_args in the new solver)
Also InstSimplify &raw* #125796 (Also InstSimplify &raw*)
Also resolve the type of constants, even if we already turned it into an error constant #125807 (Also resolve the type of constants, even if we already turned it into an error constant)
Don't build the rust-demangler binary for coverage tests #125816 (Don't build the rust-demangler binary for coverage tests)

r? @ghost
@rustbot modify labels: rollup

This shunts all the complexity of siphoning off the drop-use facts into `LivenessResults::add_extra_drop_facts()`, which may or may not be a good approach.

When a lazy logical operator (`&&` or `||`) occurs outside of an `if` condition, it normally doesn't have any associated control-flow branch, so we don't have an existing way to track whether it was true or false. This patch adds special code to handle this case, by inserting extra MIR blocks in a diamond shape after evaluating the RHS. This gives us a place to insert the appropriate marker statements, which can then be given their own counters.

We do this for `&*` and `&mut*` already; might as well do it for raw pointers too.

… an error constant

… generic type

The coverage-dump tool already needs `rustc_demangle` for its own purposes, so the amount of extra code needed for a demangle mode is very small.

This avoids the need to build `rust-demangler` when running coverage tests, since we typically need to build `coverage-dump` anyway.

This appears to be the canonical way to build a tool with the stage 0 compiler.

…g, r=oli-obk Revert propagation of drop-live information from Polonius rust-lang#64749 introduced a flow of drop-use data from Polonius to `LivenessResults::add_extra_drop_facts()`, which makes `LivenessResults` agree with Polonius on liveness in the presence of free regions that may be dropped. Later changes accidentally removed this flow. This PR restores it.

…-obk Apply `x clippy --fix` and `x fmt` on Rustc  Just run `x clippy --fix` and `x fmt`, and remove some changes like `impl Default`.

coverage: Optionally instrument the RHS of lazy logical operators (This is an updated version of rust-lang#124644 and rust-lang#124402. Fixes rust-lang#124120.) When `||` or `&&` is used outside of a branching context (such as the condition of an `if`), the rightmost value does not directly influence any branching decision, so branch coverage instrumentation does not treat it as its own true-or-false branch. That is a correct and useful interpretation of “branch coverage”, but might be undesirable in some contexts, as described at rust-lang#124120. This PR therefore adds a new coverage level `-Zcoverage-options=condition` that behaves like branch coverage, but also adds additional branch instrumentation to the right-hand-side of lazy boolean operators. --- As discussed at rust-lang#124120 (comment), this is mainly intended as an intermediate step towards fully-featured MC/DC instrumentation. It's likely that we'll eventually want to remove this coverage level (rather than stabilize it), either because it has been incorporated into MC/DC instrumentation, or because it's getting in the way of future MC/DC work. The main appeal of landing it now is so that work on tracking conditions can proceed concurrently with other MC/DC-related work. ````@rustbot```` label +A-code-coverage

…lcnr Stop using `translate_args` in the new solver It was unnecessary and also sketchy, since it was doing an out-of-search-graph fulfillment loop. Added a test for the only really minor subtlety of translating args, though not sure if it was being tested before, though I wouldn't be surprised if it wasn't. r? lcnr

…-obk Also InstSimplify `&raw*` We do this for `&*` and `&mut*` already; might as well do it for raw pointers too. r? mir-opt

…piler-errors Also resolve the type of constants, even if we already turned it into an error constant error constants can still have arbitrary types, and in this case it was turned into an error constant because there was an infer var in the *type* not the *const*. fixes rust-lang#125760

Don't build the `rust-demangler` binary for coverage tests The coverage-run tests invoke `llvm-cov`, which requires us to specify a command-line demangler that it can use to demangle Rust symbol names. Historically this used `src/tools/rust-demangler`, which means that we currently build two different command-line tools to help with the coverage tests (`rust-demangler` and `coverage-dump`). However, it occurred to me that if we add a demangler mode to `coverage-dump` (which is only a handful of lines and no extra dependencies), then we only need to build one helper binary for the coverage tests, and there is no need for tests to build `rust-demangler` at all. --- Note that the `rust-demangler` binary is separate from the `rustc-demangle` crate (which both `rust-demangler` and `coverage-dump` use as a dependency to do the actual demangling). --- So the main benefits/motivations here are: - Slightly faster builds after a fresh checkout or bootstrap bump. - Making it clear that currently no tests actually need the `rust-demangler` binary, since the coverage tests can use their own tool instead.

matthiaskrgr · 2024-05-31T15:05:40Z

@bors r+ rollup=never p=7

bors · 2024-05-31T15:05:42Z

📌 Commit df9cd17 has been approved by matthiaskrgr

It is now in the queue for this repository.

bors · 2024-05-31T16:13:00Z

⌛ Testing commit df9cd17 with merge ada5e2c...

bors · 2024-05-31T18:18:09Z

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing ada5e2c to master...

rust-timer · 2024-05-31T18:20:06Z

📌 Perf builds for each rolled up PR:

PR#	Message	Perf Build Sha
#125652	Revert propagation of drop-live information from Polonius	`eede364e9abe1c452247e1137ce21d77ac5872ce` (link)
#125730	Apply `x clippy --fix` and `x fmt` on Rustc	`362a93b355950a15f6b6e3c77830a65362e9bd1c` (link)
#125756	coverage: Optionally instrument the RHS of lazy logical ope…	`9f31206b8c3478ecfa739fa41dcb071cfde5fda8` (link)
#125776	Stop using `translate_args` in the new solver	`6e0c0fef13fd11ccf295f6e94817b810f9d7ca9a` (link)
#125796	Also InstSimplify `&raw*`	`0b25619ce8917aa68bb979f14527b76785da8939` (link)
#125807	Also resolve the type of constants, even if we already turn…	`110d3143be564c2ba51f446c48f9485b1663ff85` (link)
#125816	Don't build the `rust-demangler` binary for coverage tests	`9dedc63dce241c902f15187cd6ae12f745ea39f7` (link)

previous master: 2a2c29aafa

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

rust-timer · 2024-05-31T19:35:07Z

Finished benchmarking commit (ada5e2c): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

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

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.5%, 1.1%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.4%	[-0.4%, 1.1%]	4

Max RSS (memory usage)

Results (primary 2.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)	7.2%	[1.6%, 15.2%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-5.7%	[-6.4%, -5.0%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.1%	[-6.4%, 15.2%]	5

Cycles

Results (primary 0.9%)

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.9%	[0.7%, 1.3%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.9%	[0.7%, 1.3%]	4

Binary size

Results (primary 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.1%	[0.0%, 0.4%]	24
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.2%, -0.0%]	12
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-0.2%, 0.4%]	36

Bootstrap: 669.181s -> 672.771s (0.54%)
Artifact size: 318.82 MiB -> 318.83 MiB (0.00%)

pnkfelix · 2024-06-04T13:54:17Z

instruction-counts regressed webrender-2022-opt-full, cargo-opt-{incr-patched, full}
cycle-counts regressed webrender-2022-opt-{full, incr-full}, cranelift-codegen-opt-incr-full, and clap-opt-incr-patched
history view for webrender shows that the cycle-count effect seems real though not quite as pronounced as the original measurements indicate.
there are many potential candidates for the cause here in this rollup.
not marking as triaged; doing some followup perf runs on individual PR's

pnkfelix · 2024-06-04T13:55:22Z

@rust-timer build 110d314

pnkfelix · 2024-06-04T13:55:49Z

@rust-timer build 0b25619

rust-timer · 2024-06-04T15:10:28Z

Finished benchmarking commit (110d314): comparison URL.

Overall result: no relevant changes - no action needed

Instruction count

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

Max RSS (memory usage)

Results (primary 2.9%)

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

Cycles

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

Binary size

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

Bootstrap: 669.181s -> 667.291s (-0.28%)
Artifact size: 318.82 MiB -> 318.78 MiB (-0.01%)

lqd · 2024-06-04T15:24:53Z

We can only queue a build at a time :/

@rust-timer build 0b25619

rust-timer · 2024-06-04T17:39:48Z

Finished benchmarking commit (0b25619): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Instruction count

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.6%, 1.2%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.5%	[-0.4%, 1.2%]	4

Max RSS (memory usage)

Results (primary 3.1%, secondary 2.9%)

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)	5.0%	[1.1%, 15.6%]	5
Regressions ❌ (secondary)	2.9%	[2.9%, 2.9%]	1
Improvements ✅ (primary)	-6.7%	[-6.7%, -6.7%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.1%	[-6.7%, 15.6%]	6

Cycles

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

Binary size

Results (primary 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.1%	[0.0%, 0.4%]	24
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.2%, -0.0%]	11
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-0.2%, 0.4%]	35

Bootstrap: 669.181s -> 668.149s (-0.15%)
Artifact size: 318.82 MiB -> 318.86 MiB (0.01%)

amandasystems and others added 22 commits May 28, 2024 13:05

Fix back-porting drop-livess from Polonius to tracing

077a821

Make drop-use fact collection simpler for polonius

e155646

This shunts all the complexity of siphoning off the drop-use facts into `LivenessResults::add_extra_drop_facts()`, which may or may not be a good approach.

Move the rest of the logic into add_extra_drop_facts()

8066ebc

Apply x clippy --fix and x fmt

dabd05b

coverage: Add CLI support for -Zcoverage-options=condition

fa563c1

coverage: Add a test for -Zcoverage-options=condition

20174e6

Also InstSimplify &raw*

4b96e44

We do this for `&*` and `&mut*` already; might as well do it for raw pointers too.

Also resolve the type of constants, even if we already turned it into…

06c4cc4

… an error constant

Check that we can constrain the hidden tpye of a TAIT used in a const…

befcdec

… generic type

Add an alternate --demangle mode to coverage-dump

9abfebd

The coverage-dump tool already needs `rustc_demangle` for its own purposes, so the amount of extra code needed for a demangle mode is very small.

Use coverage-dump --demangle as the demangler for coverage-run tests

10ffc22

This avoids the need to build `rust-demangler` when running coverage tests, since we typically need to build `coverage-dump` anyway.

Use Builder::tool_exe to build the coverage-dump tool

feb8f3c

This appears to be the canonical way to build a tool with the stage 0 compiler.

Remove unused rust-demangler support from compiletest

54b6849

Stop using translate_args in the new solver

20699fe

Rollup merge of rust-lang#125796 - scottmcm:more-inst-simplify, r=oli…

234ed6a

…-obk Also InstSimplify `&raw*` We do this for `&*` and `&mut*` already; might as well do it for raw pointers too. r? mir-opt

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-review Status: Awaiting review from the assignee but also interested parties. labels May 31, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label May 31, 2024

bors merged commit ada5e2c into rust-lang:master May 31, 2024
7 checks passed

rustbot added this to the 1.80.0 milestone May 31, 2024

rustbot added the perf-regression Performance regression. label May 31, 2024

This comment has been minimized.

Sign in to view

matthiaskrgr deleted the rollup-eam7i0p branch September 1, 2024 17:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rollup of 7 pull requests #125824

Rollup of 7 pull requests #125824

matthiaskrgr commented May 31, 2024

matthiaskrgr commented May 31, 2024

bors commented May 31, 2024

bors commented May 31, 2024

bors commented May 31, 2024

rust-timer commented May 31, 2024

rust-timer commented May 31, 2024

pnkfelix commented Jun 4, 2024 •

edited

Loading

pnkfelix commented Jun 4, 2024

This comment has been minimized.

pnkfelix commented Jun 4, 2024

rust-timer commented Jun 4, 2024

lqd commented Jun 4, 2024

This comment has been minimized.

rust-timer commented Jun 4, 2024

Rollup of 7 pull requests #125824

Rollup of 7 pull requests #125824

Conversation

matthiaskrgr commented May 31, 2024

matthiaskrgr commented May 31, 2024

bors commented May 31, 2024

bors commented May 31, 2024

bors commented May 31, 2024

rust-timer commented May 31, 2024

rust-timer commented May 31, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

pnkfelix commented Jun 4, 2024 • edited Loading

pnkfelix commented Jun 4, 2024

This comment has been minimized.

pnkfelix commented Jun 4, 2024

rust-timer commented Jun 4, 2024

Overall result: no relevant changes - no action needed

lqd commented Jun 4, 2024

This comment has been minimized.

rust-timer commented Jun 4, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

pnkfelix commented Jun 4, 2024 •

edited

Loading