RFR: 8330469 : Removed or replaced (PrintOpto && VerifyLoopOptimizations) …

[Christian Hagedorn]

On Mon, 10 Mar 2025 10:34:19 GMT, Saranya Natarajan <duke at openjdk.org> wrote:

> **Issue:** There are currently 9 occurrences where we guard printing code with PrintOpto && VerifyLoopOptimizations. This flag combo is never really used in practice. 
> 
> **Solution**: I analysed the 9 occurrence. In cases, where the flag `PrintOpto && VerifyLoopOptimizations` was followed by flag `TraceLoopOpts` with `else if` or  `|| operator` I removed the former flag. In other cases, where `PrintOpto && VerifyLoopOptimizations` was the only flag, I was replaced with `TraceLoopOpts`. 
> 
> **Test Result**: Link to [GitHub Action](https://github.com/sarannat/jdk/actions/runs/13723071055) run on commit [91ecc51](https://github.com/sarannat/jdk/commit/91ecc5190ce31da94bded4de210136f337286e69)

Thanks for cleaning this up! I have a few suggestions.

src/hotspot/share/opto/loopTransform.cpp line 756:

> 754:       if (TraceLoopOpts) {
> 755:         tty->print("Peeling a 'main' loop; resetting to 'normal' ");
> 756:         loop->dump_head();

I think you can remove `loop->dump_head()` since you already dump the head on L734.

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

> 840:     if (TraceLoopOpts) {
> 841:       tty->print_cr("CMOV");
> 842:     }

I think you can just remove this since we already print "CMOV" down on L866.

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

> 849:         if (m != nullptr && !is_dominator(get_ctrl(m), cmov_ctrl)) {
> 850: #ifndef PRODUCT
> 851:           if (TraceLoopOpts) {

I think it's too verbose for `TraceLoopOpts` which should only dump the high-level information. Since there is only one place where we dump additional information for this cmove optimization, I suggest to just drop this.

If we want to trace the cmove optimization at some point, we might better introduce a "TraceConditionalMove" flag.

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

> 141:     tty->print("Cloning up: ");
> 142:     n->dump();
> 143:   }

Same here with this print and the other places in `clone_cmp_down()`: I think it's too verbose for `TraceLoopOpts`. 

Since Split-If is quite complex, I think it would make sense to add a "TraceSplitIf" flag to get more information about the optimization. It's probably out of scope of this bug, so we could do that in a separate RFE. For this PR, I suggest to just drop these printings and link this PR to the "TraceSplitIf" RFE in order to restore/update/improve these.

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

PR Review: https://git.openjdk.org/jdk/pull/23959#pullrequestreview-2670661693
PR Review Comment: https://git.openjdk.org/jdk/pull/23959#discussion_r1987122635
PR Review Comment: https://git.openjdk.org/jdk/pull/23959#discussion_r1987125824
PR Review Comment: https://git.openjdk.org/jdk/pull/23959#discussion_r1987141524
PR Review Comment: https://git.openjdk.org/jdk/pull/23959#discussion_r1987147472