On Sat, 21 May 2016, Paul Hua wrote:
> There are some mistakes in mips dsp testsuite.
>
> This patch fixing it.
Thank you for your contribution, however you need to be more explicit
with patch descriptions, and explain in detail what problem your change
addresses, in this case what mistakes you have corrected. For example:
"Code does this and that and this is wrong, because... Correct it by
doing this and that instead."
Please see the individual questions below.
Also please don't make your ChangeLog entry a part of the patch submitted
as it makes it difficult for the committer to apply the patch, because
ChangeLog changes constantly. Instead include it in the e-mail body and
the committer will prepend it to ChangeLog at the committment time.
> Index: gcc/testsuite/gcc.target/mips/mips32-dsp-run.c
> ===================================================================
> --- gcc/testsuite/gcc.target/mips/mips32-dsp-run.c (revision 236553)
> +++ gcc/testsuite/gcc.target/mips/mips32-dsp-run.c (working copy)
> @@ -394,7 +394,7 @@ NOMIPS16 void test_MIPS_DSP ()
>
> v2q15_a = (v2q15) {0x1234, 0x5678};
> i32_b = 1;
> - v2q15_s = (v2q15) {0x2468, 0x7fff};
> + v2q15_s = (v2q15) {0x2468, 0xacf0};
> v2q15_r = __builtin_mips_shll_s_ph (v2q15_a, i32_b);
> r = (int) v2q15_r;
> s = (int) v2q15_s;
The shift operation requested results in a signed integer overflow and
consequently saturation triggers. Why do you think the original result
expected is wrong?
> @@ -409,7 +409,7 @@ NOMIPS16 void test_MIPS_DSP ()
>
> q31_a = 0x70000000;
> i32_b = 1;
> - q31_s = 0x7fffffff;
> + q31_s = 0xe0000000;
> q31_r = __builtin_mips_shll_s_w (q31_a, i32_b);
> if (q31_r != q31_s)
> abort ();
Likewise, same question as above.
> @@ -961,9 +961,9 @@ NOMIPS16 void test_MIPS_DSP ()
> abort ();
> #endif
>
> - i32_a = 0x1357a468;
> + i32_a = 0x13572468;
> __builtin_mips_wrdsp (i32_a, 63);
> - i32_s = 0x03572428;
> + i32_s = 0x13572468;
> i32_r = __builtin_mips_rddsp (63);
> if (i32_r != i32_s)
> abort ();
This undoubtedly verifies that reserved bits read back as zeros, so it
does not look like a mistake to me. How did you verify your change?
Maciej
