rBreak Critical Edges and other MIR work #32210
Conversation
|
/cc #32105 |
|
I would do the transform in the MIR pipeline, rather than in a separate pass. |
|
@arielb1 I was considering this: #32105 (comment). Given that it's more of a "utility" pass, I'm not sure it really makes sense to have it in the main set of passes anyway, at least until we have a proper MIR pass that needs it. |
|
I would either run the pass in the |
|
☔ The latest upstream changes (presumably #31916) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@arielb1 I was considering a general "trans preparation" phase, similar to LLVM's CodegenPrepare, so that sounds good. It should probably include the erase regions pass too. |
|
cc me (I wish PR mentions did this automatically) |
The reason I added the end-block in the first place is that it's often convenient when doing analysis to have an easy reference point to walk backwards from. But we can always patch this up later I imagine, if it makes sense. |
👍 |
| /// | ||
| /// A preorder traversal of this graph is either `A B D C` or `A C D B` | ||
| #[derive(Clone)] | ||
| pub struct Preorder<'a, 'tcx: 'a> { |
There was a problem hiding this comment.
This kind of code is crying out for unit tests, but I'm not sure what'd be the easiest way to add them. The only thing I can think of is trying to make the core logic generic, which is probably more trouble than its worth -- though I've thought about making some kind of graph trait that MIR (as well as librustc_data_structures::graph::Graph) could implement, precisely so that we could write (and test) these sorts of algorithms generically.
|
r=me |
dotdash
changed the title
|
Apologies for the delay here. My laptop needed repair the day #32080 landed... I'll rebase and update soon. |
Adds Preorder, Postorder and Reverse Postorder traversal iterators. Also makes trans/mir use Reverse Postorder traversal for blocks.
This is a fairly standard transform that inserts blocks along critical edges so code can be inserted along the edge without it affecting other edges. The main difference is that it considers a Drop or Call terminator that would require an `invoke` instruction in LLVM a critical edge. This is because we can't actually insert code after an invoke, so it ends up looking similar to a critical edge anyway. The transform is run just before translation right now.
Some blocks won't be translated at all because they aren't reachable at the LLVM level, these need to be dealt with because they lack a terminator and therefore trigger an LLVM assertion. Other blocks aren't reachable because of codegen-time optimistions, for example not dropping types that don't need it, often resulting in blocks with no predecessors. We'll clean those up as well.
Aatch
force-pushed
the
mir-traversal
branch
2 times, most recently
from
71c948a
to
42c856a
Compare
|
Rebased and updated. I added a pipeline for running MIR passes that prepare the MIR for translation. |
Also adds a new set of passes to run just before translation that "prepare" the MIR for codegen. Removal of landing pads, region erasure and break critical edges are run in this pass. Also fixes some merge/rebase errors.
|
@bors r+ |
|
📌 Commit 63321ca has been approved by |
|
⌛ Testing commit 63321ca with merge dfd4040... |
|
💔 Test failed - auto-win-msvc-64-opt-mir |
|
Figured out what failure was. I wasn't making the inserted blocks inherit the |
Also adds a FromIterator impl for BitVector to allow construction of a BitVector from an iterator yeilding bools.
|
⌛ Testing commit 605bc04 with merge 251fd8b... |
|
💔 Test failed - auto-win-msvc-64-opt-rustbuild |
|
@bors retry |
rBreak Critical Edges and other MIR work This PR is built on top of #32080. This adds the basic depth-first traversals for MIR, preorder, postorder and reverse postorder. The MIR blocks are now translated using reverse postorder. There is also a transform for breaking critical edges, which includes the edges from `invoke`d calls (`Drop` and `Call`), to account for the fact that we can't add code after an `invoke`. It also stops generating the intermediate block (since the transform essentially does it if necessary already). The kinds of cases this deals with are difficult to produce, so the test is the one I managed to get. However, it seems to bootstrap with `-Z orbit`, which it didn't before my changes.
This PR is built on top of #32080.
This adds the basic depth-first traversals for MIR, preorder, postorder and reverse postorder. The MIR blocks are now translated using reverse postorder. There is also a transform for breaking critical edges, which includes the edges from
invoked calls (DropandCall), to account for the fact that we can't add code after aninvoke. It also stops generating the intermediate block (since the transform essentially does it if necessary already).The kinds of cases this deals with are difficult to produce, so the test is the one I managed to get. However, it seems to bootstrap with
-Z orbit, which it didn't before my changes.