mir-opt tests generate an excessive amount of output #109502
Assignees
Labels
A-contributor-roadblock
Area: Makes things more difficult for new contributors to rust itself
A-mir-opt
Area: MIR optimizations
A-testsuite
Area: The testsuite used to check the correctness of rustc
E-help-wanted
Call for participation: Help is requested to fix this issue.
E-medium
Call for participation: Medium difficulty. Experience needed to fix: Intermediate.
E-mentor
Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
scottmcm
added
A-mir-opt
jyn514
added
the
A-testsuite
|
Looks like currently we pass rust/src/tools/compiletest/src/runtest.rs Line 1983 in bb7c373 dumped filesWe already parse the comments in the file to see which files to compare: rust/src/tools/compiletest/src/runtest.rs Lines 3476 to 3479 in bb7c373 hopefully it shouldn't be too tricky to work backwards from that to determine which pass to dump. |
jyn514
added
E-help-wanted
|
Hello! Interested in contributing here, but might need some help. I understand that an output file is being created for each MIR pass and that creates a huge amount of data which is problematic, but I am not sure what the desired output would be. Do we want just one output file for each test instead of an output file for each MIR pass? |
In an ideal world, yes. Given that -Zdump-mir doesn't support that, I think "one output file for each pass under test" is going to be close enough - in practice I think most mir-opt tests only look at one pass anyway. |
|
Ok, thanks. I think I am missing something, is that not what is happening currently? For example, if I look at the output in Output
Is each one of these not an output from an MIR pass? For example, I am assuming |
|
Yes, -Zdump-mir=all is working correctly. I'm saying we shouldn't be using -Zdump-mir=all in the first place - we should only emit the passes we are actively verifying for that test, not all passes that happen to change the MIR. |
|
For example, the only check in -Zdump-mir=built.
|
|
Got it, thanks for clarifying, for some reason I thought you were saying -Zdump-mir=all was mandatory, must have misread something. Anyway I'll give this one a go @rustbot claim |
Noratrieb
added
the
T-compiler
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this issue
May 5, 2023
Reduce MIR dump file count for MIR-opt tests As referenced in issue rust-lang#109502 , mir-opt tests previously used the -Zdump-mir=all flag, which generates very large output. This PR only dumps the passes under test, greatly reducing dump output.
|
Is there any reason this issue is still open? @mj10021's PR containing the changes discussed was merged, and output has been reduced significantly. |
|
I remember this being much better after those changes, so I'll just close it. Yeah, 6× fewer files and 4× smaller size. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are 237 mir-opt tests right now.
Running them on my machine produces over 69 thousand files, using over 400 MB of disk (⅓ of which is filesystem overhead):
Could that be cut back somewhat? Windows is not a fan of lots of files -- even just deleting all those files takes over 13 seconds, despite being on a fast M2 PCI-e SSD.
The text was updated successfully, but these errors were encountered: