RFR: 8256614: AArch64: Add SVE backend implementation for integer min/max

[Andrew Dinn]

On Fri, 20 Nov 2020 10:19:40 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>>> We need to add them all to avoid the "bad AD file" crash.
>> 
>> I'm not clear how this came about. Is this fix needed as a response to another change that caused the crash? Or is the problem that the incomplete implementation was pushed without testing some cases properly?
>> 
>> Is there a before and after test case for the problem this fixes?
>
>> > We need to add them all to avoid the "bad AD file" crash.
>> 
>> I'm not clear how this came about. Is this fix needed as a response to another change that caused the crash? Or is the problem that the incomplete implementation was pushed without testing some cases properly?
>> 
>> Is there a before and after test case for the problem this fixes?
> 
> Hi @adinn , thanks for looking at this PR. Currently only the VectorAPI can generate the `MinVNode/MaxVNode` for integer types. I found this issue when I tried to use the related API to do some other investigation work. And actually there is the jtreg tests (eg: https://github.com/openjdk/jdk/blob/master/test/jdk/jdk/incubator/vector/IntMaxVectorTests.java#L2403) for it. I think the reason that we didn't find the issue is the loop count `INVOC_COUNT` , which is too small that didn't make the method hot enough to be compiled with C2. Currently the value is set to 100:
> static final int INVOC_COUNT = Integer.getInteger("jdk.incubator.vector.test.loop-iterations", 100);
> When I use a larger loop count like 10000, the test can crash with `bad AD file`.  So this issue can be reproduced by adding `-Djdk.incubator.vector.test.loop-iterations=10000` when running the tests.

Hi @XiaohongGong . Thanks for clarifying that. Your code changes look ok to me but it would be good if we could also change the default setting for this loop count to ensure that this case gets tested when SVE hw is present.

iterations=100 appears to be defaulted in file config.sh in the same dir as the test program. Do you know if using iterations=100 is actually triggering C2 compilation for any case (SVE or other, including on Intel)? If not then we really need to increase the default count to a higher value for all cases.

If this is just an SVE-specific thing then we could maybe add some special case processing to the config script to detect an arch where SVE is present and set a higher value.

Perhaps @iwanowww might be able to comment on what would be a suitable setting for this counter.

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

PR: https://git.openjdk.java.net/jdk/pull/1337