🏆 Live Soccer TV: rust lang/rust/pull/

Kobzol · 2026-07-30T07:54:21Z

Context: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Blessing.20mir-opt.20on.20a.20clean.20checkout.20produces.20a.20diff/near/613465816

It looks like we didn't run them on CI (ever? lol).

r? @RalfJung

rustbot · 2026-07-30T07:54:27Z

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

jieyouxu · 2026-07-30T08:20:56Z

cf. #154607

jieyouxu · 2026-07-30T08:21:55Z

+        let panic_abort_target = builder
+            .ensure(MirOptPanicAbortSyntheticTarget { compiler: self.compiler, base: self.target });
+        run(panic_abort_target);


Question: hm, could we have this show up in a step resolution snapshot?

View changes since the review

We have it shown in the step snapshot tests. Do you want to separate it so that instead of running mir-opt directly within the step, we generate two steps, one without mir-opt and another with mir-opt? And when blessing, we move the generation of additional steps to make_run?

RalfJung · 2026-07-30T08:33:37Z

I don't know nearly enough about bootstrap to review this.^^
@rustbot reroll

RalfJung · 2026-07-30T08:34:28Z

+        // Now also run the tests for the host with panic=abort
+        let panic_abort_target = builder
+            .ensure(MirOptPanicAbortSyntheticTarget { compiler: self.compiler, base: self.target });
+        run(panic_abort_target);


This will be redundant with --bless I think?
mir-opt --bless already takes very long due to all the sysroots it is building, I'd prefer it not to become even slower.

View changes since the review

It is only redundant if the host target matches the hardcoded targets we have below. I can add an if condition to skip them if they match the host target.

That being said, if the stdlib it built, re-running the tests again should be very fast, they will just be ignored.

It is always redundant. There are only 4 actually distinct configurations, so running more than 4 targets on --bless is redundant.

I see, so the target doesn't really matter, the only thing that matters is bitwidth and the panic strategy? Yeah, in that case I will try to optimize it to always run 2 targets when not blessing, and 4 targets when blessing.

Yeah.

When checking it's good to check the actual self.target to ensure CI covers them all. But in principle only the bitwidth and panic strategy should matter.

RalfJung · 2026-07-30T08:38:24Z

Cc @saethlin

RalfJung · 2026-07-30T08:42:18Z

The existing behavior of --bless is kind of annoying since it takes so long to build all these sysroots... and it actually builds 5 sysroots when 4 should be enough. (With this PR as-is it'd become 6 sysroots!) Not sure if there is something we could improve there. Does bootstrap know whether self.target is a 32bit target or 64bit target? If yes, we could bless

current target
current target with panic=abort
target of other bitwidth
target of other bitwidth with panic=abort

And only run the first two steps for non-bless mode. That'd be nice.

Kobzol · 2026-08-07T07:28:14Z

And only run the first two steps for non-bless mode. That'd be nice.

But when we do the check, we should test all 4 combinations, right? Otherwise we could get back into the same problem, where some of the combinations are not checked on CI, and thus bless becomes "dirty".

RalfJung · 2026-08-07T07:30:57Z

We are running mir-opt tests on all tier 1 targets, right? So 32bit and 64bit are both covered.

Kobzol · 2026-08-07T07:32:05Z

Ah, I see, makes sense. Thanks.

Kobzol · 2026-08-07T08:24:41Z

Ok, I refactored the whole thing. Now when checking, we run the current target, and then the current target with the "inverted" panic strategy. And when blessing, we run the current target + four fixed targets, as before (there was a comment saying that running also the current target when blessing is desired).

It is now possible to create synthetic targets with both panic abort/unwind strategies, and the generated miropt test steps are now created as separate step instances, which means that they will be more explicit tin the step trace, and they will also be properly cached.

Kobzol · 2026-08-10T09:54:35Z

failures:
  
  ---- [mir-opt] tests/mir-opt/dont_reset_cast_kind_without_updating_operand.rs stdout ----
  103	+ }
  104	+ 
  105	+ ALLOC0 (size: 16, align: 8) {
  -	+     01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 │ ................
  +	+     00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 │ ................
  107	  }
  108	  
  109	
  
  
  thread '[mir-opt] tests/mir-opt/dont_reset_cast_kind_without_updating_operand.rs' panicked at src/tools/compiletest/src/runtest/mir_opt.rs:73:21:
  Actual MIR output differs from expected MIR output /checkout/tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.64bit.panic-abort.diff

Hmm, the aarch64 mir-opt abort tests are failing on CI on AArch64, even though all the 4 "pinned" variants have been blessed locally. If the targets tested on CI with panic-abort won't pass the mir-opt tests, then the mechanism for blessing with 4 hardcoded targets won't work anymore :/ @RalfJung

I'm also having trouble actually reblessing the Windows tests locally, uhh.

uilding stage1 library artifacts (stage1:x86_64-unknown-linux-gnu -> stage1:i686-pc-windows-msvc)
    Checking std v0.0.0 (/projects/personal/rust/rust/library/std)
error: invalid fragment specifier `try_update`
  --> library/windows_link/src/lib.rs:41:18
   |
41 | pub macro link($($tt:tt)*) {
   |                  ^^^^^^
   |
   = help: valid fragment specifiers are `ident`, `block`, `stmt`, `expr`, `pat`, `ty`, `lifetime`, `literal`, `path`, `meta`, `tt`, `item` and `vis`, along with `expr_2021` and `pat_param` for edition compatibility

error: invalid fragment specifier `try_update`
  --> library/windows_link/src/lib.rs:22:81
   |
22 |     ($library:literal $abi:literal $($link_name:literal)? $(#[$doc:meta])? fn $($function:tt)*) => (
   |                                                                                 ^^^^^^^^^^^^
   |
   = help: valid fragment specifiers are `ident`, `block`, `stmt`, `expr`, `pat`, `ty`, `lifetime`, `literal`, `path`, `meta`, `tt`, `item` and `vis`, along with `expr_2021` and `pat_param` for edition compatibility

error: could not compile `std` (lib) due to 2 previous errors
Build completed unsuccessfully in 0:00:06

RalfJung · 2026-08-10T15:50:03Z

This looks like something with endianess? But that should have already been a problem before?

Kobzol · 2026-08-11T07:48:02Z

In that case we will likely need a new axis, that also takes endianness into account? We never ran aarch64 panic=abort on CI before, this PR adds that, and it fails.

RalfJung · 2026-08-11T07:57:05Z

Is this aarch64 le or be? We surely ran aarch64 before...?

Kobzol · 2026-08-11T08:11:40Z

It should be little endian, but before this PR, we never ran its panic=abort mir-opt tests on CI. It looks like it has different output than the 64-bit panic=abort target that we bless.

RalfJung · 2026-08-11T09:13:45Z

That would be odd, both have the same endianess.

Kobzol · 2026-08-11T09:36:15Z

Hmm, I can't reproduce it locally on main. I wonder if something can be switching the endianness of this target on CI.

RalfJung · 2026-08-11T14:25:30Z

Looks like I miscounted, this is not endianess. It is the two fields of a tuple being swapped, or so? I'm not even sure what that allocation is for since that seems to be normalized away. @scottmcm @saethlin any idea why 64bit x86 and 64bit ARM might produce different mir-opt diffs here?

saethlin · 2026-08-13T04:36:36Z

The job config has randomize-layout=true so I think there's a struct that's being randomized differently depending on the target. Every ADT has a different seed based on its DefPathHash:

rust/compiler/rustc_middle/src/ty/mod.rs

Line 1706 in 52d0866

    
           let mut field_shuffle_seed = self.def_path_hash(did.to_def_id()).0.to_smaller_hash();

🤷 seems like a pretty basic conflict between using randomize-layout and dumping the contents of every allocation into the diff.

Kobzol · 2026-08-13T12:06:02Z

Would you be ok with just disabling layout randomization for the mir opt tests?

jieyouxu · 2026-08-13T12:25:36Z

Would you be ok with just disabling layout randomization for the mir opt tests?

IINM, that only turns off layout randomization for the non-std test portion, but not the already-built standard library which was already influenced by layout randomization? Or do you mean for the whole job?

Some approaches I can think of:

Normalize out allocation contents
- Under -Zui-testing, or
- Post-normalize
Don't -Zrandomize-layouts for the whole job
Only run mir-opt tests gated behind -Zrandomize-layouts=off (seems not ideal, prone to accidentally never running it)

Not sure if there's other viable approaches because none of them seem ideal...

Kobzol · 2026-08-13T12:37:37Z

Hmm, I meant only the mir-opt tests, I didn't realize that std would be affected by that. Though we can build stdlib without layout randomization only for the mir-opt tests, maybe?

jieyouxu · 2026-08-13T12:41:56Z

Hmm, I meant only the mir-opt tests, I didn't realize that std would be affected by that. Though we can build stdlib without layout randomization only for the mir-opt tests, maybe?

Hm... maybe. I think with our existing handling, that has to invalidate all steps depending on built std?

Since stage 1 std built with versus without randomize-layouts is incompatible, so that has to invalidate all steps depending on stage 1 std. It'd probably have to look like 2 separate ./x test invocations

# Run all `./x` that "don't care" about randomize-layouts, or should always pass under randomize-layouts=true|<seed>
./x test --stage=2 ui ui-fulldeps pretty
# Run all `./x` test that do care about randomize-layouts
./x test --stage=2 mir-opt

If we only tested up to stage 1 {rustc,std} that'd be fine, except I think this would need to rebuild stage 1 std => stage 2 rustc. Which, probably is not that bad time-wise?

saethlin · 2026-08-13T14:24:19Z

Yes, this is the usual issue that codegen tests require a separate sysroot, not whatever hodgepodge of settings is configured in bootstrap.toml.

scottmcm · 2026-08-14T15:49:33Z

In the codegen(-llvm) tests the answer for this is just to add

//@ needs-deterministic-layouts

to the test. Maybe we just need to do that for these mir-opt tests that show layouts too?

And only include the target name when rendering test metadata, to avoid including filenames in it.

…anicStrategy`

… of targets to check

rustbot · 2026-08-20T06:50:50Z

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.

Kobzol · 2026-08-20T08:21:21Z

Ok, seems like that has helped, thank you!

@rustbot ready

rustbot assigned RalfJung Jul 30, 2026

This comment has been minimized.

Sign in to view

jieyouxu self-assigned this Jul 30, 2026

jieyouxu reviewed Jul 30, 2026

View reviewed changes

rustbot assigned jackh726 and unassigned RalfJung and jieyouxu Jul 30, 2026

RalfJung reviewed Jul 30, 2026

View reviewed changes

jieyouxu assigned jieyouxu and unassigned jackh726 Jul 30, 2026

This comment has been minimized.

Sign in to view

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

Kobzol force-pushed the mir-opt-panic-abort-on-ci branch from a65622e to b63868d Compare August 7, 2026 08:23

This comment has been minimized.

Sign in to view

Kobzol force-pushed the mir-opt-panic-abort-on-ci branch from 8a2d0f7 to d328e8e Compare August 14, 2026 14:23

This comment has been minimized.

Sign in to view

Kobzol added 7 commits August 20, 2026 08:49

Bless bootstrap tests

f161976

And only include the target name when rendering test metadata, to avoid including filenames in it.

Generalize MirOptPanicAbortSyntheticTarget to `SyntheticTargetWithP…

dc54f90

…anicStrategy`

Create targets for mir-opt tests explicitly and use the minimal set…

a6b9b38

… of targets to check

Fix host normalization

c739077

Bless at most four individual targets

50dc2e3

Do not call configure_linker for synthetic targets

55a180b

Add needs-deterministic-layouts flag

c833721

Kobzol force-pushed the mir-opt-panic-abort-on-ci branch from d328e8e to c833721 Compare August 20, 2026 06:50

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 Aug 20, 2026

Uh oh!

Uh oh!

Conversation

Kobzol commented Jul 30, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 30, 2026

Uh oh!

This comment has been minimized.

jieyouxu commented Jul 30, 2026

Uh oh!

jieyouxu Jul 30, 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!

Kobzol Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Jul 30, 2026

Uh oh!

RalfJung Jul 30, 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!

Kobzol Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Jul 30, 2026 • 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!

Kobzol Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Jul 30, 2026

Uh oh!

RalfJung commented Jul 30, 2026

Uh oh!

This comment has been minimized.

Kobzol commented Aug 7, 2026

Uh oh!

RalfJung commented Aug 7, 2026 via email

Uh oh!

Kobzol commented Aug 7, 2026

Uh oh!

This comment has been minimized.

Kobzol commented Aug 7, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Kobzol commented Aug 10, 2026

Uh oh!

RalfJung commented Aug 10, 2026 via email

Uh oh!

Kobzol commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Aug 11, 2026 via email

Uh oh!

Kobzol commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Aug 11, 2026 via email

Uh oh!

Kobzol commented Aug 11, 2026

Uh oh!

RalfJung commented Aug 11, 2026

Uh oh!

saethlin commented Aug 13, 2026

Uh oh!

Kobzol commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jieyouxu commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kobzol commented Aug 13, 2026

Kobzol commented Jul 30, 2026 •

edited by rustbot

Loading

jieyouxu Jul 30, 2026 •

edited by rustbot

Loading

RalfJung Jul 30, 2026 •

edited by rustbot

Loading

RalfJung Jul 30, 2026 •

edited

Loading

Kobzol commented Aug 11, 2026 •

edited

Loading

Kobzol commented Aug 11, 2026 •

edited

Loading

Kobzol commented Aug 13, 2026 •

edited

Loading

jieyouxu commented Aug 13, 2026 •

edited

Loading

jieyouxu commented Aug 13, 2026 •

edited

Loading