RFR: 8349032: C2: Parse Predicate refactoring in Loop Unswitching broke fix for JDK-8290850

[Vladimir Kozlov]

On Thu, 20 Feb 2025 12:18:57 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> In the refactoring for [JDK-8344035](https://bugs.openjdk.org/browse/JDK-8344035), the value passed for the `rewire_uncommon_proj_phi_inputs` parameter in `PhaseIdealLoop::create_new_if_for_predicate()` during Loop Unswitching was accidentally flipped. It should only be set to `true` when calling it for a false-path loop, which is the cloned loop. This is currently not the case and leads to a bad graph due to folding nodes wrongly:
> https://github.com/openjdk/jdk/blob/735805d9259037ae594eb4f75e96860d43feea5d/src/hotspot/share/opto/predicates.cpp#L84-L88
> 
> I fixed this by just flipping the parameter from `is_true_path_loop` to `is_false_path_loop` to avoid a negation. I added an additional comment to `PhaseIdealLoop::create_new_if_for_predicate()` about `rewire_uncommon_proj_phi_inputs`.
> 
> More background about why we need `rewire_uncommon_proj_phi_inputs` in the first place can be found in the corresponding fix for [JDK-8290850](https://bugs.openjdk.org/browse/JDK-8290850): https://github.com/openjdk/jdk/pull/11452, and additionally in https://github.com/openjdk/jdk/pull/5185
> 
> Thanks,
> Christian

src/hotspot/share/opto/loopPredicate.cpp line 100:

> 98: // new_iff is returned which is an IfTrue projection. This code is also used to clone predicates to cloned loops.
> 99: // 'rewire_uncommon_proj_phi_inputs' should be set to the non-default value 'true' when called for a false-path loop
> 100: // during Loop Unswitching.

Just nitpick. Can you return back length of comment's lines?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23712#discussion_r1964092615