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

[Emanuel Peter]

On Thu, 11 May 2023 09:18:21 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

> The canonicalization of MinI/MaxI chains into right-spline graphs within `MinINode/MaxINode::Ideal()` inhibits the vectorization of reductions of these nodes. This changeset reworks `MinINode/MaxINode::Ideal()` to perform the same algebraic optimizations without the need for canonicalization, re-enabling auto-vectorization of MinI/MaxI reductions. This is achieved by handling all four permutations of the targeted Ideal subgraph induced by the commutativity of MinI/MaxI directly. The algorithm (for the MaxI case, the MinI case is analogous) tries to apply two Ideal graph rewrites in the following order, where `c0` and `c1` are constants and `MAX` is a compile-time operation:
> 1. `max(x + c0, max(x + c1, z))` (or a permutation of it) to `max(x + MAX(c0, c1), z)`.
> 2. `max(x + c0, x + c1)` (or a permutation of it) to `x + MAX(c0, c1)`.
> 
> Here is an example of the four permutations handled in step 1 with `x = RShiftI`, `c0 = 100` or `150`, `c1 = 150` or `100`, and `z = ConI (#int:200)`:
> 
> ![two-level-idealization](https://github.com/openjdk/jdk/assets/8792647/bf60a2c3-39cd-4f0d-965d-c711723e374c)
> 
> Here is an example of the two permutations handled in step 2 with `x = RShiftI`, `c0 = 10` or `11`, and `c1 = 11` or `10`:
> 
> ![one-level-idealization](https://github.com/openjdk/jdk/assets/8792647/0a1fe85b-3f30-46bc-8817-d90b3eff946c)
> 
> The changeset implements `MinINode/MaxINode::Ideal()` in a common method `MaxNode::IdealI()`, since the algorithm is symmetric for both node types. The changeset also extends the existing MinI/MaxI Idealization tests with positive tests for all targeted permutations and negative tests, and adds a new test (contributed by @jbhateja) to assert that MinI/MaxI reductions are vectorized.
> 
> #### Testing
> 
> ##### Functionality
> 
> - tier1-5, stress test, fuzzing (windows-x64, linux-x64, linux-aarch64, macosx-x64, macosx-aarch64; release and debug mode).
> 
> ##### Performance
> 
> - Tested performance on a set of standard benchmark suites (DaCapo, SPECjbb2015, SPECjvm2008). No significant change was observed.

src/hotspot/share/opto/addnode.cpp line 1103:

> 1101:   }
> 1102:   return n;
> 1103: }

This was confusing to read.

Why not make it a `as_add_constant`, and explicitly always set the `con`:

case AddI(x, top): return <nullptr, 0>
case AddI(x, int_con): return <x, int_con>
default: return <n, 0>

That would make it easier to argue what the value of `con` is after the call - it does certainly not depend on what it was before.

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

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