Add a MIR pass to simplify the control flow graph #29757
Conversation
dotdash
force-pushed
the
mir_simplify_cfg
branch
2 times, most recently
from
cc76611 to
099e32c
Compare
|
|
||
| impl SimplifyCFG { | ||
| fn remove_dead_blocks(&self, mir: &mut Mir) { | ||
| let mut seen: Vec<_> = mir.basic_blocks.iter().map(|_| false).collect(); |
There was a problem hiding this comment.
This could be vec![false; mir.basic_blocks.len()].
There was a problem hiding this comment.
Right, I always forget about that form of vec! for some reason, thanks!
|
It might be useful to make MirPasses something we can hook into via plugin_registrar (quite an easy change to make, just maintain a vec of It would also make it easy to work on experimental passes out-of-tree and without long builds. |
|
@Manishearth I intent to add some kind of pass manager as a follow-up. That would allow to add passes dynamically and also provide means to store some analysis results across passes, like a predecessor map which would stay valid across passes that don't modify the graph structure. |
|
How expensive is this? (i.e. does this need to obey (I imagine not, and I guess it may even improve compile times in particularly complicated cases by reducing the interactions with LLVM (maybe?), but seems worth considering.) |
|
@huonw It seems pretty cheap. Testing a function that has 1000 times this: if x > 0 { let _y = 0 } else { let _y = 0 }The pass can't optimize those because of the Using 1000 of this instead: if x > 0 {} else {}The pass can drop a ton of empty blocks. Again, the MIR dump time is pretty much unchanged at 5ms, but since the MIR is reduced so much, LLVM passes drop from 16ms to 10ms. So a net win of 6ms. And the MIR as we currently build it has so many empty blocks that we really should run this pass once even when optimizations are off. |
| use transform::util; | ||
| use transform::MIRPass; | ||
|
|
||
| pub struct SimplifyCFG; |
There was a problem hiding this comment.
Nit: Usually we provide a top-level fn like simplify_cfg::simplify(...) rather than calling a method on a singleton; not that I care very much. And yes the capitalization convention is SimplifyCfg (but I saw that mentioned elsewhere).
There was a problem hiding this comment.
If a pass manager is added later, this needs to be a singleton, as the relevant method is in the MirPass trait, and the pass manager would hold Box<MirPass>es.
There was a problem hiding this comment.
I'll go with a new method on SimplifyCfg, and then call apply on its result.
|
OK, this looks good to me, but there have been enough comments etc I'd like to take a look at the next revision before giving r+. So ping me when it's ready. :) |
Yeah, after posting that comment I was thinking about how your version was O(D) where D is dead blocks (or at least the main loop is). Honestly I have no idea how the perf works out. I'm not sure which is better. |
dotdash
force-pushed
the
mir_simplify_cfg
branch
from
099e32c to
36309e8
Compare
| let mut replacements: Vec<_> = (0..num_blocks).map(BasicBlock::new).collect(); | ||
|
|
||
| let mut dead = 0; | ||
| for i in 0..num_blocks { |
There was a problem hiding this comment.
it occurs to me that we could make this more efficient by doing:
let first_dead = match keep.iter().position(|&k| !k) {
None => return,
Some(first_dead) => first_dead,
};
let mut dead = 0;
let mut replacements: Vec<_> = (0..num_blocks).map(BasicBlock::new).collect();
for i in first_dead .. num_blocks {
if keep[i] {
replacements[i] = BasicBlock::new(i - dead);
mir.basic_blocks.swap(i, i - dead);
} else {
dead += 1;
}
}
For now, this pass does some easy transformations, like eliminating empty blocks that just jump to another block, some trivial conversion of If terminators into Gotos and removal of dead blocks.
dotdash
force-pushed
the
mir_simplify_cfg
branch
from
36309e8 to
a4e5c0f
Compare
|
@bors r+ Nice! Obviously we'll have to figure out what to do w/ the |
|
📌 Commit a4e5c0f has been approved by |
For now, this pass does some easy transformations, like eliminating empty blocks that just jump to another block, some trivial conversion of If terminators into Gotos and removal of dead blocks. r? @nikomatsakis
For now, this pass does some easy transformations, like eliminating
empty blocks that just jump to another block, some trivial
conversion of If terminators into Gotos and removal of dead blocks.
r? @nikomatsakis