| Message ID | 20170309165252.29568.7508.stgit@brimstone.rchland.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Mar 09, 2017 at 10:52:52AM -0600, Will Schmidt wrote: > Per PR79941, we are folding the vec_mul{e,o}* operations improperly. Those > entries were added to the intrinsics-to-be-folded list where the generic > multiplies should have been instead. Test coverage in place was for the > generic multiplies, and this was missed by my testing. > > The mul[eo]* unsigned operations were missing entries in builtin_function_type() > to indicate the overloaded arguments were unsigned. This is corrected here, > and those operations now fold accurately to the desired instruction. This looks good to me, but I'll defer the actual review to PowerPC maintainers. Perhaps there was some hidden reason (xlC compatibility, whatever) that said that vmuleub etc. should have signed vector arguments and result. Also, I'd like to understand what those ALTIVEC_BUILTIN_VMULEUH_UNS etc. codes are for (the builtin doesn't seem to be user accessible). Jakub
On Thu, Mar 09, 2017 at 05:59:39PM +0100, Jakub Jelinek wrote: > On Thu, Mar 09, 2017 at 10:52:52AM -0600, Will Schmidt wrote: > > Per PR79941, we are folding the vec_mul{e,o}* operations improperly. Those > > entries were added to the intrinsics-to-be-folded list where the generic > > multiplies should have been instead. Test coverage in place was for the > > generic multiplies, and this was missed by my testing. > > > > The mul[eo]* unsigned operations were missing entries in builtin_function_type() > > to indicate the overloaded arguments were unsigned. This is corrected here, > > and those operations now fold accurately to the desired instruction. > > This looks good to me, but I'll defer the actual review to PowerPC > maintainers. Perhaps there was some hidden reason (xlC compatibility, > whatever) that said that vmuleub etc. should have signed vector arguments > and result. > > Also, I'd like to understand what those ALTIVEC_BUILTIN_VMULEUH_UNS etc. > codes are for (the builtin doesn't seem to be user accessible). It used to be, but that was removed when mult-even was removed (which seems to be the only thing it was used for). Mike, do you remember? Segher
Hi Will, On Thu, Mar 09, 2017 at 10:52:52AM -0600, Will Schmidt wrote: > Per PR79941, we are folding the vec_mul{e,o}* operations improperly. Those > entries were added to the intrinsics-to-be-folded list where the generic > multiplies should have been instead. Test coverage in place was for the > generic multiplies, and this was missed by my testing. > > The mul[eo]* unsigned operations were missing entries in builtin_function_type() > to indicate the overloaded arguments were unsigned. This is corrected here, > and those operations now fold accurately to the desired instruction. > > Testcases have been added for the mule/mulo operations, as well as a dg-do > run test based on the testcase in the PR. I'll note that the variables in that > test case are now marked as volatile so they are not entirely optimized out during > the compile. > > OK for trunk? > regtest is currently running on ppc64*. > PR target/79941 > * config/rs6000/rs6000.c (builtin_function_type): Add VMUL*UH > entries to the case statement that marks unsigned arguments to > overloaded functions. UH and UB? > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-char.c > @@ -0,0 +1,38 @@ > +/* Verify that overloaded built-ins for vec_mule,vec_mulo with char > + inputs produce the right results. */ > + > +/* { dg-do compile } */ > +/* { dg-require-effective-target powerpc_altivec_ok } */ > +/* { dg-options "-maltivec -O2 " } */ ^ this space looks off > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c > @@ -0,0 +1,61 @@ > +/* PR target/79941 */ > + > +/* { dg-do run } */ > +/* { dg-require-effective-target powerpc_vsx_ok } */ > +/* { dg-options "-mvsx -O2 -save-temps" } */ I think that -save-temps is a leftover from testing? It shouldn't be there? > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-short.c > @@ -0,0 +1,37 @@ > +/* Verify that overloaded built-ins for vec_mule,vec_mulo with short > + inputs produce the right results. */ > + > +/* { dg-do compile } */ > +/* { dg-require-effective-target powerpc_altivec_ok } */ > +/* { dg-options "-maltivec -O2 " } */ ^ another Okay with those nits fixed, but let's hear if Mike remembers anything first. Thanks, Segher
On Thu, Mar 09, 2017 at 05:15:40PM -0600, Segher Boessenkool wrote: > On Thu, Mar 09, 2017 at 05:59:39PM +0100, Jakub Jelinek wrote: > > On Thu, Mar 09, 2017 at 10:52:52AM -0600, Will Schmidt wrote: > > > Per PR79941, we are folding the vec_mul{e,o}* operations improperly. Those > > > entries were added to the intrinsics-to-be-folded list where the generic > > > multiplies should have been instead. Test coverage in place was for the > > > generic multiplies, and this was missed by my testing. > > > > > > The mul[eo]* unsigned operations were missing entries in builtin_function_type() > > > to indicate the overloaded arguments were unsigned. This is corrected here, > > > and those operations now fold accurately to the desired instruction. > > > > This looks good to me, but I'll defer the actual review to PowerPC > > maintainers. Perhaps there was some hidden reason (xlC compatibility, > > whatever) that said that vmuleub etc. should have signed vector arguments > > and result. > > > > Also, I'd like to understand what those ALTIVEC_BUILTIN_VMULEUH_UNS etc. > > codes are for (the builtin doesn't seem to be user accessible). > > It used to be, but that was removed when mult-even was removed (which > seems to be the only thing it was used for). Mike, do you remember? I don't recall. Perhaps it is related to: 2016-12-19 Will Schmidt <will_schmidt@vnet.ibm.com> * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling for early expansion of vector multiply and subtract builtins.
On Thu, Mar 09, 2017 at 07:01:06PM -0500, Michael Meissner wrote: > > > This looks good to me, but I'll defer the actual review to PowerPC > > > maintainers. Perhaps there was some hidden reason (xlC compatibility, > > > whatever) that said that vmuleub etc. should have signed vector arguments > > > and result. > > > > > > Also, I'd like to understand what those ALTIVEC_BUILTIN_VMULEUH_UNS etc. > > > codes are for (the builtin doesn't seem to be user accessible). > > > > It used to be, but that was removed when mult-even was removed (which > > seems to be the only thing it was used for). Mike, do you remember? > > I don't recall. Perhaps it is related to: > > 2016-12-19 Will Schmidt <will_schmidt@vnet.ibm.com> > > * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling for > early expansion of vector multiply and subtract builtins. That added the folding. The questions are: 1) if it is intentional that ALTIVEC_BUILTIN_VMULEUH etc. used signed rather than unsigned vector types as arguments and return value (and if yes, why)? BU_ALTIVEC_2 (VMULEUH, "vmuleuh", CONST, vec_widen_umult_even_v8hi) BU_ALTIVEC_2 (VMULEUH_UNS, "vmuleuh_uns", CONST, vec_widen_umult_even_v8hi) and builtin_function_type only mentioning /* unsigned 2 argument functions. */ case ALTIVEC_BUILTIN_VMULEUH_UNS: Back e.g. in 4.6 UNS was used in targetm.vectorize.builtin_mul_widen_even. Does the Altivec spec say that that vec_vmuleuh arguments should be vector signed short? For vec_mule it chooses {uh,ub,sh,sb} depending on whether the arguments are signed/unsigned and short/char vectors. 2) can we now remove ALTIVEC_BUILTIN_VMULEUH_UNS etc.? Jakub
On Thu, 2017-03-09 at 17:24 -0600, Segher Boessenkool wrote: > Hi Will, > > On Thu, Mar 09, 2017 at 10:52:52AM -0600, Will Schmidt wrote: > > Per PR79941, we are folding the vec_mul{e,o}* operations improperly. Those > > entries were added to the intrinsics-to-be-folded list where the generic > > multiplies should have been instead. Test coverage in place was for the > > generic multiplies, and this was missed by my testing. > > > > The mul[eo]* unsigned operations were missing entries in builtin_function_type() > > to indicate the overloaded arguments were unsigned. This is corrected here, > > and those operations now fold accurately to the desired instruction. > > > > Testcases have been added for the mule/mulo operations, as well as a dg-do > > run test based on the testcase in the PR. I'll note that the variables in that > > test case are now marked as volatile so they are not entirely optimized out during > > the compile. > > > > OK for trunk? > > regtest is currently running on ppc64*. > > > > > PR target/79941 > > * config/rs6000/rs6000.c (builtin_function_type): Add VMUL*UH > > entries to the case statement that marks unsigned arguments to > > overloaded functions. > > UH and UB? Yes. I've updated that to read "Add VMUL*U[HB]" in my local copy. > > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-char.c > > @@ -0,0 +1,38 @@ > > +/* Verify that overloaded built-ins for vec_mule,vec_mulo with char > > + inputs produce the right results. */ > > + > > +/* { dg-do compile } */ > > +/* { dg-require-effective-target powerpc_altivec_ok } */ > > +/* { dg-options "-maltivec -O2 " } */ > > ^ this space looks off It is. fixed locally in both *mule-char.c and *mule-short.c testcases. > > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c > > @@ -0,0 +1,61 @@ > > +/* PR target/79941 */ > > + > > +/* { dg-do run } */ > > +/* { dg-require-effective-target powerpc_vsx_ok } */ > > +/* { dg-options "-mvsx -O2 -save-temps" } */ > > I think that -save-temps is a leftover from testing? It shouldn't be there? Because this is a "dg-do run", and I am also doing a scan-assembler stanza at the end, I needed that to preserve the intermediate. I can rework things if this is not considered proper.. :-) > > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-short.c > > @@ -0,0 +1,37 @@ > > +/* Verify that overloaded built-ins for vec_mule,vec_mulo with short > > + inputs produce the right results. */ > > + > > +/* { dg-do compile } */ > > +/* { dg-require-effective-target powerpc_altivec_ok } */ > > +/* { dg-options "-maltivec -O2 " } */ > > ^ another yup, got it updated locally now, thanks. > > Okay with those nits fixed, but let's hear if Mike remembers anything first. yup. > > Thanks, > > > Segher >
Jumping in so we can continue the Will/Bill confusion: ;) The official prototypes from the original AltiVec PIM are: vector unsigned short vec_mule (vector unsigned char, vector unsigned char); vector signed short vec_mule (vector signed char, vector signed char); vector unsigned int vec_mule (vector unsigned short, vector unsigned short); vector signed int vec_mule (vector signed short, vector signed short); These are still the only supported forms. For POWER9, we are adding similar prototypes for 32-bit multiplies with a 64-bit result. I do not know why we have this _UNS variant, but it seems to be something we can do without. It does not appear in the built-in table in rs6000-c.c All the entries in that built-in table are type-correct with respect to the above definitions. We should delete that entry from rs6000-builtin.def. -- Bill Bill Schmidt, Ph.D. GCC for Linux on Power Linux on Power Toolchain IBM Linux Technology Center wschmidt@linux.vnet.ibm.com > On Mar 9, 2017, at 6:15 PM, Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Mar 09, 2017 at 07:01:06PM -0500, Michael Meissner wrote: >>>> This looks good to me, but I'll defer the actual review to PowerPC >>>> maintainers. Perhaps there was some hidden reason (xlC compatibility, >>>> whatever) that said that vmuleub etc. should have signed vector arguments >>>> and result. >>>> >>>> Also, I'd like to understand what those ALTIVEC_BUILTIN_VMULEUH_UNS etc. >>>> codes are for (the builtin doesn't seem to be user accessible). >>> >>> It used to be, but that was removed when mult-even was removed (which >>> seems to be the only thing it was used for). Mike, do you remember? >> >> I don't recall. Perhaps it is related to: >> >> 2016-12-19 Will Schmidt <will_schmidt@vnet.ibm.com> >> >> * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling for >> early expansion of vector multiply and subtract builtins. > > That added the folding. The questions are: > 1) if it is intentional that ALTIVEC_BUILTIN_VMULEUH etc. used signed rather > than unsigned vector types as arguments and return value (and if yes, why)? > BU_ALTIVEC_2 (VMULEUH, "vmuleuh", CONST, vec_widen_umult_even_v8hi) > BU_ALTIVEC_2 (VMULEUH_UNS, "vmuleuh_uns", CONST, vec_widen_umult_even_v8hi) > and builtin_function_type only mentioning > /* unsigned 2 argument functions. */ > case ALTIVEC_BUILTIN_VMULEUH_UNS: > Back e.g. in 4.6 UNS was used in targetm.vectorize.builtin_mul_widen_even. > Does the Altivec spec say that that vec_vmuleuh arguments should be > vector signed short? For vec_mule it chooses {uh,ub,sh,sb} depending on > whether the arguments are signed/unsigned and short/char vectors. > 2) can we now remove ALTIVEC_BUILTIN_VMULEUH_UNS etc.? > > Jakub >
On Fri, Mar 10, 2017 at 08:08:06AM -0600, Will Schmidt wrote: > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c > > > @@ -0,0 +1,61 @@ > > > +/* PR target/79941 */ > > > + > > > +/* { dg-do run } */ > > > +/* { dg-require-effective-target powerpc_vsx_ok } */ > > > +/* { dg-options "-mvsx -O2 -save-temps" } */ > > > > I think that -save-temps is a leftover from testing? It shouldn't be there? > > Because this is a "dg-do run", and I am also doing a scan-assembler > stanza at the end, I needed that to preserve the intermediate. I can > rework things if this is not considered proper.. :-) Oh I missed that. No, it is fine then. Segher
Just so there's no duplication of effort -- I'll follow up with a patch to remove the outdated built-in. -- Bill Bill Schmidt, Ph.D. GCC for Linux on Power Linux on Power Toolchain IBM Linux Technology Center wschmidt@linux.vnet.ibm.com > On Mar 10, 2017, at 8:24 AM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote: > > Jumping in so we can continue the Will/Bill confusion: ;) > > The official prototypes from the original AltiVec PIM are: > > vector unsigned short vec_mule (vector unsigned char, vector unsigned char); > vector signed short vec_mule (vector signed char, vector signed char); > > vector unsigned int vec_mule (vector unsigned short, vector unsigned short); > vector signed int vec_mule (vector signed short, vector signed short); > > These are still the only supported forms. For POWER9, we are adding similar > prototypes for 32-bit multiplies with a 64-bit result. > > I do not know why we have this _UNS variant, but it seems to be something > we can do without. It does not appear in the built-in table in rs6000-c.c All > the entries in that built-in table are type-correct with respect to the above > definitions. We should delete that entry from rs6000-builtin.def. > > -- Bill > > Bill Schmidt, Ph.D. > GCC for Linux on Power > Linux on Power Toolchain > IBM Linux Technology Center > wschmidt@linux.vnet.ibm.com > >> On Mar 9, 2017, at 6:15 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> >> On Thu, Mar 09, 2017 at 07:01:06PM -0500, Michael Meissner wrote: >>>>> This looks good to me, but I'll defer the actual review to PowerPC >>>>> maintainers. Perhaps there was some hidden reason (xlC compatibility, >>>>> whatever) that said that vmuleub etc. should have signed vector arguments >>>>> and result. >>>>> >>>>> Also, I'd like to understand what those ALTIVEC_BUILTIN_VMULEUH_UNS etc. >>>>> codes are for (the builtin doesn't seem to be user accessible). >>>> >>>> It used to be, but that was removed when mult-even was removed (which >>>> seems to be the only thing it was used for). Mike, do you remember? >>> >>> I don't recall. Perhaps it is related to: >>> >>> 2016-12-19 Will Schmidt <will_schmidt@vnet.ibm.com> >>> >>> * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling for >>> early expansion of vector multiply and subtract builtins. >> >> That added the folding. The questions are: >> 1) if it is intentional that ALTIVEC_BUILTIN_VMULEUH etc. used signed rather >> than unsigned vector types as arguments and return value (and if yes, why)? >> BU_ALTIVEC_2 (VMULEUH, "vmuleuh", CONST, vec_widen_umult_even_v8hi) >> BU_ALTIVEC_2 (VMULEUH_UNS, "vmuleuh_uns", CONST, vec_widen_umult_even_v8hi) >> and builtin_function_type only mentioning >> /* unsigned 2 argument functions. */ >> case ALTIVEC_BUILTIN_VMULEUH_UNS: >> Back e.g. in 4.6 UNS was used in targetm.vectorize.builtin_mul_widen_even. >> Does the Altivec spec say that that vec_vmuleuh arguments should be >> vector signed short? For vec_mule it chooses {uh,ub,sh,sb} depending on >> whether the arguments are signed/unsigned and short/char vectors. >> 2) can we now remove ALTIVEC_BUILTIN_VMULEUH_UNS etc.? >> >> Jakub >> >
On Mär 09 2017, Will Schmidt <will_schmidt@vnet.ibm.com> wrote: > diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c > new file mode 100644 > index 0000000..4bb6185 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c > @@ -0,0 +1,61 @@ > +/* PR target/79941 */ > + > +/* { dg-do run } */ > +/* { dg-require-effective-target powerpc_vsx_ok } */ > +/* { dg-options "-mvsx -O2 -save-temps" } */ FAIL: gcc.target/powerpc/fold-vec-mule-misc.c execution test $ ./fold-vec-mule-misc.exe Illegal instruction Andreas.
On Sun, Apr 02, 2017 at 09:26:24AM +0200, Andreas Schwab wrote: > > diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c > > new file mode 100644 > > index 0000000..4bb6185 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c > > @@ -0,0 +1,61 @@ > > +/* PR target/79941 */ > > + > > +/* { dg-do run } */ > > +/* { dg-require-effective-target powerpc_vsx_ok } */ > > +/* { dg-options "-mvsx -O2 -save-temps" } */ > > FAIL: gcc.target/powerpc/fold-vec-mule-misc.c execution test > > $ ./fold-vec-mule-misc.exe > Illegal instruction It should test vsx_hw, not just vsx_ok. I'll handle it (and the other problem you found). Thanks, and sorry for not spotting the problems! Segher
On Sun, Apr 02, 2017 at 09:26:24AM +0200, Andreas Schwab wrote: > > +/* PR target/79941 */ > > + > > +/* { dg-do run } */ > > +/* { dg-require-effective-target powerpc_vsx_ok } */ > > +/* { dg-options "-mvsx -O2 -save-temps" } */ > > FAIL: gcc.target/powerpc/fold-vec-mule-misc.c execution test > > $ ./fold-vec-mule-misc.exe > Illegal instruction I cannot get this one to fail, unless I explicitly use -mcpu=power8 or similar (which will not run on a power7, direct move insns do not yet exist on power7). What is different about your setup? (What *is* your setup?) Segher
On Apr 02 2017, Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Sun, Apr 02, 2017 at 09:26:24AM +0200, Andreas Schwab wrote: >> > +/* PR target/79941 */ >> > + >> > +/* { dg-do run } */ >> > +/* { dg-require-effective-target powerpc_vsx_ok } */ >> > +/* { dg-options "-mvsx -O2 -save-temps" } */ >> >> FAIL: gcc.target/powerpc/fold-vec-mule-misc.c execution test >> >> $ ./fold-vec-mule-misc.exe >> Illegal instruction > > I cannot get this one to fail, unless I explicitly use -mcpu=power8 or > similar (which will not run on a power7, direct move insns do not yet > exist on power7). > > What is different about your setup? (What *is* your setup?) http://gcc.gnu.org/ml/gcc-testresults/2017-04/msg00126.html Andreas.
On Sun, Apr 02, 2017 at 03:45:05PM +0200, Andreas Schwab wrote: > > On Sun, Apr 02, 2017 at 09:26:24AM +0200, Andreas Schwab wrote: > >> > +/* PR target/79941 */ > >> > + > >> > +/* { dg-do run } */ > >> > +/* { dg-require-effective-target powerpc_vsx_ok } */ > >> > +/* { dg-options "-mvsx -O2 -save-temps" } */ > >> > >> FAIL: gcc.target/powerpc/fold-vec-mule-misc.c execution test > >> > >> $ ./fold-vec-mule-misc.exe > >> Illegal instruction > > > > I cannot get this one to fail, unless I explicitly use -mcpu=power8 or > > similar (which will not run on a power7, direct move insns do not yet > > exist on power7). > > > > What is different about your setup? (What *is* your setup?) > > http://gcc.gnu.org/ml/gcc-testresults/2017-04/msg00126.html That only says it is powerpc64-linux. I don't see these problems there (or anywhere else). The testcase should not run on anything older than a Power7. Is your system something older? Why does powerpc_vsx_ok return true then, ugh. Segher
On Apr 02 2017, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> Why does powerpc_vsx_ok return true then, ugh.
Because the assembler is new enough. That's all it checks.
Andreas.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 25b10f1..42109f5 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -18530,6 +18530,10 @@ builtin_function_type (machine_mode mode_ret, machine_mode mode_arg0, case ALTIVEC_BUILTIN_VMULEUH_UNS: case ALTIVEC_BUILTIN_VMULOUB_UNS: case ALTIVEC_BUILTIN_VMULOUH_UNS: + case ALTIVEC_BUILTIN_VMULEUB: + case ALTIVEC_BUILTIN_VMULEUH: + case ALTIVEC_BUILTIN_VMULOUB: + case ALTIVEC_BUILTIN_VMULOUH: case CRYPTO_BUILTIN_VCIPHER: case CRYPTO_BUILTIN_VCIPHERLAST: case CRYPTO_BUILTIN_VNCIPHER: diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-char.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-char.c new file mode 100644 index 0000000..88cd6cf --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-char.c @@ -0,0 +1,38 @@ +/* Verify that overloaded built-ins for vec_mule,vec_mulo with char + inputs produce the right results. */ + +/* { dg-do compile } */ +/* { dg-require-effective-target powerpc_altivec_ok } */ +/* { dg-options "-maltivec -O2 " } */ + +#include <altivec.h> + +vector signed short +test_even (vector signed char x, vector signed char y) +{ + return vec_mule (x, y); +} + +vector unsigned short +test_uns_even (vector unsigned char x, vector unsigned char y) +{ + return vec_mule (x, y); +} + +vector signed short +test_odd (vector signed char x, vector signed char y) +{ + return vec_mulo (x, y); +} + +vector unsigned short +test_uns_odd (vector unsigned char x, vector unsigned char y) +{ + return vec_mulo (x, y); +} + +/* { dg-final { scan-assembler-times "vmuleub" 1 } } */ +/* { dg-final { scan-assembler-times "vmulesb" 1 } } */ +/* { dg-final { scan-assembler-times "vmuloub" 1 } } */ +/* { dg-final { scan-assembler-times "vmulosb" 1 } } */ + diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c new file mode 100644 index 0000000..4bb6185 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c @@ -0,0 +1,61 @@ +/* PR target/79941 */ + +/* { dg-do run } */ +/* { dg-require-effective-target powerpc_vsx_ok } */ +/* { dg-options "-mvsx -O2 -save-temps" } */ + +#include <altivec.h> + +__attribute__((noinline)) void +test_eub_char () +{ + volatile vector unsigned char v0 = {1, 0, 0, 0, 0, 0, 0, 0}; + volatile vector unsigned char v1 = {0xff, 0, 0, 0, 0, 0, 0, 0}; + vector unsigned short res = vec_vmuleub (v0, v1); + if (res[0] != (unsigned short)v0[0] * (unsigned short)v1[0]) + __builtin_abort (); +} + +__attribute__((noinline)) void +test_oub_char () +{ + volatile vector unsigned char v0 = {0, 1, 0, 0, 0, 0, 0, 0}; + volatile vector unsigned char v1 = {0, 0xff, 0, 0, 0, 0, 0, 0}; + vector unsigned short res = vec_vmuloub (v0, v1); + if (res[0] != (unsigned short)v0[1] * (unsigned short)v1[1]) + __builtin_abort (); +} + +__attribute__((noinline)) void +test_euh_short () +{ + volatile vector unsigned short v0 = {1, 0, 0, 0}; + volatile vector unsigned short v1 = {0xff, 0, 0, 0}; + vector unsigned int res = vec_vmuleuh (v0, v1); + if (res[0] != (unsigned int)v0[0] * (unsigned int)v1[0]) + __builtin_abort (); +} + +__attribute__((noinline)) void +test_ouh_short () +{ + volatile vector unsigned short v0 = {0, 1, 0, 0}; + volatile vector unsigned short v1 = {0, 0xff, 0, 0}; + vector unsigned int res = vec_vmulouh (v0, v1); + if (res[0] != (unsigned int)v0[1] * (unsigned int)v1[1]) + __builtin_abort (); +} + +int main () +{ + test_eub_char(); + test_oub_char(); + test_euh_short(); + test_ouh_short(); +} + +/* { dg-final { scan-assembler-times "vmuleub" 1 } } */ +/* { dg-final { scan-assembler-times "vmuloub" 1 } } */ +/* { dg-final { scan-assembler-times "vmuleuh" 1 } } */ +/* { dg-final { scan-assembler-times "vmulouh" 1 } } */ + diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-short.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-short.c new file mode 100644 index 0000000..8c856aa --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mule-short.c @@ -0,0 +1,37 @@ +/* Verify that overloaded built-ins for vec_mule,vec_mulo with short + inputs produce the right results. */ + +/* { dg-do compile } */ +/* { dg-require-effective-target powerpc_altivec_ok } */ +/* { dg-options "-maltivec -O2 " } */ + +#include <altivec.h> + +vector signed int +test_even (vector signed short x, vector signed short y) +{ + return vec_mule (x, y); +} + +vector unsigned int +test_uns_even (vector unsigned short x, vector unsigned short y) +{ + return vec_mule (x, y); +} + +vector signed int +test_odd (vector signed short x, vector signed short y) +{ + return vec_mulo (x, y); +} + +vector unsigned int +test_uns_odd (vector unsigned short x, vector unsigned short y) +{ + return vec_mulo (x, y); +} + +/* { dg-final { scan-assembler-times "vmuleuh" 1 } } */ +/* { dg-final { scan-assembler-times "vmulesh" 1 } } */ +/* { dg-final { scan-assembler-times "vmulouh" 1 } } */ +/* { dg-final { scan-assembler-times "vmulosh" 1 } } */
Hi, Per PR79941, we are folding the vec_mul{e,o}* operations improperly. Those entries were added to the intrinsics-to-be-folded list where the generic multiplies should have been instead. Test coverage in place was for the generic multiplies, and this was missed by my testing. The mul[eo]* unsigned operations were missing entries in builtin_function_type() to indicate the overloaded arguments were unsigned. This is corrected here, and those operations now fold accurately to the desired instruction. Testcases have been added for the mule/mulo operations, as well as a dg-do run test based on the testcase in the PR. I'll note that the variables in that test case are now marked as volatile so they are not entirely optimized out during the compile. OK for trunk? regtest is currently running on ppc64*. gcc: 2017-03-09 Will Schmidt <will_schmidt@vnet.ibm.com> PR target/79941 * config/rs6000/rs6000.c (builtin_function_type): Add VMUL*UH entries to the case statement that marks unsigned arguments to overloaded functions. testsuite: 2017-03-09 Will Schmidt <will_schmidt@vnet.ibm.com> PR target/79941 * gcc.target/powerpc/fold-vec-mult-even_odd_misc.c: New test. * gcc.target/powerpc/fold-vec-mult-even_odd_char.c: New test. * gcc.target/powerpc/fold-vec-mult-even_odd_short.c: New test. --- gcc/config/rs6000/rs6000.c | 4 + .../gcc.target/powerpc/fold-vec-mule-char.c | 38 ++++++++++++ .../gcc.target/powerpc/fold-vec-mule-misc.c | 61 ++++++++++++++++++++ .../gcc.target/powerpc/fold-vec-mule-short.c | 37 ++++++++++++ 4 files changed, 140 insertions(+) create mode 100644 gcc/testsuite/gcc.target/powerpc/fold-vec-mule-char.c create mode 100644 gcc/testsuite/gcc.target/powerpc/fold-vec-mule-misc.c create mode 100644 gcc/testsuite/gcc.target/powerpc/fold-vec-mule-short.c