RFR: 8295166: IGV: dump graph at more locations
Tobias Hartmann
thartmann at openjdk.org
Tue Dec 5 12:12:56 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)
>  
>
>
> 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.
Nice enhancement! Looks good to me overall. Some comments below.
Please add an IGV screenshot of how the new phases look in the phase list.
Could you summarize which of the ideas / proposals from the RFE are not covered? We should file a follow-up RFE for them.
> Many of the proposed dump locations are educated guesses. Should we adjust any of them?
They look good to me but let's see what others think.
> Are the proposed levels (for PrintIdealGraphLevel) reasonable or should we adjust them? I put the loop optimization dumps at level 4 and adjusted IdealGraphVisualizer/README.md accordingly.
I think that's reasonable.
> I put most new calls to print_method/print_method_iter within a NOT_PRODUCT. Is this OK?
Existing calls to `print_method` are not guarded because the method also commits a JFR event and updates `Compile::_latest_stage_start_counter`, so I think your new code should behave similar.
And please make sure that you verify that the 'optimized' build (`--with-debug-level optimized`) still works. It's a level between fastdebug and release where both `#ifdef ASSERT` and `#ifdef PRODUCT` are false.
src/hotspot/share/opto/compile.cpp line 625:
> 623: #ifndef PRODUCT
> 624: _igv_idx(0),
> 625: _igv_phase_iter(),
This is value initialization which guarantees proper zeroing, right? For other arrays, for example `Compile::_trap_hist`, we use explicit `Copy::zero_to_bytes` but I think your variant is fine.
src/hotspot/share/opto/compile.cpp line 5115:
> 5113: print_method(cpt, level, n, iter);
> 5114: #else
> 5115: print_method(cpt, level, n);
This is dead code because all calls are guarded by `NOT_PRODUCT`, right?
src/utils/IdealGraphVisualizer/.gitignore line 6:
> 4: /lastModified/
> 5: /localeVariants
> 6: /package-attrs.dat
Is that really needed? Just wondering why these files haven't been added before.
-------------
PR Review: https://git.openjdk.org/jdk/pull/16120#pullrequestreview-1673474298
PR Review Comment: https://git.openjdk.org/jdk/pull/16120#discussion_r1356396506
PR Review Comment: https://git.openjdk.org/jdk/pull/16120#discussion_r1356402215
PR Review Comment: https://git.openjdk.org/jdk/pull/16120#discussion_r1356405379
More information about the hotspot-compiler-dev
mailing list