RFR: 8295166: IGV: dump graph at more locations [v5]

[Christian Hagedorn]

On Thu, 7 Dec 2023 13:01:58 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.
>
> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove trailing whitespace

Otherwise, it looks good to me! Thanks for addressing all the comments and suggestions.

src/hotspot/share/opto/loopopts.cpp line 1451:

> 1449:     C->print_method(PHASE_BEFORE_SPLIT_IF, 4, iff);
> 1450:     if ((PrintOpto && VerifyLoopOptimizations) || TraceLoopOpts) {
> 1451:       tty->print_cr("Split-if");

Suggestion:

      tty->print_cr("Split-If");

src/hotspot/share/opto/phasetype.hpp line 55:

> 53:   flags(AFTER_LOOP_UNROLLING,           "After Loop Unrolling") \
> 54:   flags(BEFORE_SPLIT_IF,                "Before Split If") \
> 55:   flags(AFTER_SPLIT_IF,                 "After Split If") \

I suggest to use a `-`:
Suggestion:

  flags(BEFORE_SPLIT_IF,                "Before Split-If") \
  flags(AFTER_SPLIT_IF,                 "After Split-If") \

test/hotspot/jtreg/compiler/lib/ir_framework/CompilePhase.java line 66:

> 64:     AFTER_LOOP_UNROLLING("After Loop Unrolling"),
> 65:     BEFORE_SPLIT_IF("Before Split If"),
> 66:     AFTER_SPLIT_IF("After Split If"),

Suggestion:

    BEFORE_SPLIT_IF("Before Split-If"),
    AFTER_SPLIT_IF("After Split-If"),

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16120#pullrequestreview-1770210494
PR Review Comment: https://git.openjdk.org/jdk/pull/16120#discussion_r1418995778
PR Review Comment: https://git.openjdk.org/jdk/pull/16120#discussion_r1419002759
PR Review Comment: https://git.openjdk.org/jdk/pull/16120#discussion_r1419003229