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 #113370

Merged
merged 30 commits into from
Jul 5, 2023
Merged

Conversation

compiler-errors
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

lcnr and others added 30 commits July 4, 2023 10:06
This effectively inlines most of `FunctionCx::codegen_coverage` into the LLVM
implementation of `CoverageInfoBuilderMethods`.
…d it

These methods are only ever called from within `rustc_codegen_llvm`, so they
can just be declared there as well.
rust-installer & rls: remove exclusion from rustfmt & tidy

<strike>based on rust-lang#112884</strike>

`rust-installer` and `rls` no longer submodules, but not removed from exclude list for rustfmt and tidy, preventing running fmt and lints on them.
 -Ztrait-solver=next: stop depending on old solver

removes the final dependencies on the old solver when `-Ztrait-solver=next` is enabled.
…iler-errors

`TypeParameterDefinition` always require a `DefId`

the `None` case never actually reaches diagnostics so it feels better for diagnostics to be able to rely on the `DefId` being there, cc rust-lang#113310
…etrochenkov,BoxyUwU

Add some extra information to opaque type cycle errors

Plus a bunch of cleanups.

This should help users debug query cycles due to auto trait checking. We'll probably want to fix cycle errors in most (or all?) cases by looking at the current item's hidden types (new solver does this), and by delaying the auto trait checks to after typeck.
…r=oli-obk

Move `ty::ConstKind` to `rustc_type_ir`

Needed this in another PR for custom debug impls, and this will also be required to move the new solver into a separate crate that does not use `TyCtxt` so that r-a and friends can depend on the trait solver.

Rebased on top of rust-lang#113325, only the second and third commits needs reviewing
…-specializing, r=lcnr

Winnow specialized impls during selection in new solver

We need to be able to winnow impls that are specialized by more specific impls in order for codegen to be able to proceed.

r? ``@lcnr``
Move most coverage code out of `rustc_codegen_ssa`

*This is one step in my larger coverage refactoring ambitions described at <https://github.com/rust-lang/compiler-team/issues/645>.*

The backend implementation of coverage instrumentation was originally split between SSA and LLVM, perhaps in the hopes that it could be used by other backends.

In practice, this split mostly just makes the coverage implementation harder to navigate and harder to modify. It seems unlikely that any backend will actually implement coverage instrumentation in the foreseeable future, especially since many parts of the existing implementation (outside the LLVM backend) are heavily tied to the specific details of LLVM's coverage instrumentation features.

The current shared implementation of `codegen_coverage` is heavily tied to the details of `StatementKind::Coverage`, which makes those details difficult to change. I have reason to want to change those details as part of future fixes/improvements, so this will reduce the amount of interface churn caused by those later changes.

---

This is intended to be a pure refactoring change, with no changes to actual behaviour. All of the “added” code has really just been moved from other files.
Add support for NetBSD/riscv64 aka. riscv64gc-unknown-netbsd.
@compiler-errors
Copy link
Member Author

@bors r+ rollup=never p=8

@bors
Copy link
Contributor

bors commented Jul 5, 2023

📌 Commit 560136f has been approved by compiler-errors

It is now in the queue for this repository.

@bors 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 Jul 5, 2023
@bors
Copy link
Contributor

bors commented Jul 5, 2023

⌛ Testing commit 560136f with merge 5dac6b3...

@bors
Copy link
Contributor

bors commented Jul 5, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 5dac6b3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 5, 2023
@bors bors merged commit 5dac6b3 into rust-lang:master Jul 5, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jul 5, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#113010 rust-installer & rls: remove exclusion from rustfmt & tidy 7415fdf538c0a7b0201800c610e3a9078ff07455 (link)
#113317 -Ztrait-solver=next: stop depending on old solver 3cfc22c1d7b73b6e6f271d624b47ed04b02eb208 (link)
#113319 TypeParameterDefinition always require a DefId d842389b6d6a2f6e20759e9ac53799f98cd2ad52 (link)
#113320 Add some extra information to opaque type cycle errors a469b5fc80c0d4250147c756dc5c44516090467f (link)
#113321 Move ty::ConstKind to rustc_type_ir 3d733ddbaf4539600bc959bb68240204c37d9a6a (link)
#113337 Winnow specialized impls during selection in new solver a318de9f87de1dc2e38c0586adfc0e0042db7012 (link)
#113355 Move most coverage code out of rustc_codegen_ssa 2c85e87413716a42138c0ee06958d6713062b8b7 (link)
#113356 Add support for NetBSD/riscv64 aka. riscv64gc-unknown-netbs… 20b421757878f1d047bfb0ad76adc7db38daa69f (link)

previous master: e4cd161006

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 (5dac6b3): comparison URL.

Overall result: ❌ regressions - 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)
1.3% [0.6%, 2.1%] 11
Regressions ❌
(secondary)
1.6% [1.6%, 1.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.3% [0.6%, 2.1%] 11

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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.9% [2.5%, 5.3%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.3% [-2.7%, -2.1%] 4
All ❌✅ (primary) - - 0

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.

mean range count
Regressions ❌
(primary)
2.0% [1.7%, 2.4%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.0% [1.7%, 2.4%] 3

Binary size

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

Bootstrap: 655.622s -> 658.838s (0.49%)

@rustbot rustbot added the perf-regression Performance regression. label Jul 5, 2023
@compiler-errors
Copy link
Member Author

Strange. Hopefully noise? But I started a rust-timer on #113320 (comment) and #113317 (comment).

Other PRs don't look like they affected anything? But maybe I overlooked something.

@lqd
Copy link
Member

lqd commented Jul 5, 2023

Noise spikes on bitmaps are usually smaller IIRC, at least recently they were. These 2 runs, or the next couple PRs to be merged, should hopefully be enough to see if that's just noise.

(In case you don't know: you don't have to launch the perf runs on the rolled up PR's individual page, you can also launch them here in this rollup, e.g. so that all your investigation is in the same place, and it's a bit more convenient than going to multiple pages)

@compiler-errors
Copy link
Member Author

Huh, well looks like #113320 (comment) may be the culprit? cc @oli-obk

Or else someone should look at those results and conclude they're noise.

@rylev
Copy link
Member

rylev commented Jul 11, 2023

Looks like @oli-obk investigated more here.

@oli-obk any further thoughts on this or shall we officially give up on what might be the cause?

@lqd
Copy link
Member

lqd commented Jul 11, 2023

Looks like @oli-obk investigated more here.

As well as here where the revert results are also confusing.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 11, 2023

I should try reverting the entire PR, will do that tomorrow.

@lqd
Copy link
Member

lqd commented Jul 11, 2023

I got you, hopefully you'll have #113587's results ready for breakfast.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 12, 2023

Thanks ❤️

Looks like there is indeed no regression from that PR and it was all noise?! Unless something changed to solidify the regression and now we can't undo it (which would speak for inliner noise)

@lqd
Copy link
Member

lqd commented Jul 12, 2023

Very confusing indeed, the bitmaps regression detected in this rollup has not reverted since then.

image

Maybe some noise when testing individual PRs has prevented us from seeing the correct PR introducing it, or it's a combination of multiple PRs in this rollup that is causing the issue.

@compiler-errors compiler-errors deleted the rollup-8gvyy8e branch August 11, 2023 20:16
@lqd lqd mentioned this pull request May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.