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

[Emanuel Peter]

On Mon, 24 Mar 2025 15:19:54 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> 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 :-)

I leave it up to you :)

But could `opaque_node->loop_node() == _loop_node` even be true if we have `!_loop_node->is_CountedLoop()`? or do we actually need a CountedLoop to even have the match?

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

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