RFR: 8321278: C2: Partial peeling fails with assert "last_peel <- first_not_peeled" [v3]

[Christian Hagedorn]

On Fri, 22 Mar 2024 16:40:32 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> Thanks for reviewing this.
> 
> > Removing the regions during `Dominators()` seems reasonable. I guess doing a pass of IGVN after `Dominators()` is probably too much to get these regions removed?
> 
> It does feel like a lot of overhead for such a simple corner case.

Indeed, I don't think it's worth.

> > Could you also remove the region and the phi where the unreachable loops are cleaned up and the region and phis become single-entry nodes? I.e. here:
> 
> I considered it but it felt harder. The algorithm collects cfg nodes in dfs and then iterate over them several times. If we remove a region at the point you mention, it would need to be removed from the dfs node list. Or we would need to make later iterations over the dfs node list handle dead region nodes. It's not a problem when it's done later as I propose here.

I see, thanks for the explanation. Then it makes sense to handle this edge-case like you proposed to keep things simple. Maybe you can add a comment accordingly why we remove the region at this point.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18353#discussion_r1537156282