Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 8 pull requests #133865

Merged
merged 23 commits into from
Dec 4, 2024
Merged

Rollup of 8 pull requests #133865

merged 23 commits into from
Dec 4, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Walnut356 and others added 23 commits December 2, 2024 01:07
A "site" is a node or edge in the coverage graph.
This is more convenient for subsequent patches.
These simplifications are now handled by the transcribe step.
It's very old (added in rust-lang#12087). It's strange, and it's not clear what
its use cases are. It only works with the crate root file because it
runs before expansion. I suspect it won't be missed.
…-ozkan

Include LLDB and GDB visualizers in MSVC distribution

MSVC distributions currently don't include the lldb or GDB python files. MSVC and LLDB/GDB are not mutually exclusive (and end up being a common case with vscode + codelldb/lldb-dap), so they should probably be included.

the existing visualizers currently only partially work on MSVC due to the differences in how the debug info is generated, but they also only partially work on GNU anyway - both of which are actively being fixed.
…ce-pointee-errors, r=jieyouxu

Make CoercePointee errors translatable

Tracked by rust-lang#123430

Just in case that a translatable error message would become a blocker to stabilization, this PR switches over to fluent error messages, which also slightly improve the wordings and use more accurate span information.

cc `@Darksonn` `@traviscross`
…mpiler-errors

Don't try and handle unfed `type_of` on anon consts

The `type_of` query for anon consts in the type system is actually implemented by feeding the return value during hir ty lowering, not the hir-based logic in `const_arg_anon_type_of`. The HIR based logic is incomplete (doesn't handle all hir nodes) and also generally wrong to call (re-lowers HIR or invokes typeck which can result in query cycles).

r? `@compiler-errors`
…iler-errors

Remove `-Zshow-span`.

It's very old (added in rust-lang#12087). It's strange, and it's not clear what its use cases are. It only works with the crate root file because it runs before expansion. I suspect it won't be missed.

r? `@estebank`
coverage: Use a separate counter type and simplification step during counter creation

When instrumenting a function's MIR for coverage, there is a point where we need to decide, for each node in the control-flow graph, whether its execution count will be tracked by a physical counter, or by an expression that combines physical counters from other parts of the graph.

Currently the code for doing that is heavily tied to the final form of the LLVM coverage mapping format, and performs some important simplification steps on-the-fly. These factors make the code extremely difficult to modify without breaking or massively worsening the resulting coverage-instrumentation metadata.

---

This PR aims to improve that situation somewhat by adding an extra intermediate representation between the code that chooses how each node will be counted, and the code that converts those decisions into actual tables of physical counters and trees of counter expressions.

As part of doing that, some of the simplifications that are currently performed during the main counter creation step have been pulled out into a separate step.

In most cases the resulting coverage metadata is equivalent, slightly better, or slightly worse. The biggest outlier is `counters.rs`, where the coverage metadata ends up about 10% larger. This seems to be the result of the new approach having less subexpression sharing (because it relies on flatten-sort-cancel), and therefore being less effective at taking advantage of MIR optimizations to replace counters for unused control-flow with zeroes. I think the modest downside is acceptable in light of the future possibilities opened up by this decoupling.
…ler-errors

Avoid `opaque type not constrained` errors in the presence of other errors

pulled out of rust-lang#128440

These errors carry no new information if the opaque type was actually used in a constraining (but erroneous) way somewhere.
Stop git from merging generated files

See the relevant documentation for details: https://git-scm.com/docs/gitattributes#Documentation/gitattributes.txt-Unset-1-1-1

This will cause `.stderr`, `.stdout`, `.mir`, and `.fixed` files to stop generating conflict markers and instead just pick the version from your branch, not the one you're rebasing over. Git will still raise a conflict and thus stop a rebase at that commit, but it avoids having to deal with conflict markers in files where we just bless them away anyway.
…lubby789

Update sysinfo version to 0.33.0
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Dec 4, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=8

@bors
Copy link
Contributor

bors commented Dec 4, 2024

📌 Commit 670affb has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 4, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 4, 2024
@bors
Copy link
Contributor

bors commented Dec 4, 2024

⌛ Testing commit 670affb with merge acabb52...

@bors
Copy link
Contributor

bors commented Dec 4, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 4, 2024
@bors bors merged commit acabb52 into rust-lang:master Dec 4, 2024
7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 4, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#133737 Include LLDB and GDB visualizers in MSVC distribution f4397e8ec218eb636baf5c26c23757753c158aba (link)
#133774 Make CoercePointee errors translatable 12853ff2a71b5237c26af2e6c25e968cc9669b86 (link)
#133831 Don't try and handle unfed type_of on anon consts b65a9d500b9e9c2354c78f50d6dd6f6c5bb22a7e (link)
#133847 Remove -Zshow-span. cd621cfbeddf11766a8dc843a7290f7fa83a1e68 (link)
#133849 coverage: Use a separate counter type and simplification st… 3252548ff5538d48d400dfcb3688b24288404088 (link)
#133850 Avoid opaque type not constrained errors in the presence … 27e43d76ebb4840b2d94f777b692dbf28abe06eb (link)
#133851 Stop git from merging generated files 88d3d86728d1cbc89ff329d58adb1706ab2c8aab (link)
#133856 Update sysinfo version to 0.33.0 f18ef8379514d0df58a4c95e5a47871ac629e201 (link)

previous master: 96e51d9482

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
Copy link
Collaborator

Finished benchmarking commit (acabb52): 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)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.3%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

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

Cycles

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

Binary size

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

Bootstrap: 767.015s -> 769.678s (0.35%)
Artifact size: 330.85 MiB -> 330.86 MiB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.