| Message ID | 20170307213638.26542.70346.stgit@brimstone.rchland.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Mar 07, 2017 at 03:36:38PM -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. > > Thusly, remove those entries from the folding list. At the same time, I > am adding a testcase to provide some basic coverage for those ops. > > Functional gimple folding for those operations will be showing up at > a later time. > > OK for trunk? > regtest is currently running on ppc64*. > > > gcc: > 2017-03-07 Will Schmidt <will_schmidt@vnet.ibm.com> > > PR middle-end/79941 > * config/rs6000/rs6000.c (gimplify_init_constructor): Remove multiply > even and multiply odd (vmule/vmulo) intrinsics from the multiply folding > sequence. > > testsuite: > 2017-03-07 Will Schmidt <will_schmidt@vnet.ibm.com> > > PR middle-end/79941 > * gcc.target/powerpc/fold-vec-mult-even_odd_misc.c: New test. > --- > gcc/config/rs6000/rs6000.c | 31 ----------- > .../powerpc/fold-vec-mult-even_odd_misc.c | 58 ++++++++++++++++++++ > 2 files changed, 58 insertions(+), 31 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/powerpc/fold-vec-mult-even_odd_misc.c > > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 25b10f1..ce0ece5 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -16852,37 +16852,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) > gsi_replace (gsi, g, true); > return true; > } Doesn't the ALTIVEC_BUILTIN_VMUL[EO]S[BH] folding work properly already? Perhaps you could just comment out the VMUL[EO]U[BH] folding for now, or depending on whether there is S or U set uns flag and punt if TYPE_SIGN (TREE_TYPE (TREE_TYPE (arg0))) is not equal to that (similarly for arg1 and lhs). Or VIEW_CONVERT_EXPR it if not the right sign. Or tweak the builtin prototype for VMUL[EO]U[BH] so that it uses unsigned vector arguments and result. > - /* Even element flavors of vec_mul (signed). */ > - case ALTIVEC_BUILTIN_VMULESB: > - case ALTIVEC_BUILTIN_VMULESH: > - /* Even element flavors of vec_mul (unsigned). */ > - case ALTIVEC_BUILTIN_VMULEUB: > - case ALTIVEC_BUILTIN_VMULEUH: > - { > - arg0 = gimple_call_arg (stmt, 0); > - arg1 = gimple_call_arg (stmt, 1); > - lhs = gimple_call_lhs (stmt); > - gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_EVEN_EXPR, arg0, arg1); > - gimple_set_location (g, gimple_location (stmt)); > - gsi_replace (gsi, g, true); > - return true; > - } > - /* Odd element flavors of vec_mul (signed). */ > - case ALTIVEC_BUILTIN_VMULOSB: > - case ALTIVEC_BUILTIN_VMULOSH: > - /* Odd element flavors of vec_mul (unsigned). */ > - case ALTIVEC_BUILTIN_VMULOUB: > - case ALTIVEC_BUILTIN_VMULOUH: > - { > - arg0 = gimple_call_arg (stmt, 0); > - arg1 = gimple_call_arg (stmt, 1); > - lhs = gimple_call_lhs (stmt); > - gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_ODD_EXPR, arg0, arg1); > - gimple_set_location (g, gimple_location (stmt)); > - gsi_replace (gsi, g, true); > - return true; > - } > - > default: > break; > } Jakub
On Tue, 2017-03-07 at 22:52 +0100, Jakub Jelinek wrote: > On Tue, Mar 07, 2017 at 03:36:38PM -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. > > > > Thusly, remove those entries from the folding list. At the same time, I > > am adding a testcase to provide some basic coverage for those ops. > > > > Functional gimple folding for those operations will be showing up at > > a later time. > > > > OK for trunk? > > regtest is currently running on ppc64*. > > > > > > gcc: > > 2017-03-07 Will Schmidt <will_schmidt@vnet.ibm.com> > > > > PR middle-end/79941 > > * config/rs6000/rs6000.c (gimplify_init_constructor): Remove multiply > > even and multiply odd (vmule/vmulo) intrinsics from the multiply folding > > sequence. > > > > testsuite: > > 2017-03-07 Will Schmidt <will_schmidt@vnet.ibm.com> > > > > PR middle-end/79941 > > * gcc.target/powerpc/fold-vec-mult-even_odd_misc.c: New test. > > --- > > gcc/config/rs6000/rs6000.c | 31 ----------- > > .../powerpc/fold-vec-mult-even_odd_misc.c | 58 ++++++++++++++++++++ > > 2 files changed, 58 insertions(+), 31 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/powerpc/fold-vec-mult-even_odd_misc.c > > > > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > > index 25b10f1..ce0ece5 100644 > > --- a/gcc/config/rs6000/rs6000.c > > +++ b/gcc/config/rs6000/rs6000.c > > @@ -16852,37 +16852,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) > > gsi_replace (gsi, g, true); > > return true; > > } > Hi Jakub, > Doesn't the ALTIVEC_BUILTIN_VMUL[EO]S[BH] folding work properly already? Perhaps by accident. :-) I have yet to review the generated assembly for the mul[eo] code, so I don't have a good feel for the health of the generated code at the moment. And I freely admit that I am still learning this code, and the VMUL[EO]* bits were only upstreamed due to error on my part. If there is any time pressure at all, I would prefer reverting this chunk entirely. Doing those intrinsics properly is in my queue, but may or may not end up being during stage4. > Perhaps you could just comment out the VMUL[EO]U[BH] folding for now, > or depending on whether there is S or U set uns flag and punt if TYPE_SIGN (TREE_TYPE > (TREE_TYPE (arg0))) is not equal to that (similarly for arg1 and lhs). > Or VIEW_CONVERT_EXPR it if not the right sign. Or tweak the builtin > prototype for VMUL[EO]U[BH] so that it uses unsigned vector arguments and > result. Thanks for this analysis... this will help.. :-) Thanks, -Will > > > - /* Even element flavors of vec_mul (signed). */ > > - case ALTIVEC_BUILTIN_VMULESB: > > - case ALTIVEC_BUILTIN_VMULESH: > > - /* Even element flavors of vec_mul (unsigned). */ > > - case ALTIVEC_BUILTIN_VMULEUB: > > - case ALTIVEC_BUILTIN_VMULEUH: > > - { > > - arg0 = gimple_call_arg (stmt, 0); > > - arg1 = gimple_call_arg (stmt, 1); > > - lhs = gimple_call_lhs (stmt); > > - gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_EVEN_EXPR, arg0, arg1); > > - gimple_set_location (g, gimple_location (stmt)); > > - gsi_replace (gsi, g, true); > > - return true; > > - } > > - /* Odd element flavors of vec_mul (signed). */ > > - case ALTIVEC_BUILTIN_VMULOSB: > > - case ALTIVEC_BUILTIN_VMULOSH: > > - /* Odd element flavors of vec_mul (unsigned). */ > > - case ALTIVEC_BUILTIN_VMULOUB: > > - case ALTIVEC_BUILTIN_VMULOUH: > > - { > > - arg0 = gimple_call_arg (stmt, 0); > > - arg1 = gimple_call_arg (stmt, 1); > > - lhs = gimple_call_lhs (stmt); > > - gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_ODD_EXPR, arg0, arg1); > > - gimple_set_location (g, gimple_location (stmt)); > > - gsi_replace (gsi, g, true); > > - return true; > > - } > > - > > default: > > break; > > } > > Jakub >
On Tue, Mar 07, 2017 at 03:36:38PM -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. > > Thusly, remove those entries from the folding list. At the same time, I > am adding a testcase to provide some basic coverage for those ops. > > Functional gimple folding for those operations will be showing up at > a later time. > > OK for trunk? > regtest is currently running on ppc64*. Okay if testing passed. Thanks, Segher
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 25b10f1..ce0ece5 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -16852,37 +16852,6 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) gsi_replace (gsi, g, true); return true; } - /* Even element flavors of vec_mul (signed). */ - case ALTIVEC_BUILTIN_VMULESB: - case ALTIVEC_BUILTIN_VMULESH: - /* Even element flavors of vec_mul (unsigned). */ - case ALTIVEC_BUILTIN_VMULEUB: - case ALTIVEC_BUILTIN_VMULEUH: - { - arg0 = gimple_call_arg (stmt, 0); - arg1 = gimple_call_arg (stmt, 1); - lhs = gimple_call_lhs (stmt); - gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_EVEN_EXPR, arg0, arg1); - gimple_set_location (g, gimple_location (stmt)); - gsi_replace (gsi, g, true); - return true; - } - /* Odd element flavors of vec_mul (signed). */ - case ALTIVEC_BUILTIN_VMULOSB: - case ALTIVEC_BUILTIN_VMULOSH: - /* Odd element flavors of vec_mul (unsigned). */ - case ALTIVEC_BUILTIN_VMULOUB: - case ALTIVEC_BUILTIN_VMULOUH: - { - arg0 = gimple_call_arg (stmt, 0); - arg1 = gimple_call_arg (stmt, 1); - lhs = gimple_call_lhs (stmt); - gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_ODD_EXPR, arg0, arg1); - gimple_set_location (g, gimple_location (stmt)); - gsi_replace (gsi, g, true); - return true; - } - default: break; } diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-even_odd_misc.c b/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-even_odd_misc.c new file mode 100644 index 0000000..583539e --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-even_odd_misc.c @@ -0,0 +1,58 @@ + +/* { dg-do run } */ +/* { dg-require-effective-target powerpc_vsx_ok } */ +/* { dg-options "-maltivec -mvsx -O2 -save-temps" } */ + +#include <altivec.h> + +__attribute__((noinline)) void +test_eub_char () +{ + vector unsigned char v0 = {1, 0, 0, 0, 0, 0, 0, 0}; + 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 () +{ + vector unsigned char v0 = {0, 1, 0, 0, 0, 0, 0, 0}; + 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 () +{ + vector unsigned short v0 = {1, 0, 0, 0}; + 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 () +{ + vector unsigned short v0 = {0, 1, 0, 0}; + 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 } } */ +
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. Thusly, remove those entries from the folding list. At the same time, I am adding a testcase to provide some basic coverage for those ops. Functional gimple folding for those operations will be showing up at a later time. OK for trunk? regtest is currently running on ppc64*. gcc: 2017-03-07 Will Schmidt <will_schmidt@vnet.ibm.com> PR middle-end/79941 * config/rs6000/rs6000.c (gimplify_init_constructor): Remove multiply even and multiply odd (vmule/vmulo) intrinsics from the multiply folding sequence. testsuite: 2017-03-07 Will Schmidt <will_schmidt@vnet.ibm.com> PR middle-end/79941 * gcc.target/powerpc/fold-vec-mult-even_odd_misc.c: New test. --- gcc/config/rs6000/rs6000.c | 31 ----------- .../powerpc/fold-vec-mult-even_odd_misc.c | 58 ++++++++++++++++++++ 2 files changed, 58 insertions(+), 31 deletions(-) create mode 100644 gcc/testsuite/gcc.target/powerpc/fold-vec-mult-even_odd_misc.c