RFR: 8302673: [SuperWord] MaxReduction and MinReduction should vectorize for int [v2]

[Roberto Castañeda Lozano]

On Wed, 31 May 2023 11:06:26 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Roberto Castañeda Lozano has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 22 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into JDK-8302673
>>  - Defer op(x, x) to constant/identity propagation early
>>  - Merge branch 'master' into JDK-8302673
>>  - Refactor idealization and extracted Identity transformation for clarity
>>  - Make auxiliary add operand extraction function return a tuple
>>  - Randomize array values in min/max test computation
>>  - Merge branch 'master' into JDK-8302673
>>  - Merge branch 'master' into JDK-8302673
>>  - Refine comments
>>  - Update copyright header
>>  - ... and 12 more: https://git.openjdk.org/jdk/compare/d7cb4210...a6db3cc4
>
> src/hotspot/share/opto/addnode.cpp line 1141:
> 
>> 1139:   }
>> 1140:   return ConstAddOperands(x, c_type->is_int()->get_con());
>> 1141: }
> 
> This is what it was on my last review:
> 
> 
> // Return:
> // <x, C>,       if n is of the form x + C, where 'C' is a non-TOP constant;
> // <nullptr, _>, if n is of the form x + C, where 'C' is a TOP constant;
> // <n, con>      otherwise.
> static Node* constant_add_input(Node* n, jint* con) {
>   if (n->Opcode() == Op_AddI && n->in(2)->is_Con()) {
>     const Type* t = n->in(2)->bottom_type();
>     if (t == Type::TOP) {
>       return nullptr;
>     }
>     *con = t->is_int()->get_con();
>     n = n->in(1);
>   }
>   return n;
> }
> 
> Here, you used to also allow packing just a single `n`, and leave the constant as `zero`. Did you remove this possibility on purpose? Now `n` must be an `AddI`.
> 
> This used to allow cases like this to be folded:
> `max(max(a, b),  a + 1)      ->      max(a + max(0, 1), b)`
> 
> Or am I missing something? Do you have tests for this case?

Good catch, thanks Emanuel! The simplification of `as_add_with_constant()` was too aggressive, and the lost optimizations were not caught by any test or noticeable regression on the standard Java benchmark suites. I added more test cases to catch them now and reverted `as_add_with_constant()` to return `<n, 0>`  for non-additions.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13924#discussion_r1212860862