@dotnet/jit-contrib PTAL

This assert was firing in xunit code. Because of how xunit randomizes test execution order the PGO data for this method is quite different from one run to the next, so it took a while to repro this.

Are we not walking the blocklist in RPO here? If not, can we? That way, we never waste time with unreachable blocks.

Are we not walking the blocklist in RPO here? If not, can we? That way, we never waste time with unreachable blocks.

We are generally walking in RPO, but when we find a CEA opportunity we need to walk backwards in flow from the GDV uses, and that is where we may find unreachable preds.

We are generally walking in RPO, but when we find a CEA opportunity we need to walk backwards in flow from the GDV uses, and that is where we may find unreachable preds.

I see. Normally, if a block's pred becomes unreachable, I'd expect the previous phase to remove the edge into the block, so we wouldn't encounter unreachable preds during normal pred edge iteration. In this case, is the unreachable pred we're finding an EH pred?

Sorry for asking for these clarifications when the repro is tricky...

We are generally walking in RPO, but when we find a CEA opportunity we need to walk backwards in flow from the GDV uses, and that is where we may find unreachable preds.

I see. Normally, if a block's pred becomes unreachable, I'd expect the previous phase to remove the edge into the block, so we wouldn't encounter unreachable preds during normal pred edge iteration. In this case, is the unreachable pred we're finding an EH pred?

Sorry for asking for these clarifications when the repro is tricky...

Yes it's EH flow from some blocks that couldn't be removed by the DFS/remove phase. We have deeply nested EH in this huge example, here's snippet of the flow graph

BB449 [0539]  0  4  5                       0       0 [000..000)                             (throw ) T4 H5 try { }     i IBC rare keep bwd
BB450 [0540]  1  6  4                       0       0 [000..000)                             (throw ) T6 H4 finally { } i IBC rare keep bwd
...
BB52  [0051]  1  7    BB39                  1.00  162 [???..???)-> BB40(1)                   (callf ) T7                i IBC internal
BB53  [0052]  1  7    BB43                  1.00  162 [???..???)-> BB44(1)                   (callfr) T7                i IBC internal
BB40  [0039]  2  7  6 BB52                  1.00  162 [4AB..4AF)-> BB43(0),BB41(1)           ( cond ) T7 H6 finally {   i IBC keep

BB449/BB450 are unreachable but remain; BB40 has BB450 as an EH pred and is part of the region we want to clone for CEA.

Along those lines, we might want to revisit places where we decide we can't remove blocks because of EH, as much of it was written before we had upgraded the flow graph representation and EH removal capabilities.

Along those lines, we might want to revisit places where we decide we can't remove blocks because of EH, as much of it was written before we had upgraded the flow graph representation and EH removal capabilities.

Agreed, sounds like something I can take a look at.