RFR: 8294715: Add IR checks to the reduction vectorization tests [v4]

Roberto Castañeda Lozano rcastanedalo at openjdk.org
Wed Mar 8 12:43:09 UTC 2023


On Wed, 8 Mar 2023 08:19:05 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.
>
> Daniel Skantz 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 14 additional commits since the last revision:
> 
>  - Merge branch 'master' of github.com:openjdk/jdk into JDK-8294715-IR-new
>  - Correctly reset totals in RedTest*; put debug print msgs in exception
>  - Remove 2-unroll scenario
>  - remove duped unrolllimit
>  - fix typo; remove non-store case from SumRedSqrt_Double due to slow run time
>  - Merge branch 'master' of github.com:openjdk/jdk into JDK-8294715-IR-new
>  - Remove print statements for prints that were silenced by IR framework addition
>  - Remove non-double stores
>  - Revert much of last commit, and part of the first commit addressing review comments : intention is to remove all the negative tests, except for on -XX:-SuperWordReductions. Keep some comments and additional IR nodes added to existing checks.
>  - Address further review comments (edits)
>  - ... and 4 more: https://git.openjdk.org/jdk/compare/c7819e4d...ec02160d

Looks good. Thanks again for the thorough and meticulous work, Daniel!

All `@IR` checks are either trivially negative (`@IR(applyIf = {"SuperWordReductions", "false"}, failOn = ...)`) or guarded by x86-specific features, so these changes should not cause false failures for non-x86 architectures anymore.

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

Marked as reviewed by rcastanedalo (Reviewer).

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


More information about the hotspot-compiler-dev mailing list