RFR: 8350579: Remove Template Assertion Predicates belonging to a loop once it is folded away [v2]

[Christian Hagedorn]

On Mon, 24 Mar 2025 12:33:33 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
>> 
>>  - Small things
>>  - Fix test comments
>>  - New approach: Marking unrelated Template Assertion Predicates outside of IGVN by storing a reference to the loop it originally was created for.
>>  - Merge branch 'master' into JDK-8350579
>>  - Revert fix completely
>>  - 8350579: Remove Template Assertion Predicates belonging to a
>>     loop once it is folded away during IGVN
>
> src/hotspot/share/opto/predicates.cpp line 1267:
> 
>> 1265:       template_assertion_predicate.opaque_node()->mark_useful();
>> 1266:     }
>> 1267:   }
> 
> I'm not sure if it makes to split this into two methods, but that's subjective 😅 
> 
> It seems to me that the code in `visit` is an optimization for what happens in `mark_template_useful_if_matching_loop`, and does not really make sense on its own.

The reasons I've split it is the following:
- The bailout for non-counted loops is actually separate to the marking. So I have a two-step algorithm: bailout + marking which can nicely be split.
- Having `mark_template_useful_if_matching_loop()` allows me to quickly read `visit()` and understand what's going on. Additionally, I can put the details about why we do the marking at the method comment for more interested code readers. Without the extracted method, I would probably need to put an extra "mark template useful if matching loop" comment + the 6 lines of comments at `mark_template_useful_if_matching_loop()` into the `visit()` method which makes it harder to grasp.

I would prefer to stick to what I have now - but I admit it's a subjective matter :-)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23823#discussion_r2010396527