| Message ID | CAKjxQH=MD=5Z-XTpM4N8BYkW8wiKKQyiScUBHz4fy72hCq8nCw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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
I am wrong, I lost the sta16() in mips dsp manual.
Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (revision 236553) +++ gcc/testsuite/ChangeLog (working copy) @@ -1,3 +1,7 @@ +2016-05-21 Chenghua Xu <paul.hua.gm@gmail.com> + + * gcc.target/mips/mips32-dsp-run.c: Fix mistake. + 2016-05-20 Martin Sebor <msebor@redhat.com> PR c/71115 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; @@ -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 (); @@ -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 ();
Hi, There are some mistakes in mips dsp testsuite. This patch fixing it. Ok to commit? [mips] Fix mips dsp testsuite mistake. gcc/testsuite/gcc.target/mips/ *mips32-dsp-run.c: Fix mistake. Regards, Chenghua