Add `SliceLike` to `rustc_type_ir`, use it in the generic solver code (+ some other changes) #126813

compiler-errors · 2024-06-21T22:51:49Z

First, we split out TraitRef::new_from_args which takes just ty::GenericArgsRef from TraitRef::new which takes impl IntoIterator<Item: Into<GenericArg>>. I will explain in a minute why.

Second, we introduce SliceLike, which allows us to be generic over List<T> and [T]. This trait has an as_slice() and into_iter() method, and some other convenience functions. However, importantly, since types like I::GenericArgs now implement SliceLike rather than IntoIter<Item = I::GenericArg>, we can't use TraitRef::new on this directly. That's where new_from_args comes in.

Finally, we adjust all the code to use these slice operators. Some things get simpler, some things get a bit more annoying since we need to use as_slice() in a few places. 🤷

r? lcnr

rustbot · 2024-06-21T22:51:59Z

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred in compiler/rustc_sanitizers

cc @rust-lang/project-exploit-mitigations, @rcvalle

HIR ty lowering was modified

cc @fmease

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler-errors · 2024-06-21T22:52:46Z

@bors try @rust-timer queue

Add `SliceLike` to `rustc_type_ir`, use it in the generic solver code (+ some other changes) First, we split out `TraitRef::new_from_args` which takes *just* `ty::GenericArgsRef` from `TraitRef::new` which takes `impl IntoIterator<Item: Into<GenericArg>>`. I will explain in a minute why. Second, we introduce `SliceLike`, which allows us to be generic over `List<T>` and `[T]`. This trait has an `as_slice()` and `into_iter()` method, and some other convenience functions. However, importantly, since types like `I::GenericArgs` now implement `SliceLike` rather than `IntoIter<Item = I::GenericArg>`, we can't use `TraitRef::new` on this directly. That's where `new_from_args` comes in. Finally, we adjust all the code to use these slice operators. Some things get simpler, some things get a bit more annoying since we need to use `as_slice()` in a few places. 🤷 r? lcnr

bors · 2024-06-21T22:53:58Z

⌛ Trying commit 0770597 with merge d906ddd...

bors · 2024-06-22T00:34:13Z

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

rust-timer · 2024-06-22T01:49:11Z

Finished benchmarking commit (d906ddd): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

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
Regressions ❌ (secondary)	0.5%	[0.4%, 0.9%]	7
Improvements ✅ (primary)	-0.3%	[-0.6%, -0.2%]	18
Improvements ✅ (secondary)	-0.5%	[-0.5%, -0.4%]	7
All ❌✅ (primary)	-0.3%	[-0.6%, -0.2%]	18

Max RSS (memory usage)

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

Cycles

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

Binary size

Results (secondary 0.3%)

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

Bootstrap: 697.477s -> 694.569s (-0.42%)
Artifact size: 326.87 MiB -> 326.70 MiB (-0.05%)

compiler/rustc_type_ir/src/inherent.rs

compiler-errors · 2024-06-23T15:01:29Z

Looks like there's a slight perf improvement -- either noise, or because of the addition of new_from_args constructors which mean we don't try to re-intern GenericArgsRef once it's interned.

lcnr

iter::zip(a.into_iter(), b.into_iter()) seems quite confusing. What are your thoughts about adding zip to slice_like to handle this. Alternatively zip_slice_like as a free function

lcnr · 2024-06-23T15:38:38Z

compiler/rustc_type_ir/src/ty_kind.rs

    #[derivative(Debug = "ignore")]
    pub(crate) _use_alias_ty_new_instead: (),
 }

 impl<I: Interner> AliasTy<I> {
+    pub fn new_from_args(interner: I, def_id: I::DefId, args: I::GenericArgs) -> AliasTy<I> {


can now just delegate to new_from_args?

Yep, that's simpler

compiler-errors · 2024-06-24T14:52:46Z

iter::zip(a.into_iter(), b.into_iter()) seems quite confusing. What are your thoughts about adding zip to slice_like to handle this. Alternatively zip_slice_like as a free function.

It seems excessive to basically pull down one iterator combination. I don't actually understand what you find confusing about that, either. I guess I could make it slightly less noisy by renaming SliceLike::into_iter into SliceLike::iter so it's consistent with List::iter which returns impl Iterator<Item = T>?

zip_slice_like also seems weird because we still need to call std::iter::zip in case we're zipping a SliceLike and non-slice like thing (i.e. a range) -- we have a couple of those calls. I'd rather keep it "as close" to stdlib as possible so users don't need to choose whether to use the real zip or one that just copies it with name punning.

lcnr · 2024-06-24T14:57:46Z

yeah, I'd prefer fn iter over fn into_iter so that it's clear we're not using the method from IntoIterator.

…sTerm/TraitRef/projection

lcnr · 2024-06-24T17:52:41Z

@bors r+ rollup=never

bors · 2024-06-24T17:52:44Z

📌 Commit d521e21 has been approved by lcnr

It is now in the queue for this repository.

bors · 2024-06-25T00:33:52Z

⌛ Testing commit d521e21 with merge 5b270e1...

bors · 2024-06-25T02:43:46Z

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 5b270e1 to master...

rust-timer · 2024-06-25T03:59:39Z

Finished benchmarking commit (5b270e1): 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
Regressions ❌ (secondary)	0.5%	[0.4%, 0.8%]	7
Improvements ✅ (primary)	-0.4%	[-0.6%, -0.3%]	12
Improvements ✅ (secondary)	-0.7%	[-2.2%, -0.2%]	9
All ❌✅ (primary)	-0.4%	[-0.6%, -0.3%]	12

Max RSS (memory usage)

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

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

Cycles

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

Binary size

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

Bootstrap: 692.774s -> 693.576s (0.12%)
Artifact size: 326.72 MiB -> 326.73 MiB (0.00%)

pnkfelix · 2024-07-02T19:14:49Z

regressions are all to secondary benchmark: match-stress
marking as triaged

@rustbot label: +perf-regression-triaged

rustbot assigned lcnr Jun 21, 2024

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 21, 2024

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 22, 2024

Nadrieril reviewed Jun 22, 2024

View reviewed changes

compiler/rustc_type_ir/src/inherent.rs Outdated Show resolved Hide resolved

lcnr reviewed Jun 23, 2024

View reviewed changes

compiler-errors added 3 commits June 24, 2024 11:28

Split out IntoIterator and non-Iterator constructors for AliasTy/Alia…

f26cc34

…sTerm/TraitRef/projection

Replace Deref bounds on Interner in favor of a SliceLike trait

24e41f1

Also migrate FnInputTys

d521e21

compiler-errors force-pushed the SliceLike branch from 0770597 to d521e21 Compare June 24, 2024 15:53

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 24, 2024

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 24, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 25, 2024

bors merged commit 5b270e1 into rust-lang:master Jun 25, 2024
7 checks passed

rustbot added this to the 1.81.0 milestone Jun 25, 2024

bors mentioned this pull request Jun 25, 2024

Rename tcx to cx in new solver generic code #126812

Merged

rustbot added the perf-regression-triaged The performance regression has been triaged. label Jul 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `SliceLike` to `rustc_type_ir`, use it in the generic solver code (+ some other changes) #126813

Add `SliceLike` to `rustc_type_ir`, use it in the generic solver code (+ some other changes) #126813

compiler-errors commented Jun 21, 2024

rustbot commented Jun 21, 2024

compiler-errors commented Jun 21, 2024

This comment has been minimized.

bors commented Jun 21, 2024

This comment has been minimized.

bors commented Jun 22, 2024

This comment has been minimized.

rust-timer commented Jun 22, 2024

compiler-errors commented Jun 23, 2024

lcnr left a comment

lcnr Jun 23, 2024 •

edited

Loading

compiler-errors Jun 24, 2024

compiler-errors commented Jun 24, 2024

lcnr commented Jun 24, 2024

lcnr commented Jun 24, 2024

bors commented Jun 24, 2024

bors commented Jun 25, 2024

bors commented Jun 25, 2024

rust-timer commented Jun 25, 2024

pnkfelix commented Jul 2, 2024

Add SliceLike to rustc_type_ir, use it in the generic solver code (+ some other changes) #126813

Add SliceLike to rustc_type_ir, use it in the generic solver code (+ some other changes) #126813

Conversation

compiler-errors commented Jun 21, 2024

rustbot commented Jun 21, 2024

compiler-errors commented Jun 21, 2024

This comment has been minimized.

bors commented Jun 21, 2024

This comment has been minimized.

bors commented Jun 22, 2024

This comment has been minimized.

rust-timer commented Jun 22, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

compiler-errors commented Jun 23, 2024

lcnr left a comment

Choose a reason for hiding this comment

lcnr Jun 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors commented Jun 24, 2024

lcnr commented Jun 24, 2024

lcnr commented Jun 24, 2024

bors commented Jun 24, 2024

bors commented Jun 25, 2024

bors commented Jun 25, 2024

rust-timer commented Jun 25, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

pnkfelix commented Jul 2, 2024

Add `SliceLike` to `rustc_type_ir`, use it in the generic solver code (+ some other changes) #126813

Add `SliceLike` to `rustc_type_ir`, use it in the generic solver code (+ some other changes) #126813

lcnr Jun 23, 2024 •

edited

Loading