Skip to content

Evaluate free constants and statics in definition order under the parallel front-end#157353

Open
xmakro wants to merge 1 commit into
rust-lang:mainfrom
xmakro:parallel-frontend-const-eval-order
Open

Evaluate free constants and statics in definition order under the parallel front-end#157353
xmakro wants to merge 1 commit into
rust-lang:mainfrom
xmakro:parallel-frontend-const-eval-order

Conversation

@xmakro
Copy link
Copy Markdown
Contributor

@xmakro xmakro commented Jun 3, 2026

Under the parallel front-end, tests/ui/consts/chained-constants-stackoverflow.rs intermittently fails with "queries overflow the depth limit" even though it is a valid program that compiles fine single-threaded. This is the depth-limit nondeterminism tracked in issue 146616.

check_crate forces the value of every free static and non-generic free const inside par_hir_body_owners, so worker threads pick up items in an arbitrary order. A const whose initializer refers to another const evaluates it as a nested, depth-limited query, so when a thread starts on a const before its dependencies are cached the query stack grows as deep as the still-uncached chain. With a long enough chain (350 here) that depth depends on scheduling and can exceed the recursion limit, so the same crate compiles single-threaded but intermittently fails in parallel.

When more than one thread is in use, this forces the values sequentially in definition order before the parallel loop, matching the single-threaded order so each const's dependencies are already cached when it is evaluated. It only changes the order in which the existing set of free statics and free consts is forced: which items get evaluated, the recursion limit, and single-threaded output are all unchanged. The genuine overflow tests each have a single evaluation entry point whose depth does not depend on scheduling, so they still overflow with identical output. maybe_check_static_with_link_section stays in the parallel loop so it still runs exactly once. The previously ignored test is re-enabled.

Verified with the rebuilt compiler: 0 overflows across 240 runs of chained-constants-stackoverflow.rs at -Zthreads=8/16/32, the four genuine overflow tests (consts/recursive-block, query-system/query_depth, consts/recursive-const-in-impl, generic-const-items/recursive) still overflow deterministically, and the affected ui tests pass single-threaded.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 3, 2026
@xmakro xmakro force-pushed the parallel-frontend-const-eval-order branch from 001ad70 to 049476e Compare June 3, 2026 04:18
@rust-log-analyzer

This comment has been minimized.

@petrochenkov petrochenkov self-assigned this Jun 3, 2026
@petrochenkov petrochenkov 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 3, 2026
@xmakro
Copy link
Copy Markdown
Contributor Author

xmakro commented Jun 3, 2026

@petrochenkov please only look at PRs that are ready for review. Any draft PR is not in a reviewable state

@petrochenkov
Copy link
Copy Markdown
Contributor

r? @oli-obk @RalfJung could you check if this generally makes sense?

The associated constant change in particular may be language-observable?
Otherwise it seems plausible at surface level.

(The change is most likely LLM-generated so feel free to close if you don't want to look.)

@rustbot rustbot assigned oli-obk and unassigned petrochenkov Jun 3, 2026
// depends on are cached, building a query stack whose depth depends on scheduling and
// can spuriously exceed the recursion limit.
if matches!(tcx.sess.threads(), Some(threads) if threads > 1) {
for item_def_id in tcx.hir_body_owners() {
Copy link
Copy Markdown
Contributor

@petrochenkov petrochenkov Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the high level idea is to disable parallelization for a subset of the work below, because that subset causes more complex changes in test outputs than simple diagnostic reordering which can be easily normalized away.

View changes since the review

…allel front-end

Under the parallel front-end, free constant and static values are forced
inside par_hir_body_owners, so worker threads pick up items in an arbitrary
order. A const whose initializer refers to another const evaluates it as a
nested, depth-limited query, so a thread that starts on a const before its
dependencies are cached builds a query stack as deep as the uncached chain.
A long enough chain can then spuriously exceed the recursion limit depending
on scheduling (issue 146616).

When more than one thread is in use, force these values sequentially in
definition order before the parallel loop, matching the single-threaded order
so each const's dependencies are already cached when it is evaluated. This
only changes the order in which the existing set of free statics and free
consts is forced; it does not change which items are evaluated, the recursion
limit, or single-threaded output. maybe_check_static_with_link_section stays
in the parallel loop so it still runs exactly once.

Re-enable chained-constants-stackoverflow.rs under the parallel front-end.
@xmakro xmakro force-pushed the parallel-frontend-const-eval-order branch from 049476e to 3e9f80f Compare June 3, 2026 17:41
@xmakro
Copy link
Copy Markdown
Contributor Author

xmakro commented Jun 3, 2026

I reverted the associated const change, so there shouldn't be any language observable change anymore.

Now it is only the order change. Under the parallel front-end, free statics and free consts are forced in definition order before the parallel loop.

@xmakro xmakro marked this pull request as ready for review June 3, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

5 participants