Skip to content

Make parallel frontend alloc-id numbering deterministic and re-enable the tests#157333

Closed
xmakro wants to merge 3 commits into
rust-lang:mainfrom
xmakro:reenable-parallel-alloc-id-tests
Closed

Make parallel frontend alloc-id numbering deterministic and re-enable the tests#157333
xmakro wants to merge 3 commits into
rust-lang:mainfrom
xmakro:reenable-parallel-alloc-id-tests

Conversation

@xmakro
Copy link
Copy Markdown

@xmakro xmakro commented Jun 2, 2026

Part of #154314. Addresses the "different alloc ids" row (#154278).

A group of const-eval ui tests were marked //@ ignore-parallel-frontend different alloc ids. AllocIds are handed out from a global counter, so under -Zthreads the numbers in the output differ run to run. compiletest already normalizes this: it remaps allocN to ALLOC{index} by order of first appearance (runtest.rs), so a pure renumbering does not affect the comparison.

For most of these tests the renumbering is the only difference. Each one emits a single const-eval error, or emits its errors in a deterministic order because const items are evaluated during the analysis phase rather than during parallel mono collection, so the order of first appearance is stable and the normalized output matches the single-threaded blessing. The first commit removes the directive from those tests and blesses the line numbers that shift from removing the directive line. No test output content changes, only span line numbers.

The remaining tests emit several const-eval errors whose emission order is itself nondeterministic under the parallel front-end. That reshuffles which allocation appears first, so the order-of-first-appearance normalization assigns different ALLOC indices and the normalized line text differs run to run.

To fix that at the source, the second commit makes compiletest sort the rendered diagnostics by their primary source location before it renumbers allocation ids. The first-appearance order then no longer depends on the racy emission order, so the ALLOC numbering is stable. Ties at the same location are broken by the diagnostic text with the raw allocation numbers blinded, since those numbers are racy too. The sort only runs under the parallel front-end, where stderr is already compared line by line, so single-threaded output is unchanged. With that in place the third commit re-enables the remaining ten tests.

Validation: every re-enabled test passes single threaded and under --parallel-frontend-threads=16 --iteration-count=100 --force-rerun (the multi-allocation ones at 200). Removing the directive shifts span line numbers by one and nothing else; the .32bit.stderr files, which cannot be blessed on a 64-bit host, were updated with the same shift and verified against the .64bit.stderr delta. A baseline run with the sort disabled confirms all ten fail without it and that no other test changes behavior under the parallel front-end.

@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 2, 2026
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 2, 2026
@xmakro xmakro changed the title Re-enable parallel frontend tests with deterministic alloc ids Make parallel frontend alloc-id numbering deterministic and re-enable the tests Jun 2, 2026
@xmakro xmakro marked this pull request as ready for review June 3, 2026 00:56
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 3, 2026

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 3, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 3, 2026

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @tiif (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue
Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 18 candidates

// Strip out raw byte dumps to make comparison platform-independent:
//@ normalize-stderr: "(the raw bytes of the constant) \(size: [0-9]*, align: [0-9]*\)" -> "$1 (size: $$SIZE, align: $$ALIGN)"
//@ normalize-stderr: "([0-9a-f][0-9a-f] |╾─*A(LLOC)?[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼ )+ *│.*" -> "HEX_DUMP"
//@ ignore-parallel-frontend different alloc ids
Copy link
Copy Markdown
Contributor

@zetanumbers zetanumbers Jun 3, 2026

Choose a reason for hiding this comment

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

Hi there. Could you replace these directives with an empty line instead to reduce gitdiff?

View changes since the review

@petrochenkov petrochenkov self-assigned this Jun 3, 2026
xmakro added 3 commits June 3, 2026 08:51
These const-eval tests were marked ignore-parallel-frontend for "different
alloc ids", but their only nondeterminism under -Zthreads is the alloc id
numbering, which compiletest already normalizes by order of first appearance.
Their diagnostic output is otherwise deterministic, so replace the directive
with a blank line rather than deleting it, keeping the following line numbers
and the expected output unchanged.

The remaining tests in that group emit multiple errors whose order is itself
nondeterministic, which also reshuffles the alloc id numbering; those are left
ignored.
Under `-Zthreads` the compiler emits diagnostics in a nondeterministic
order. compiletest renumbers allocation ids by their first appearance in
the rendered output, so that order leaked into the normalized `ALLOC<n>`
indices and made several const-eval UB tests flaky.

Sort the rendered diagnostics by their primary source location before
normalizing, so the numbering no longer depends on emission order. Ties
are broken by the diagnostic text with the raw allocation numbers blinded,
since those numbers are themselves racy. This only runs under the parallel
frontend, where stderr is already compared line by line, so single-threaded
output is unchanged.
These ten const-eval tests emit multiple errors carrying allocation ids.
They were ignored under the parallel frontend because the racy emission order
produced unstable ALLOC<n> numbering. Now that compiletest sorts diagnostics
by source location before renumbering, the numbering is stable, so replace the
directive with a blank line rather than deleting it. The expected stderr is
unchanged because the line numbers stay the same.
@xmakro xmakro force-pushed the reenable-parallel-alloc-id-tests branch from 4666307 to 7c09163 Compare June 3, 2026 15:54
@petrochenkov
Copy link
Copy Markdown
Contributor

Then alloc ID normalization needs to happen somewhere in fn normalize_output, not in fn extract_rendered.

This task was already taken by #156716, that PR just needs to move the normalization to compiletest, so I'll close this PR.

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants