-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add MIR Optimization Tests #34715
Add MIR Optimization Tests #34715
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
let file_name = format!("rustc.node{}{}.{}.{}.mir", | ||
node_id, promotion_id, pass_name, disambiguator); | ||
let _ = fs::File::create(&file_name).and_then(|mut file| { | ||
file_path.push_str("/"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this mean that, if no path is specified, the MIR will be dumped into the root folter (/rustc.node[...].mir
)? Using PathBuf
can avoid that, and would be semantically more appropriate and more robust anyway.
Makefile changes look good to me! Could you also be sure to update |
@alexcrichton is there a bot that runs rustbuild? I think the travis bot builds with make, right? I'm pretty sure I tested this with both build systems, but we could double check. |
@scottcarr all correct! I'm fine letting bors figure out the rest |
|
||
let mir_dump_dir = self.get_mir_dump_dir(); | ||
if !mir_dump_dir.exists() { | ||
fs::create_dir(mir_dump_dir.clone()).expect("the dir should exist"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be the "racy" create-dir option, probably-- though I guess each test will be making disjoint directories, so maybe it's not needed.
Can you maybe add the |
But basically r+ from me |
36d589d
to
1669574
Compare
@bors r+ |
📌 Commit 8f9844d has been approved by |
⌛ Testing commit 8f9844d with merge 6a8b9e9... |
💔 Test failed - auto-win-gnu-32-opt-rustbuild |
@bors: retry On Thu, Jul 21, 2016 at 11:01 AM, bors notifications@github.com wrote:
|
⌛ Testing commit 8f9844d with merge 9121b49... |
💔 Test failed - auto-win-gnu-32-opt |
@bors: retry On Thu, Jul 21, 2016 at 11:56 AM, bors notifications@github.com wrote:
|
⌛ Testing commit 8f9844d with merge d46ed83... |
Add MIR Optimization Tests I've starting working on the infrastructure for testing MIR optimizations. The plan now is to have a set of test cases (written in Rust), compile them with -Z dump-mir, and check the MIR before and after each pass.
I've starting working on the infrastructure for testing MIR optimizations.
The plan now is to have a set of test cases (written in Rust), compile them with -Z dump-mir, and check the MIR before and after each pass.