[jdk17] RFR: 8269820: C2 PhaseIdealLoop::do_unroll get wrong opaque node

[Vladimir Kozlov]

On Mon, 19 Jul 2021 14:13:31 GMT, Hui Shi <hshi at openjdk.org> wrote:

> More discussion in previous PR(https://github.com/openjdk/jdk17/pull/208) is confilict with fix for JDK-8269752.
> 
> Opaque1 in main loop entry is expected only used in compare node's second input. However split if might clone opaque1 and replace original node with phi of opaque1 node. This causes assertion in CountedLoopNode::is_canonical_loop_entry now.
> 
> Fix is in BoolNode::Ideal, avoiding switching compare node's input when second input is phi of opaque1.
> 
> Test: Linux X64 tier1/2/3 release/fastdebug no regression.

There could be a case when Opaque1 is used by predicate which follows a Region without been placed there by code Roland is pointing. Then Roland's suggested fix may not work in general case.
And I understand the concern about hidden Opaque1 behind Phis. I need to look on this case to make suggestion. In general we do split_if if we can fold values on one of paths. I think we should not do it for Cmp which reference Opaque1. But I need more details.

I will look on this case today.

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

PR: https://git.openjdk.java.net/jdk17/pull/255