Do not increase depth when evaluating nested goals of `NormalizesTo` by ShoyuVanilla · Pull Request #157718 · rust-lang/rust

ShoyuVanilla · 2026-06-10T16:56:53Z

~~Only the last commit is relevant since this is based on #156619~~

r? lcnr

lcnr · 2026-06-16T09:05:06Z

+            parent.required_depth =
+                parent.required_depth.max(match parent.increase_depth_for_nested {
+                    IncreaseDepthForNested::Yes => required_depth_for_nested + 1,
+                    IncreaseDepthForNested::No => required_depth_for_nested,
+                });


I've been thinking about about this as this feels brittle. Instead of required_depth, could we change the search graph to track the "min_reached_available_depth" and then required depth is computed as "available_depth - min_reached_available_depth"

that way the way depth is actually tracked doesn't matter anymore

View changes since the review

Sounds good to me. I'll try that way 👍

lcnr · 2026-06-18T08:18:32Z

                    &mut self.stack,
                    step_kind_from_parent,
-                    0,
+                    None,


can you put the min_reachable_for_nested in the UpdateParentGoalCtxt instead?

View changes since the review

lcnr

does wgpu compile with this change now?

View changes since this review

ShoyuVanilla · 2026-06-18T09:29:05Z

does wgpu compile with this change now?

View changes since this review

Hmm, this doesn't help it at all 🤔
wgpu 25.0.2 is compiled with -Zmin-recursion-limit=141 but not with any lower number regardless of this change.
Maybe the deepest recursion in it isn't a normalization or I'm doing things wrong.
I'll look into that crate more.

ShoyuVanilla · 2026-06-18T10:41:20Z

Yeah, we only have <= 1 NormalizesTo goal for each overflowing goal's search graph stack for that crate

lcnr · 2026-06-18T10:44:41Z

oh 🤔 interesting xd do we have AliasRelate goals there? these do still exist rn after all

ShoyuVanilla · 2026-06-18T10:55:57Z

Yes, they do still exist. But we have very few of them anyway (<= 1 for each stack, total 3)

lcnr · 2026-06-18T12:18:27Z

okay, so there are some other goals whose depth we don't properly increase in the old solver but now do in the new one 🤔

what a pain

ShoyuVanilla · 2026-06-18T14:17:43Z

Indeed. I'll take a look this weekend

ShoyuVanilla · 2026-06-19T07:10:47Z

https://rust-lang.zulipchat.com/#narrow/channel/364551-t-types.2Ftrait-system-refactor/topic/The.20old.20solver.20don't.20record.20required.20depth.20on.20cache/with/604759762

lcnr · 2026-06-29T10:56:08Z

+                UpdateParentGoalCtxt::Ordinary { nested_goals, min_reachable_available_depth } => {
+                    parent.min_reached_available_depth = AvailableDepth(
+                        parent.min_reached_available_depth.0.min(min_reachable_available_depth.0),
+                    );


can we just impl Ord for AvailableDepth to avoid accessing 0 here?

r=me on the change I think

It's unfortunate that this doesn't actually change things for wgpu, but still just seems vaguely desirable?

View changes since the review

rustbot · 2026-06-30T23:32:57Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

ShoyuVanilla · 2026-07-01T01:32:47Z

@bors r=lcnr

rust-bors · 2026-07-01T01:32:50Z

📌 Commit 464fb78 has been approved by lcnr

It is now in the queue for this repository.

…without-depth-increase, r=lcnr Do not increase depth when evaluating nested goals of `NormalizesTo` cc rust-lang#156619 (comment) ~~Only the last commit is relevant since this is based on rust-lang#156619~~ r? lcnr

Rollup of 6 pull requests Successful merges: - #157718 (Do not increase depth when evaluating nested goals of `NormalizesTo`) - #158449 (QNX target renaming) - #158483 (signed strict division: just use normal division) - #158516 ( Deduplicate codegen backends in bootstrap config) - #158542 (Rename `align` to `default_align` on `Scalar` and `Primitive`) - #158636 (linkchecker: upgrade to `html5ever v0.39`)

Rollup merge of #157718 - ShoyuVanilla:nested-normalizes-to-without-depth-increase, r=lcnr Do not increase depth when evaluating nested goals of `NormalizesTo` cc #156619 (comment) ~~Only the last commit is relevant since this is based on #156619~~ r? lcnr

Kobzol · 2026-07-01T10:58:20Z

Seems like this made compilation of typenum with the new solver 2x slower (#158639 (comment)).

ShoyuVanilla · 2026-07-02T01:47:18Z

Seems like this made compilation of typenum with the new solver 2x slower (#158639 (comment)).

AFAIK, this PR doesn't regress the perf by its own, like making our trait solving algorithm less efficient.

typenum is a very interesting crate which heavily depends on very complex, recursive associated types: https://docs.rs/typenum/latest/src/typenum/uint.rs.html#424-431 🙃
In its method resolution some candidates indefinitely go down into those nested chain of normalizations and they are discarded by purely hitting the recursion limit.

This PR's intended effect is to increase the available recursion depth when we are dealing with normalization, and in theory, it can increase it by at most twice, when the search graph is a sheer chain of nested normalizations.
So, we have to go into 2 times deeper into search graph to discard the problematic ambiguous candidate and that's the cause of the regression.
We can get the similar regression with the previous revisions before this PR by passing -Zmin-recursion-limit=256(twice of the default 128), comparing it to the result without the flag.

So, I think the regression is direct outcome from the intention of this PR, rather than some sort of side effect. But 50% regression feels very unfortunate 😭 @lcnr Could you share your thoughts on this?

lcnr · 2026-07-02T07:56:39Z

🤣 I love it

lcnr · 2026-07-02T07:58:11Z

see rust-lang/trait-system-refactor-initiative#73 for what exactly is wrecking performance here

lcnr

wait 😅 your doing something different than what I expected xd

I was thinking that we don't increase the depth of the NormalizesTo goal

as in

Projection available_depth: 6
NoramalizesTo available_depth: 6 (do not increase depth)
Trait available_depth: 5 (actually decrease)

not increasing the depth for the nested goals of normalization goals means that we don't slice off important parts of the search tree when encountering overflow, i.e

rust/compiler/rustc_type_ir/src/search_graph/mod.rs

Line 297 in c397dae

AvailableDepth(last.available_depth.0 / D::DIVIDE_AVAILABLE_DEPTH_ON_OVERFLOW)

That slicing is necessary to avoid the exponential blowup in rust-lang/trait-system-refactor-initiative#73. Doing the thing I thought we are doing shouldn't be able to have such a perf impact 🤣

View changes since this review

ShoyuVanilla · 2026-07-02T08:25:41Z

not increasing the depth for the nested goals of normalization goals means that we don't slice off important parts of the search tree when encountering overflow, i.e

rust/compiler/rustc_type_ir/src/search_graph/mod.rs

Line 297 in c397dae

AvailableDepth(last.available_depth.0 / D::DIVIDE_AVAILABLE_DEPTH_ON_OVERFLOW)

That slicing is necessary to avoid the exponential blowup in rust-lang/trait-system-refactor-initiative#73. Doing the thing I thought we are doing shouldn't be able to have such a perf impact 🤣

Oh, yeah.. looks like I've done things in a wrong way 🥲. I'll try it as you intended

lcnr · 2026-07-02T08:39:30Z

Oh, yeah.. looks like I've done things in a wrong way 🥲.

happens :3 i didn't catch it myself

ShoyuVanilla · 2026-07-02T08:45:13Z

That slicing is necessary to avoid the exponential blowup in rust-lang/trait-system-refactor-initiative#73. Doing the thing I thought we are doing shouldn't be able to have such a perf impact 🤣

But I guess the perf impact wouldn't be that different (though regardless of this, I think doing things as you intended make sense) because it only shifts where the available depth is decreased/overflowed by a single step from the parent normalizes-to goal -> nested goal?

I mean, if a nested goal of a normalizes-to is going to overflow in the correct implementation, the normalizes-to goal itself overflows in the current, wrong implementation and still divides the available depth, and if both of them is not going to overflow, then stepping into from that nested goal into its own nested goal will still decrease the available depth by 1?

lcnr · 2026-07-02T10:21:26Z

Unless I am mistaken here, this does matter imagine a NormalizesTo goal with 2 nested goals, the first one overflows and then divides the recursion depth for the second nested goal. With your PR, the first overflows and the second also gets the same recursion depth as the first 🤔

Rollup of 6 pull requests Successful merges: - rust-lang/rust#157718 (Do not increase depth when evaluating nested goals of `NormalizesTo`) - rust-lang/rust#158449 (QNX target renaming) - rust-lang/rust#158483 (signed strict division: just use normal division) - rust-lang/rust#158516 ( Deduplicate codegen backends in bootstrap config) - rust-lang/rust#158542 (Rename `align` to `default_align` on `Scalar` and `Primitive`) - rust-lang/rust#158636 (linkchecker: upgrade to `html5ever v0.39`)

ShoyuVanilla · 2026-07-02T14:07:13Z

Unless I am mistaken here, this does matter imagine a NormalizesTo goal with 2 nested goals, the first one overflows and then divides the recursion depth for the second nested goal. With your PR, the first overflows and the second also gets the same recursion depth as the first 🤔

🤔 Oh, that makes sense. I'll try the correct way

rustbot assigned lcnr Jun 10, 2026

ShoyuVanilla mentioned this pull request Jun 10, 2026

-Znext-solver Less normalizes-to janks #156619

Merged

This comment has been minimized.

Sign in to view

lcnr reviewed Jun 16, 2026

View reviewed changes

ShoyuVanilla force-pushed the nested-normalizes-to-without-depth-increase branch from e59d7fa to dc07fda Compare June 17, 2026 15:32

ShoyuVanilla marked this pull request as ready for review June 17, 2026 15:33

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 17, 2026

lcnr reviewed Jun 18, 2026

View reviewed changes

ShoyuVanilla force-pushed the nested-normalizes-to-without-depth-increase branch from dc07fda to ee25de1 Compare June 18, 2026 14:26

This comment has been minimized.

Sign in to view

ShoyuVanilla force-pushed the nested-normalizes-to-without-depth-increase branch from ee25de1 to e23e501 Compare June 18, 2026 15:03

This comment has been minimized.

Sign in to view

lcnr reviewed Jun 29, 2026

View reviewed changes

Do not increase depth when evaluating nested goals of normalizes-to

464fb78

ShoyuVanilla force-pushed the nested-normalizes-to-without-depth-increase branch from e23e501 to 464fb78 Compare June 30, 2026 23:32

rust-bors Bot 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 1, 2026

This was referenced Jul 1, 2026

Rollup of 5 pull requests #158638

Closed

Rollup of 6 pull requests #158639

Merged

rust-bors Bot merged commit 90faa9c into rust-lang:main Jul 1, 2026
13 checks passed

rustbot added this to the 1.98.0 milestone Jul 1, 2026

lcnr reviewed Jul 2, 2026

View reviewed changes

This comment was marked as off-topic.

Sign in to view

Uh oh!

Uh oh!

Conversation

ShoyuVanilla commented Jun 10, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

lcnr Jun 16, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShoyuVanilla Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

lcnr Jun 18, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShoyuVanilla commented Jun 18, 2026

Uh oh!

ShoyuVanilla commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lcnr commented Jun 18, 2026

Uh oh!

ShoyuVanilla commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lcnr commented Jun 18, 2026

Uh oh!

ShoyuVanilla commented Jun 18, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ShoyuVanilla commented Jun 19, 2026

Uh oh!

lcnr Jun 29, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jun 30, 2026

Uh oh!

ShoyuVanilla commented Jul 1, 2026

Uh oh!

rust-bors Bot commented Jul 1, 2026

Uh oh!

Uh oh!

Kobzol commented Jul 1, 2026

Uh oh!

ShoyuVanilla commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lcnr commented Jul 2, 2026

Uh oh!

lcnr commented Jul 2, 2026

Uh oh!

lcnr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

ShoyuVanilla commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lcnr commented Jul 2, 2026

Uh oh!

ShoyuVanilla commented Jul 2, 2026

Uh oh!

lcnr commented Jul 2, 2026

Uh oh!

ShoyuVanilla commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

ShoyuVanilla commented Jun 10, 2026 •

edited by rustbot

Loading

lcnr Jun 16, 2026 •

edited by rustbot

Loading

lcnr Jun 18, 2026 •

edited by rustbot

Loading

lcnr left a comment •

edited by rustbot

Loading

ShoyuVanilla commented Jun 18, 2026 •

edited

Loading

ShoyuVanilla commented Jun 18, 2026 •

edited

Loading

lcnr Jun 29, 2026 •

edited by rustbot

Loading

ShoyuVanilla commented Jul 2, 2026 •

edited

Loading

lcnr left a comment •

edited

Loading

ShoyuVanilla commented Jul 2, 2026 •

edited

Loading