RFR: 8347555: [REDO] C2: implement optimization for series of Add of unique value [v19]
    Kangcheng Xu 
    kxu at openjdk.org
       
    Thu Oct  2 22:28:39 UTC 2025
    
    
  
On Thu, 2 Oct 2025 08:03:23 GMT, Roland Westrelin <roland at openjdk.org> wrote:
>> Kangcheng Xu has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 69 commits:
>> 
>>  - Merge remote-tracking branch 'origin/master' into arithmetic-canonicalization
>>  - update naming and comments
>>  - Merge branch 'openjdk:master' into arithmetic-canonicalization
>>  - Merge remote-tracking branch 'origin/master' into arithmetic-canonicalization
>>  - Allow swapping LHS/RHS in case not matched
>>  - Merge branch 'refs/heads/master' into arithmetic-canonicalization
>>  - improve comment readability and struct helper functions
>>  - remove asserts, add more documentation
>>  - fix typo: lhs->rhs
>>  - update comments
>>  - ... and 59 more: https://git.openjdk.org/jdk/compare/0366d882...ce23d393
>
> src/hotspot/share/opto/addnode.cpp line 555:
> 
>> 553: 
>> 554:     // Pattern (1)
>> 555:     if (lhs.valid && rhs.valid && lhs.variable == rhs.variable) {
> 
> Would it make sense to add a method to `Multiplication`, let's say `add`. The 3 `if`s here would then be replaced by:
> 
> Multiplication res = lhs.add(rhs);
> if (res.valid()) {
>   return res;
> }
> return find_simple_addition_pattern(n, bt);
> 
> and the logic from the 3 ifs would be moved to `Multiplication::add`.
> What do you think?
I added `add(Mulitplication)` and moved `Pattern (1)` to it. However, for pattern `(2)` and `(3)`, one side is not a `Multiplcation` but a simple node (`(a << CON) + a` and `a + (a << CON)`). Therefore I left the two other `if`s here.
> src/hotspot/share/opto/addnode.hpp line 46:
> 
>> 44:   virtual uint hash() const;
>> 45: 
>> 46:   struct Multiplication {
> 
> Is there a benefit to this being a `struct` instead of a `class`?
No, I think not. Earlier iterations were much simpler so I used `struct`. I've updated the code to be `class`
> src/hotspot/share/opto/addnode.hpp line 60:
> 
>> 58:   };
>> 59: 
>> 60:   static Multiplication find_collapsible_addition_patterns(const Node* a, const Node* pattern, BasicType bt);
> 
> Shouldn't these methods be member methods of `Multiplication`
Move to static members of `Multiplication`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23506#discussion_r2400258106
PR Review Comment: https://git.openjdk.org/jdk/pull/23506#discussion_r2400250708
PR Review Comment: https://git.openjdk.org/jdk/pull/23506#discussion_r2400253138
    
    
More information about the hotspot-compiler-dev
mailing list