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

Hendrik Schick duke at openjdk.org
Tue Apr 29 11:58:48 UTC 2025


On Mon, 28 Apr 2025 15:28:52 GMT, Daniel Lundén <dlunden 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.

src/hotspot/share/opto/gcm.cpp line 674:

> 672: //
> 673: //   1. raise the load's LCA to force the load to (eventually) be scheduled at
> 674: //      latest in the stores's block, and

Suggestion:

//      latest in the store's block, and

src/hotspot/share/opto/gcm.cpp line 682:

> 680: // path relative to the load if there are no paths from early to LCA that go
> 681: // through the store's block. Such stores are not anti-dependent, and there is
> 682: // no need to update the LCA nor to add anti-depencence edges.

Suggestion:

// no need to update the LCA nor to add anti-dependence edges.

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

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


More information about the hotspot-compiler-dev mailing list