RFR: 8350579: Remove Template Assertion Predicates belonging to a loop once it is folded away [v2]
Christian Hagedorn
chagedorn at openjdk.org
Tue Mar 25 08:50:11 UTC 2025
On Tue, 25 Mar 2025 08:23:31 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
>> 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?
No, it cannot be true. `opaque_node->loop_node()` is a `CountedLoop`. We could have only `opaque_node->loop_node() == _loop_node`. I think I first had that as an assertion in place but then turned it into a bailout. But you're right, it would already be covered by the check. Given it's a rare edge case, I guess we can get rid of it. Then the reason above does not apply anymore that we have 2 steps but only 1. Then it does not make sense to split it further. I merged it back together.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23823#discussion_r2011591603
More information about the hotspot-compiler-dev
mailing list