[vector api] RFR: Fine-tune test cases for reduction operations

[Yang Zhang (Arm Technology China)]

Hi Vladimir

The following is an example on AArch64 platform. Without the patch, the test is passed . But it is failed with this patch.

I make a little change to this instruction:
instruct reduce_max8S(iRegINoSp dst, iRegIorL2I src1, vecX src2, vecX tmp, rFlagsReg cr) %{
  format %{ "smaxv  $tmp, T8H, $src2\n\t"
            "umov  $dst, $tmp, H, 0\n\t"----------------> smov should be used. because umov will recognize a negative as a great positive.
            "cmpw  $dst, $src1\n\t"
            "cselw $dst, $dst, $src1 gt\t max reduction8S" %}

%}
smov: Signed Move vector element to general-purpose register.
umov: Move vector element to general-purpose register.

The test is passed with original test. The error is missed, because src1 is always Short.MIN_VALUE, reduce_max8S could give the correct result of one vector.
test Short128VectorTests.maxAllShort128VectorTests(short[i * 5]): success

The test is failed with new test. The error is caught.
test Short128VectorTests.maxAllShort128VectorTests(short[i * 5]): failure
java.lang.AssertionError: Final result is incorrect! expected [32765] but found [-25541]

So that I think checking both the final and intermediate result is better.
I update the patch a little. Please check it.
http://cr.openjdk.java.net/~yzhang/vectorapi.test/webrev.01/

Regards,
Yang

-----Original Message-----
From: Vladimir Ivanov <vladimir.x.ivanov at oracle.com> 
Sent: Tuesday, February 12, 2019 9:37 AM
To: Yang Zhang (Arm Technology China) <Yang.Zhang at arm.com>; 'panama-dev at openjdk.java.net' <panama-dev at openjdk.java.net>
Subject: Re: [vector api] RFR: Fine-tune test cases for reduction operations

Though I like the changes you propose, I don't see how they improve test coverage. It implies C2 does the following transformation, but I don't think that's the case (yet):

   AddI i (AddReductionVI (ConI 0) v) => AddReductionVI i v

Am I missing something?

Best regards,
Vladimir Ivanov

On 11/02/2019 00:25, Yang Zhang (Arm Technology China) wrote:
> Hi
> 
> Currently, test cases of reduction operations only check the intermediate result (the result of one vector), not final result ( the result of one array). So that for following examples, src1 is always initial value without this patch.
> instruct rsadd2I_reduction_reg(rRegI dst, rRegI src1, vecD src2, vecD 
> tmp, vecD tmp2) %{  ----------------------X86 instruct 
> reduce_add2I(iRegINoSp dst, iRegIorL2I src1, vecD src2, iRegINoSp tmp, iRegINoSp tmp2) ------------------AArch64 To cover more cases, I add test cases that check the final result of reduction operations. Some indentation errors are also fixed.
> With this patch, all Vector API tests are passed.
> 
> Could you please help to review this patch?
> http://cr.openjdk.java.net/~yzhang/vectorapi.test/webrev.00/
> 
> Regards,
> Yang
>