RFR: 8326139: C2 SuperWord: split packs (match use/def packs, implemented, mutual independence) [v2]

[Emanuel Peter]

On Mon, 26 Feb 2024 18:14:25 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 27 commits:
>> 
>>  - manual merge
>>  - more fixes for TestSplitPacks.java
>>  - fix some IR rules in TestSplitPacks.java
>>  - fix MulAddS2I
>>  - Fix data dependency IR rules in LoopArrayIndexComputeTest.java
>>  - fix populate index cases and test7
>>  - test7 WIP
>>  - WIP fix reductions
>>  - Update test/hotspot/jtreg/compiler/loopopts/superword/TestSplitPacks.java
>>    
>>    fix bug number
>>  - cleanup
>>  - ... and 17 more: https://git.openjdk.org/jdk/compare/490825fb...c39ace04
>
> src/hotspot/share/opto/superword.cpp line 1492:
> 
>> 1490: // Split pack according to task. Return true if any change was made, else false.
>> 1491: SuperWord::SplitStatus
>> 1492: SuperWord::split_pack(const char* split_name, Node_List* pack, SplitTask task) {
> 
> Style. Please, put return type on the same line as method. Yes, the line will be bigger but I prefer to have arguments on separate line than the return type.

Fixed. Used to have this code-style in my previous company 🙈

> src/hotspot/share/opto/superword.hpp line 436:
> 
>> 434:   private:
>> 435:     const uint _split_size;
>> 436:     const bool _reject;
> 
> Should this be enum with 3 states you listed in comment?

Refactored it :)

> src/hotspot/share/opto/superword.hpp line 471:
> 
>> 469:     Node_List* _first_pack;
>> 470:     Node_List* _second_pack;
>> 471:     bool _changed;
> 
> Again, should this be enum?

Refactored it :)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17848#discussion_r1503917212
PR Review Comment: https://git.openjdk.org/jdk/pull/17848#discussion_r1503917609
PR Review Comment: https://git.openjdk.org/jdk/pull/17848#discussion_r1503917817