RFR: 8347901: C2 should remove unused leaf / pure runtime calls [v3]

[Marc Chevalier]

On Mon, 7 Jul 2025 07:53:24 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   mostly comments
>
> src/hotspot/share/opto/divnode.cpp line 1522:
> 
>> 1520:   Node* super = CallLeafPureNode::Ideal(phase, can_reshape);
>> 1521:   if (super != nullptr) {
>> 1522:     return super;
> 
> Can't we just do `return CallLeafPureNode::Ideal(phase, can_reshape);` at the end of `ModFNode::Ideal` instead of `return nullptr`? That's what we usually do in C2, for example in `CallStaticJavaNode::Ideal` -> `CallNode::Ideal`. Feels more natural to me and would avoid the `super != nullptr` check. Also for the other `Ideal` methods that you modified.

We could but it's not that direct. `ModFNode::Ideal` has 6 `return`s (without mine):
- 3 are `return replace_with_con(...);` which in their turn return `nullptr` but after making changes in the graph.
- 2 are `return nullptr;`
- 1 is actually returning a node.
And especially the final one is

return replace_with_con(igvn, TypeF::make(jfloat_cast(xr)));

If we change `replace_with_con` to actually return a `TupleNode` to do the job, we still have 2 places where to call the base class' `Ideal`. So I'm not sure how much better it would be to duplicate the call. It also adds a maintenance burden: if one adds another case where we don't want to make changes, one needs to add another call to `CallLeafPureNode::Ideal`. I think it's because of the structure of this function: rather than selecting cases where we want to do something and reaching the end with only the leftover cases, we select the cases we don't want to continue with, and we return early, making more cases where we should call the super method.

I'll try something.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25760#discussion_r2192664653