RFR: 8294715: Add IR checks to the reduction vectorization tests

Roberto Castañeda Lozano rcastanedalo at openjdk.org
Wed Feb 22 11:31:10 UTC 2023


On Tue, 21 Feb 2023 08:08:31 GMT, Daniel Skantz <duke at openjdk.org> wrote:

> We are lifting some loopopts/superword tests to use the IR framework, and add IR annotations to check that vector reductions take place on x86_64. This can be useful to prevent issues such as JDK-8300865.
> 
> Approach: lift the more general tests in loopopts/superword, mainly using matching rules in cpu/x86/x86.ad, but leave tests mostly unchanged otherwise. Some reductions are considered non-profitable (superword.cpp), so we might need to raise sse/avx value pre-conditions from what would be a strict reading of x86.ad (as noted by @eme64).
> 
> Testing: Local testing (x86_64) using UseSSE={2,3,4}, UseAVX={0,1,2,3}. Tested running all jtreg compiler tests. Tier1-tier5 runs to my knowledge never showed any compiler-related regression in other tests as a result from this work. GHA. Validation: all tests fail if we put unreasonable counts for the respective reduction node, such as counts = {IRNode.ADD_REDUCTION_VI, ">= 10000000"}).
> 
> Thanks @robcasloz  and @eme64 for advice.
> 
> Notes: ProdRed_Double does not vectorize (JDK-8300865). SumRed_Long does not vectorize on 32-bit, according to my reading of source, test on GHA and cross-compiled JDK on 32-bit Linux, so removed these platforms from @requires. Lifted the AbsNeg tests too but added no checks, as these are currently not run on x86_64.

Thanks for working on this, Daniel! I have a few style comments. I think it would be good if someone from Intel could review this PR (@sviswa7, @jatin-bhateja, @smita-kamath ?).

test/hotspot/jtreg/compiler/loopopts/superword/ProdRed_Double.java line 57:

> 55: 
> 56:     @Run(test = {"prodReductionImplement"},
> 57:         mode = RunMode.STANDALONE)

Wrong indentation (applies to the same line in other files as well).

test/hotspot/jtreg/compiler/loopopts/superword/ProdRed_Double.java line 84:

> 82:     }
> 83: 
> 84:     //8300865 : No reduction nodes emitted (x86_64)

We try to avoid references to issue numbers in the code since they do not tend to age well (an exception is of course the top-level `@bug` annotation). I think it is valuable to have a comment explaining why you do not currently expect any vectorization in this case, but could you rewrite it without the explicit issue reference?

test/hotspot/jtreg/compiler/loopopts/superword/RedTest_long.java line 48:

> 46:         Scenario[] scenarios = new Scenario[8];
> 47:         for (int maxUnroll : new int[] {2, 4, 8, 16}) {
> 48:           scenarios[i] = new Scenario(i, "-XX:+SuperWordReductions",

Indentation of Java code is inconsistent across the changeset. Please make it consistent with the existing style in the test files (four whitespaces).

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

Changes requested by rcastanedalo (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12683


More information about the hotspot-compiler-dev mailing list