RFR: 8325589: C2 SuperWord refactoring: create VLoopAnalyzer with Submodules [v3]

[Christian Hagedorn]

On Wed, 14 Feb 2024 09:51:15 GMT, Christian Hagedorn <chagedorn 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 eight commits:
>> 
>>  - manual merge
>>  - move _loop_or_ctrl from ResouceArena, bc ResourceMark in SuperWord::dependence_graph
>>  - remove some comments
>>  - VLoopTypes
>>  - VLoopBody
>>  - VLoopMemorySlices
>>  - VLoopReductions
>>  - 8325589
>
> src/hotspot/share/opto/vectorization.cpp line 144:
> 
>> 142:   // Skip any loop that has not been assigned max unroll by analysis.
>> 143:   if (SuperWordLoopUnrollAnalysis && vloop().cl()->slp_max_unroll() == 0) {
>> 144:     return VLoopAnalyzer::FAILURE_NO_MAX_UNROLL;
> 
> How about directly printing here instead of going over a failure string and returning false? You could, for example, have a method `trace_setup_submodules_failure(failure_string)`, and directly pass the string of `FAILURE_NO_MAX_UNROLL` here. Otherwise, I guess you still need to find this usage of `FAILURE_NO_MAX_UNROLL` here to understand exactly which conditions failed. By directly printing here, you have the entire context of the error.
> 
> Doing that would also allow the merge of `setup_submodules()` and `setup_submodules_helper()` into a single method.
> 
> It's just a suggestion - I leave it up to you to decide :-)

Another thought here: Shouldn't this check here already be performed in `VLoop::check_preconditions_helper()`?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17800#discussion_r1489235297