RFR: 8351568: Improve source code documentation for PhaseCFG::insert_anti_dependences

[Daniel Lundén]

On Tue, 29 Apr 2025 05:01:16 GMT, Galder Zamarreño <galder at openjdk.org> wrote:

>> The current documentation for `PhaseCFG::insert_anti_dependences` is difficult to follow and sometimes even misleading. We should ensure the method is appropriately documented.
>> 
>> ### Changeset
>> 
>> - Rename `PhaseCFG::insert_anti_dependences` to `PhaseCFG::raise_above_anti_dependences`. The purpose of `PhaseCFG::raise_above_anti_dependences` is twofold: raise the load's LCA so that the load is scheduled before anti-dependent stores, and if necessary add anti-dependence edges between the load and certain anti-dependent stores (to ensure we later "raise" the load before anti-dependent stores in LCM). The name `PhaseCFG::insert_anti_dependences` suggests that we only add anti-dependence edges. The name `PhaseCFG::raise_above_anti_dependences`, therefore, seems more appropriate.
>> - Significantly add to and revise the source code documentation of `PhaseCFG::raise_above_anti_dependences`.
>> - Add, move, and revise `assert`s in `PhaseCFG::raise_above_anti_dependences`, including improved `assert` messages in a few places.
>> - In the main worklist loop of `PhaseCFG::raise_above_anti_dependences`:
>>   - Clean up how we identify the search root (avoid mutation).
>>   - Add a missing early exit for `Phi` nodes when `LCA == early`.
>> 
>> ### Testing
>> 
>> - [GitHub Actions](https://github.com/dlunde/jdk/actions/runs/14706896111)
>> - `tier1` to `tier4` (and additional Oracle-internal testing) on Windows x64, Linux x64, Linux aarch64, macOS x64, and macOS aarch64.
>
> test/hotspot/jtreg/compiler/loopopts/TestSplitIfPinnedLoadInStripMinedLoop.java line 141:
> 
>> 139: 
>> 140:     // Same as test2 but with reference to inner loop induction variable 'j' and different order of instructions.
>> 141:     // Triggers an assert in PhaseCFG::raise_above_anti_dependences if loop strip mining verification is disabled:
> 
> Is this test still valid? According to the comment it should trigger an assert but this assert appears to be removed? Is the test correct if the test is passing even though the assert has been removed? See my above comment on the removal of this assertion.

Ah, good catch. Let me try to verify that my new asserts also trigger if I revert the fix for [JDK-8260420](https://bugs.openjdk.org/browse/JDK-8260420).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24926#discussion_r2068365753