[lworld] RFR: 8337747: [lworld] Refactor Loop Unswitching code after merging in JDK-8325746

[Christian Hagedorn]

On Tue, 6 Aug 2024 11:05:42 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> In the merge of JDK-8325746 for tag jdk-23+12, I've applied just a minimal refactoring to get things to work with as few changes as possible in Valhalla and decided to make a full refactoring separately with this patch.
> 
> This patch includes the following:
> - Improved/updated comments
> - New `UnswitchCandidate` class:
>   - Since we can have multiple unswitch candidates with flat array checks, I've introduced a new class `UnswitchCandidate` which finds either a unique candidate or a list of flat array checks which can also be queried from this class.
>   - Offers methods `update_in_false/true_path_loop()` methods to remove the dominated unswitch candidates (previously in `OriginalLoop::remove_unswitch_candidate_from_loops()`
>   - Offers method `merge_flat_array_checks()` to create a merged flat array check bool (previously in `UnswitchedLoopSelector::find_unswitch_candidate()`)
> - Updated `UnswitchedLoopSelector` class:
>   - Now only responsible for creating the loop unswitching loop selector and not for finding the unswitch candidate - this is passed into the class now.   
> - Clean up `PhaseIdealLoop::find_unswitch_candidates()`
> - Clean up printing for `TraceLoopOpts` and `TraceLoopUnswitching`. Now the details are only guarded with `TraceLoopUnswitching` to reduce the noise with `TraceLoopOpts`. Example output with `TraceLoopUnswitching`:
> 
> Loop Unswitching:
> - Unswitch-Candidate-If: 519 If
> - Loop-Selector-If: 2094 If
> - True-Path-Loop (=Orig): 1346 Loop
> - False-Path-Loop (=Clone): 2221 Loop
> - Unswitched Flat Array Checks:
>   - 519 If  ->  2226 If
>   - 922 If  ->  2208 If
>   - 685 If  ->  2359 If
>   - 580 If  ->  2388 If
> 
> 
> Thanks,
> Christian

src/hotspot/share/opto/loopUnswitch.cpp line 170:

> 168:     }
> 169:   }
> 170:   return unswitch_candidate;

Extracted to `collect_flat_array_checks()`

src/hotspot/share/opto/loopUnswitch.cpp line 210:

> 208:     return unswitch_candidate;
> 209:   }
> 210: 

Now in `UnswitchCandidate::find_unswitch_candidate()`.

src/hotspot/share/opto/loopUnswitch.cpp line 221:

> 219: //                    (i.e. a fast path loop).
> 220: // - False-path-loop: We keep all flat array checks in this loop (i.e. a slow path loop).
> 221: class UnswitchCandidate : public StackObj {

New class as mentioned in PR description.

src/hotspot/share/opto/loopUnswitch.cpp line 237:

> 235:       }
> 236:     }
> 237: 

Now in `UnswitchCandidate::merge_flat_array_checks()`

src/hotspot/share/opto/loopUnswitch.cpp line 347:

> 345:     }
> 346:   }
> 347: 

Now in `UnswitchCandidate::update_in_true/false_path_loop()`

src/hotspot/share/opto/loopUnswitch.cpp line 434:

> 432:       unswitch_iffs.at(i)->dump(3);
> 433:       tty->cr();
> 434:     }

Removed to reduce noise in favor of `TraceLoopUnswitching` (see `PhaseIdealLoop::trace_loop_unswitching_result()`).

src/hotspot/share/opto/loopUnswitch.cpp line 459:

> 457:                                              cloned_unswitch_iff->_idx, cloned_unswitch_iff->Name());
> 458:       }
> 459:     }

Extracted to `UnswitchCandidate::trace_flat_array_checks()`

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1194#discussion_r1705344490
PR Review Comment: https://git.openjdk.org/valhalla/pull/1194#discussion_r1705345564
PR Review Comment: https://git.openjdk.org/valhalla/pull/1194#discussion_r1705344724
PR Review Comment: https://git.openjdk.org/valhalla/pull/1194#discussion_r1705345973
PR Review Comment: https://git.openjdk.org/valhalla/pull/1194#discussion_r1705346536
PR Review Comment: https://git.openjdk.org/valhalla/pull/1194#discussion_r1705347380
PR Review Comment: https://git.openjdk.org/valhalla/pull/1194#discussion_r1705347789