JDK-8302673 : [SuperWord] MaxReduction and MinReduction should vectorize for int
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 21
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2023-02-16
  • Updated: 2024-08-01
  • Resolved: 2023-06-05
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 21
21 b26Fixed
Related Reports
Blocks :  
Relates :  
Description
I found this during the work of JDK-8302139.

See example Test.java

./java -XX:-TieredCompilation -Xbatch -XX:CompileCommand=compileonly,Test::test -XX:+TraceSuperWord Test.java

    static int test(int[] data) {
        int res = 1;
        for (int i = 2; i < N-2; i++) {
            int v = data[i] * 11;
            res = Math.min(res, v);
        }
        return res;
    }

In TraceSuperWord, we can see that it tries to vectorize.
find_adjacent_refs: we detect the LoadI
extend_packlist: we extend to the MulI, but unfortunately not the MinI
combine_packs: combines the LoadI and MulI (we are missing the MinI)
filter_packs: rejects the whole vectorization because the MinI were not put in a pack, now the MulI has a non-vectorized use in the loop.

A first analysis showed that maybe something with the order of the MinI is unexpected.
Maybe the inputs of the min / max get flipped, and it does not get detected correctly in SuperWord::reduction(Node* s1, Node* s2)

This is both for Math.min and Math.max.

The same example with float array does already vectorize.

Double does not vectorize because of this issue: JDK-8300865
For long it did not even try to vectorize with SuperWord. Maybe because of this line in x86.ad:
      } else if (bt == T_LONG && (UseAVX < 3 || !VM_Version::supports_avx512vlbwdq())) {
Yeah, probably. I have AVX512 on my laptop, but avx512vlbwdq is missing. So it would have to be tested on a machine that has that hardware support.
Comments
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/13260 Date: 2023-03-31 07:30:30 +0000
01-08-2024

Changeset: 3fa776d6 Author: Roberto CastaƱeda Lozano <rcastanedalo@openjdk.org> Date: 2023-06-05 07:08:33 +0000 URL: https://git.openjdk.org/jdk/commit/3fa776d66a8eb117410025bca870b2e7f3f00517
05-06-2023

A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/13924 Date: 2023-05-11 09:18:21 +0000
12-05-2023

Here is a prototype (WIP) that performs the same Ideal optimization without canonicalization: https://github.com/openjdk/jdk/compare/master...robcasloz:jdk:JDK-8302673. This approach has the advantage of not interfering with auto-vectorization (thus regaining the missing optimization reported in this RFE), and generally separating concerns (so that we do not need to e.g. reason within Ideal() about whether the involved nodes are part of a reduction chain). [~jbhateja] would you like me to take over this RFE?
14-04-2023

Re-opened, as this RFE is orthogonal to JDK-8287087, see https://github.com/openjdk/jdk/pull/13120#discussion_r1166778814.
14-04-2023

Marking as duplicate of JDK-8287087.
01-04-2023

Min/MaxI1( Min/MaxI2(a,b), c) into Min/MaxI1( a, Min/MaxI2(b,c) ) looks like a canonicalizing transformation to supplement downstream optimization. We can prevent this if participating nodes are marked with reduction flags.
30-03-2023

[~sviswanathan] thinks it is because of "Force a right-spline graph" in MinINode::Ideal This transforms MinI1( MinI2(a,b), c) into MinI1( a, MinI2(b,c) ) But that then destroys the ordering we expect in SuperWord for reductions. We need to figure out if the optimization in MinINode::Ideal has a good reason. Maybe we can also improve the detection in SuperWord, and allow this reordering to be re-reordered there. We are discussing this because of JDK-8302652, https://github.com/openjdk/jdk/pull/13056 What are your plans with this [~jbhateja] ?
24-03-2023

As discussed in email, I assign this to you, [~jbhateja]
17-02-2023