[PATCH][Testsuite] Fix mips dsp testsuite mistakes

Maciej W. Rozycki macro@imgtec.com
Sat May 21 07:05:00 GMT 2016


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



More information about the Gcc-patches mailing list