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

Add ConstKind::Expr #99798

Merged
merged 16 commits into from
Nov 26, 2022
Merged

Add ConstKind::Expr #99798

merged 16 commits into from
Nov 26, 2022

Conversation

JulianKnodt
Copy link
Contributor

Starting to implement ty::ConstKind::Abstract, most of the match cases are stubbed out, some I was unsure what to add, others I didn't want to add until a more complete implementation was ready.

r? @lcnr

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 27, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 27, 2022
@rust-log-analyzer

This comment has been minimized.

compiler/rustc_codegen_cranelift/src/constant.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_cranelift/src/constant.rs Outdated Show resolved Hide resolved
compiler/rustc_infer/src/infer/freshen.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/mir/syntax.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/consts/kind.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/fast_reject.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/structural_impls.rs Outdated Show resolved Hide resolved
compiler/rustc_trait_selection/src/traits/wf.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@JulianKnodt JulianKnodt force-pushed the ac1 branch 2 times, most recently from 5a83027 to bc5d4fe Compare July 29, 2022 07:33
@JulianKnodt JulianKnodt marked this pull request as ready for review July 29, 2022 07:33
@rustbot
Copy link
Collaborator

rustbot commented Jul 29, 2022

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@JulianKnodt
Copy link
Contributor Author

This will probably require a perf run, considering the size of consts increased, and if there's any alternative way to store consts more efficiently that would also be useful.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 29, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 29, 2022
@bors
Copy link
Contributor

bors commented Jul 29, 2022

⌛ Trying commit 55f46fc49a77c65c10117d9b412a0061dc2355e5 with merge e5e10c53062b74626dfbbe444c6526d38982715b...

@bors
Copy link
Contributor

bors commented Jul 29, 2022

☀️ Try build successful - checks-actions
Build commit: e5e10c53062b74626dfbbe444c6526d38982715b (e5e10c53062b74626dfbbe444c6526d38982715b)

@rust-timer
Copy link
Collaborator

Queued e5e10c53062b74626dfbbe444c6526d38982715b with parent 2f847b8, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e5e10c53062b74626dfbbe444c6526d38982715b): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
2.5% 3.3% 6
Improvements 🎉
(primary)
-0.4% -0.9% 18
Improvements 🎉
(secondary)
-0.6% -0.8% 14
All 😿🎉 (primary) -0.4% -0.9% 18

Max RSS (memory usage)

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)
1.6% 1.6% 1
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
-2.9% -2.9% 1
Improvements 🎉
(secondary)
-2.1% -2.4% 2
All 😿🎉 (primary) -0.6% -2.9% 2

Cycles

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)
1.6% 1.8% 2
Regressions 😿
(secondary)
2.9% 3.9% 15
Improvements 🎉
(primary)
-2.6% -3.1% 2
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) -0.5% -3.1% 4

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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-review -S-waiting-on-perf -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2 3

  2. number of relevant changes 2 3

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 29, 2022
@JulianKnodt JulianKnodt force-pushed the ac1 branch 2 times, most recently from a210ed4 to 6cd8f41 Compare July 30, 2022 09:04
@JulianKnodt
Copy link
Contributor Author

Since the size of the const is the same, I wonder if the perf changes are caused by an increased number of branches, causing it to switch from a jump table to an explicit search?

@rust-log-analyzer

This comment has been minimized.

@BoxyUwU
Copy link
Member

BoxyUwU commented Nov 25, 2022

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Nov 25, 2022

📌 Commit d0209db has been approved by BoxyUwU

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 25, 2022
@bors
Copy link
Contributor

bors commented Nov 25, 2022

⌛ Testing commit d0209db with merge aff003b...

@bors
Copy link
Contributor

bors commented Nov 26, 2022

☀️ Test successful - checks-actions
Approved by: BoxyUwU
Pushing aff003b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 26, 2022
@bors bors merged commit aff003b into rust-lang:master Nov 26, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 26, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (aff003b): 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)
0.3% [0.2%, 0.5%] 6
Regressions ❌
(secondary)
0.9% [0.3%, 1.6%] 21
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.2%] 1
All ❌✅ (primary) 0.3% [0.2%, 0.5%] 6

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)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.4% [-5.5%, -2.2%] 3
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)
- - 0
Regressions ❌
(secondary)
7.7% [6.4%, 9.1%] 6
Improvements ✅
(primary)
-1.2% [-1.8%, -0.8%] 21
Improvements ✅
(secondary)
-3.1% [-4.6%, -1.3%] 14
All ❌✅ (primary) -1.2% [-1.8%, -0.8%] 21

@rustbot rustbot added the perf-regression Performance regression. label Nov 26, 2022
@nnethercote
Copy link
Contributor

Previous perf runs showed slight improvements, but the post-merge one shows regressions. They are mostly among secondary benchmarks.

@Mark-Simulacrum Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Nov 29, 2022
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…i-obk

stop using `ty::UnevaluatedConst` directly

best reviewed commit by commit.

simplifies rust-lang#99798 because we now don't have to expand `ty::UnevaluatedConst` to `ty::Const`.
I also remember some other places where using `ty::UnevaluatedConst` directly was annoying and caused issues, though I don't quite remember what they were rn '^^

r? `@oli-obk` cc `@JulianKnodt`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.