RFR: 8295166: IGV: dump graph at more locations

Christian Hagedorn chagedorn at openjdk.org
Thu Dec 7 07:55:40 UTC 2023


On Tue, 10 Oct 2023 13:31:00 GMT, Daniel Lundén <duke at openjdk.org> wrote:

> This changeset
> 1. adds a number of new graph dumps for IdealGraphVisualizer (IGV):
>    - Before conditional constant propagation
>    - After register allocation
>    - After block ordering
>    - After peephole optimization
>    - After post-allocation expansion
>    - Before and after
>      - loop predication
>      - loop peeling
>      - pre/main/post loops
>      - loop unrolling
>      - range check elimination
>      - loop unswitching
>      - partial peeling
>      - split if
>      - superword
> 2. adds support for enumeration of repeated IGV graph dumps.
> 3. adjusts IGV print levels to encompass the new graph dumps. The old levels 4 and 5 are now levels 5 and 6. The new level 4 is for loop optimization dumps.
> 
> Example phase list screenshots in IGV (first at level 6, second at level 4)
> ![Screenshot from 2023-12-04 13-55-38](https://github.com/openjdk/jdk/assets/4222397/6759dc5a-9c9a-42b9-8d9e-2d0b53e76ab4) ![Screenshot from 2023-12-04 13-56-29](https://github.com/openjdk/jdk/assets/4222397/44d6a239-587b-4f7c-8ce1-f7613cb2fa35)
> 
> 
> Some notes:
> - While discussing the above changes, a separate question was brought up by @chhagedorn:
>   > On a separate note, I'm wondering how useful it is to always dump all JFR events when calling print_method(). Should this be revisited again in general?
> - The new IGV graph dump enumeration enables a number of cleanups. There is now another RFE for IGV cleanup: [JDK-8319599](https://bugs.openjdk.org/browse/JDK-8319599).
> 
> ### Testing
> #### Platforms: windows-x64, linux-x64, linux-aarch64, macosx-x64, macosx-aarch64
> - tier1, tier2, tier3, tier4, tier5.
> - Check that optimized builds (`--with-debug-level optimized`) still work.
> 
> #### Platforms: linux-x64
> - Tested that thousands of graphs are correctly opened and visualized with IGV.

src/hotspot/share/opto/split_if.cpp line 595:

> 593: void PhaseIdealLoop::do_split_if(Node* iff, RegionNode** new_false_region, RegionNode** new_true_region) {
> 594: 
> 595:   C->print_method(PHASE_BEFORE_SPLIT_IF, 4, iff);

We call `do_split_if()` for the actual split if here:
https://github.com/openjdk/jdk/blob/632a3c56e0626b4c4f79c8cb3d2ae312668d63fc/src/hotspot/share/opto/loopopts.cpp#L1448-L1450

but also to merge identical back to back ifs here:

https://github.com/openjdk/jdk/blob/632a3c56e0626b4c4f79c8cb3d2ae312668d63fc/src/hotspot/share/opto/loopopts.cpp#L1529-L1545

I think we should only track the "real" split ifs done in the former case. Should we move this and the `tty` printing to L1448? But could also be done separately.

On a separate note, I think we do not need the printing twice and can merge these two lines as well when doing this change:

  if (PrintOpto && VerifyLoopOptimizations) {
    tty->print_cr("Split-if");
  }
  if (TraceLoopOpts) {
    tty->print_cr("SplitIf");
  }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16120#discussion_r1418517568


More information about the hotspot-compiler-dev mailing list