RFR: 8347555: [REDO] C2: implement optimization for series of Add of unique value [v20]

Kangcheng Xu kxu at openjdk.org
Mon Oct 6 14:17:13 UTC 2025


On Mon, 6 Oct 2025 11:34:57 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   refactor Multiplication into a class
>
> src/hotspot/share/opto/addnode.cpp line 447:
> 
>> 445:   }
>> 446: 
>> 447:   Node* con = (bt == T_INT)
> 
> With an `if`, the `static_cast<Node*>` are not needed, right? I would do that then. It would more readable.

Good point. Used `if` instead. Thanks!

> src/hotspot/share/opto/addnode.cpp line 524:
> 
>> 522:   if (n->Opcode() == Op_Mul(bt) && (n->in(1)->is_Con() || n->in(2)->is_Con())) {
>> 523:     // Pattern (1)
>> 524:     Node* con = n->in(1);
> 
> Isn't con always input 2 because `MulNode::Ideal` canonicalize it?

I think you're right. An assertion is also added to verify this.

> src/hotspot/share/opto/addnode.hpp line 74:
> 
>> 72:     Multiplication add(const Multiplication rhs) const {
>> 73:       if (is_valid_with(rhs.variable()) && rhs.is_valid_with(variable())) {
>> 74:         return {variable(), java_add(multiplier(), rhs.multiplier())};
> 
> You should use a constructor call here

clang-tidy suggested to

> Avoid repeating the return type from the declaration; use a braced initializer list instead 

I believe they generate the same code for this case, but I updated to explicitly use constructor making it more clear. Thanks!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23506#discussion_r2406573167
PR Review Comment: https://git.openjdk.org/jdk/pull/23506#discussion_r2406573652
PR Review Comment: https://git.openjdk.org/jdk/pull/23506#discussion_r2406574112


More information about the hotspot-compiler-dev mailing list