JIT: Revise DFS RPO computation #82752
Conversation
Give block postorder numbers their customary meaning: the block's position in the postorder, not in the reverse postorder. Rename "InvPostOrder" to the more customary ReversePostorder. Also, compute preorder numbers. Preparatory work for running DFS RPO earlier to classify edges.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsGive block postorder numbers their customary meaning: the block's position in the postorder, not in the reverse postorder. Rename "InvPostOrder" to the more customary ReversePostorder. Also, compute preorder numbers. Preparatory work for running DFS RPO earlier to classify edges.
|
|
@jakobbotsch PTAL |
| @@ -1279,11 +1293,11 @@ BasicBlock* Compiler::fgIntersectDom(BasicBlock* a, BasicBlock* b) | |||
| BasicBlock* finger2 = b; | |||
| while (finger1 != finger2) | |||
| { | |||
| while (finger1->bbPostOrderNum > finger2->bbPostOrderNum) | |||
| while (finger1->bbPostorderNum < finger2->bbPostorderNum) | |||
There was a problem hiding this comment.
Note this method is now very similar to the SSA builder's IntersectDom -- the main difference being how nodes with no predecessors are handled.
| @@ -961,12 +975,12 @@ void Compiler::fgComputeDoms() | |||
| bbRoot.bbPreds = nullptr; | |||
| bbRoot.bbNum = 0; | |||
| bbRoot.bbIDom = &bbRoot; | |||
| bbRoot.bbPostOrderNum = 0; | |||
| bbRoot.bbPostorderNum = fgBBNumMax + 1; | |||
There was a problem hiding this comment.
This is one subtle part I got wrong initially -- the fake "root" BB must now have a postnumber larger than the maximum possible postnumber instead of a reverse postnumber smaller than the the minimum possible reverse postnumber.
|
As hinted at above, the SSA builder's DFS numbering was already using postnumbers this way, so now the two uses are similar. There are still other differences between the two dominance computations. We really ought to just have one. |
|
Runtime failure is very likely unrelated, opened #82771. |
ghost
locked as resolved and limited conversation to collaborators
Give block postorder numbers their customary meaning: the block's position in the postorder, not in the reverse postorder.
Rename "InvPostOrder" to the more customary ReversePostorder. Also, compute preorder numbers.
Preparatory work for running DFS RPO earlier to classify edges.