RFR: 8289604: compiler/vectorapi/VectorLogicalOpIdentityTest.java failed on x86 AVX1 system

[Christian Hagedorn]

On Tue, 5 Jul 2022 07:47:58 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

> The sub-test "`testMaskAndZero()`" failed on x86 systems when
> `UseAVX=1` with the IR check failure:
> 
>   - counts: Graph contains wrong number of nodes:
>      * Regex 1: (\\d+(\\s){2}(StoreVector.*)+(\\s){2}===.*)
>         - Failed comparison: [found] 0 >= 1 [given]
>         - No nodes matched!
> 
> The root cause is the `VectorMask.fromArray/intoArray` APIs
> are not intrinsified when "`UseAVX=1`" for long type vectors
> with following reasons:
>  1) The system supported max vector size is 128 bits for
> integral vector operations when "`UseAVX=1`".
>  2) The match rule of `VectorLoadMaskNode/VectorStoreMaskNode`
> are not supported for vectors with 2 elements (see [1]).
> 
> Note that `VectorMask.fromArray()` needs to be intrinsified
> with "`LoadVector+VectorLoadMask`". And `VectorMask.intoArray()`
> needs to be intrinsified with "`VectorStoreMask+StoreVector`".
> Either "`VectorStoreMask`" or "`StoreVector`" not supported by the
> compiler backend will forbit the relative API intrinsification.
> 
> Replacing the vector type from Long to other integral types
> in the test case can fix the issue.
> 
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86.ad#L1861

> Including @chhagedorn for discussion.
> 
> I was wondering why our testing with `UseAVX=1` did not catch this issue (test passed). But then I remember that IR framework skip testing if such flag is used by testing. It is not on whitelist: https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/compiler/lib/ir_framework/TestFramework.java#L104
> 
> We are adding more IR framework testing for vectors. I think it is important to test them with different CPU's features **if needed**.
> 
> #8999 added filter to run some sub-tests if CPU's feature is present/absent. But it relies on testing infrastructure to be executed on corresponding machine. It is not reliable (how many machines left with only AVX1).
> 
> I suggest to allow a test add flags to whitelist with which it allows to run. It (together with #8999 and `@requires`) will allow test's author to specify range of CPU features which can be used for test and make sure they will be run.

Right, `UseAVX` is not a whitelisted flag and thus IR matching is disabled. I think it would be a good idea to consider whitelisting this flag in general to allow (pre-integration) testing of such different default machine setups (we could be executing any test on such a machine at some point). Same for other flags such as `UseSVE`, `UseSSE` etc. I've filed [JDK-8289801](https://bugs.openjdk.org/browse/JDK-8289801) to follow up on that. I will be on vacation starting from tomorrow. If someone wants to take this over, please go ahead. Otherwise, I'll come back to this when I'm back at the beginning of August.

Thanks,
Christian

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

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