RFR: 8342941: IGV: Add various new graph dumps during loop opts [v3]

[Saranya Natarajan]

On Fri, 11 Jul 2025 07:18:17 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> src/hotspot/share/opto/phasetype.hpp line 83:
>> 
>>> 81:   flags(AFTER_REMOVE_EMPTY_LOOP,        "After Remove Empty Loop") \
>>> 82:   flags(BEFORE_ONE_ITERATION_LOOP,      "Before Replacing One Iteration Loop") \
>>> 83:   flags(AFTER_ONE_ITERATION_LOOP,       "After Replacing One Iteration Loop") \
>> 
>> Very much a nit, but I think this should be "One-Iteration Loop". Or, is it in fact one _iteration loop_ (as it reads now)? Looking at the code, I think it is the former. @chhagedorn can maybe clarify?
>> 
>> This is not specific to your changeset, but also appears in existing source code comments. Maybe a good opportunity to clean this up everywhere?
>> 
>> Also, maybe "Replacing" should be "Replace"? Seems to better fit the style used for other phase names.
>
> One-Iteration loop sounds better indeed. I also agree with the other suggestions.
> 
> Something else I've noticed is that we could also benefit when we add dumps for `duplicate_loop_backedge()` which creates a new loop node (i.e. could be seen as "major modification"). I just looked into recently and found myself adding dumps there manually for debugging. I guess since this is a dump adding RFE, we could also add that one. What do you think? But then we would need to update the PR title to something like "add various new graph dumps during loop opts".

Thank you for the comments. I have made the suggested changes to the source code. Attached is a screenshot for the graph dump for  `duplicate_loop_backedge() `. I have added the suggested new title.

<img width="348" height="101" alt="image" src="https://github.com/user-attachments/assets/e401f994-17c1-442c-9f91-f25255964a4e" />

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25756#discussion_r2205559207