Option::map_or generates a bunch of garbage IR #46758
Comments
arielb1
added
the
I-compiletime
|
So the main problem is that this pollution goes further than |
|
@arielb1 I'd like to help with this. Would you consider this a good issue for mentorship? The linked issue suggests this might be hard to tackle...? |
|
Here is that IR transliterated into Rust syntax to be more readable at a glance. impl Option<String> {
fn map_or(self: *Option<String>, default: i32) {
let personalityslot: struct(*i8, i32);
let t: String;
let ret: i32;
let _12 = true;
let _11 = true;
let _10 = true;
let %3 = if self.ptr == null { 0i64 } else { 1i64 }
match %3 {
0 => goto 'bb2,
1 => goto 'bb4,
_ => goto 'bb3,
}
'bb1:
panic!(personalityslot);
'bb2:
_11 = false;
ret = default;
goto 'bb5;
'bb3:
unreachable!();
'bb4:
_10 = false;
t = *(self as *Some<String> as *String);
_12 = false;
try {
let %20 = (main::{{closure}})(&t);
goto 'bb7;
} catch_unwind {
goto 'cleanup;
}
'bb5:
if _12 {
goto 'bb15;
} else {
goto 'bb8;
}
'bb6:
let %26 = if self.ptr == null { 0i64 } else { 1i64 };
match %26 {
1 => goto 'bb10,
_ => goto 'bb12,
}
'bb7:
ret = %20;
goto 'bb5;
'bb8:
if _11 {
goto 'bb16;
} else {
goto 'bb9;
}
'bb9:
let %32 = if self.ptr == null { 0i64 } else { 1i64 };
match %32 {
1 => goto 'bb18,
_ => goto 'bb20,
}
'bb10:
if _10 {
goto 'bb11;
} else {
goto 'bb1;
}
'bb11:
_10 = false;
ptr::drop_in_place::<String>(self as *Some<String> as *String);
goto 'bb1;
'bb12:
ptr::drop_in_place::<Option<String>>(self);
goto 'bb1;
'bb13:
_11 = false;
goto 'bb6;
'bb14:
if _11 {
goto 'bb13;
} else {
goto 'bb6;
}
'bb15:
_12 = false;
goto 'bb8;
'bb16:
_11 = false;
goto 'bb9;
'bb17:
return ret;
'bb18:
if _10 {
goto 'bb19;
} else {
goto 'bb17;
}
'bb19:
_10 = false;
ptr::drop_in_place::<String>(self as *Some<String> as *String)
goto 'bb17;
'bb20:
ptr::drop_in_place::<Option<String>>(self);
goto 'bb17;
'cleanup:
personalityslot = landingpad();
goto 'bb14;
}
} |
dtolnay
added
the
T-compiler
|
|
|
The first thing that stands out is that some of these drop flags (?) are useless. In particular let mut _12 = true;
let mut _11 = true;
if _12 {
_12 = false;
}
if _11 {
_11 = false;
}
// never used again@arielb1 are these somehow tracking whether a non-Drop type has been dropped, which is unnecessary? Or is it more complicated? Eliminating these unused flags would cut the number of basic blocks by about a third. |
|
These drop flags should trivially disappear in near future as a consequence of us starting to do constant propagation with MIRI. |
|
Note that initially we'll only run the const-eval to produce warnings, without actually modifying the MIR , to avoid misoptimizations. But we do want to do const-prop soon after. |
|
Thanks @nagisa @eddyb. MIR constant propagation is nice but in this case some of the drop flags are not controlling any behavior. Check out the drop flags |
|
Response in #rustc:
I will investigate with Just from the LLVM IR it looks like @arielb1 do you have a sense of where this IR is "garbage"? Which of the basic blocks would you expect us not to be emitting or expect to be simpler? |
|
IIRC, these drop flags are used if the destructor of one of the types (i.e. |
|
Would you be able to briefly suggest a path forward for @mrhota or someone else to start working on? |
|
The MIR cfg is the following - see https://play.rust-lang.org/?gist=a642496d2949e24738b27b030644f833&version=stable (not sure how to upload graphviz graphs): I'm actually quite unsure of the best way to do it, but it should be possible to turn this into a better generic MIR. This requires some work to look at it and find ways to optimize. |
|
Might it help to represent Are there MIR passes that need to see the control flow in a conditional drop as control flow? |
|
Adding more complexity to the MIR shouldn't be needed here IMO. |
XAMPPRocky
added
the
C-enhancement
jonas-schievink
added
A-codegen
workingjubilee
added
the
C-optimization
|
Current optimized, generic MIR for Option::map_or contains only two drop flags:
This is pretty good already, but not perfect. With I think jump threading, maybe some other opts on (-Zmir-opt-level=4), we get this:
which eliminates the closure's drop flag entirely but not the alternative value's (_2) -- maybe too high cost in the pass, not entirely clear. The LLVM IR for the particular case at the top isn't ideal -- we still generate some no-op goto basic blocks -- but I don't think we can do much at the generic MIR level there, it's mostly due to drop(usize) being a no-op. |
|
I don't think there are any clear further opportunities left that need to be tracked here, vs. just enabling e.g. jump threading by default (as is planned: #117206). |
See e.g.
Which generates this initial IR:
Which is quite a bit of IR for such a little function:
I suspect #46525 might be involved in causing drop elaboration to go crazy
The text was updated successfully, but these errors were encountered: