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

Marc Chevalier mchevalier at openjdk.org
Fri Jun 20 08:16:11 UTC 2025


On Wed, 18 Jun 2025 14:30:53 GMT, Emanuel Peter <epeter 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/compile.cpp line 3313:
> 
>> 3311:       for (unsigned int i = 0; i < call->tf()->domain()->cnt() - TypeFunc::Parms; i++) {
>> 3312:         new_call->init_req(TypeFunc::Parms + i, call->in(TypeFunc::Parms + i));
>> 3313:       }
> 
> The `TypeFunc::Parms` offsets are a bit confusing / unnecessary. Why not:
> Suggestion:
> 
>       // Copy all <something>.
>       for (unsigned int i = TypeFunc::Parms; i < call->tf()->domain()->cnt(); i++) {
>         new_call->init_req(i, call->in(i));
>       }

Fine with me! Looks perfectly identical to me. Comment seems superfluous to me: the code is very simple, let's not distract the reader.

> src/hotspot/share/opto/divnode.cpp line 1530:
> 
>> 1528: 
>> 1529:   if (is_unused()) {
>> 1530:     return make_tuple_of_input_state_and_top_return_values(igvn->C);
> 
> What does this do? Add a short comment :)

Same as above: better commented on definition than bloating every single callsite of a function. If I wonder, I go see the definition, if I don't, I don't have useless stuff on my screen.

> src/hotspot/share/opto/multnode.hpp line 113:
> 
>> 111:   const TypeTuple* _tf;
>> 112: 
>> 113:   template <typename... NN>
> 
> Does this need to be a template? Or would a type like `Node*` or `Node` suffice?

The point is that it's a variadic template. Of course, ideally, I'd like it to be a pack of `Node*` but there isn't a simple way to write that. The upside is that one can write `TupleNode::make(some_type, input1, input2, input3, input4)` with how many input you want in a single construction. I prefer hiding the not nice here, to have compact and readable usages of TupleNode, rather than having every usage look like

TupleNode* tuple = new TupleNode(some_type);
tuple.set_req(0, input1);
tuple.set_req(1, input2);
tuple.set_req(2, input3);
tuple.set_req(3, input4);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25760#discussion_r2158312752
PR Review Comment: https://git.openjdk.org/jdk/pull/25760#discussion_r2158313274
PR Review Comment: https://git.openjdk.org/jdk/pull/25760#discussion_r2158310606


More information about the graal-dev mailing list