Re: [PATCH, PR68062] Fix uniform vector operation lowering
On 26 Oct 10:56, Richard Biener wrote: > On Mon, Oct 26, 2015 at 10:35 AM, Ilya Enkovich> wrote: > > On 26 Oct 10:09, Richard Biener wrote: > >> On Sat, Oct 24, 2015 at 12:29 AM, Ilya Enkovich > >> wrote: > >> > 2015-10-24 0:32 GMT+03:00 Jeff Law : > >> >> On 10/23/2015 09:26 AM, Ilya Enkovich wrote: > >> >>> > >> >>> Hi, > >> >>> > >> >>> This patch checks optab exists before using it vector vector statement > >> >>> lowering. It fixes compilation of test from PR68062 with > >> >>> -funsigned-char > >> >>> option added (doesn't fix original testcase). Bootstrapped for > >> >>> x86_64-unknown-linux-gnu. OK for trunk if no regressions? > >> >>> > >> >>> Thanks, > >> >>> Ilya > >> >>> -- > >> >>> gcc/ > >> >>> > >> >>> 2015-10-23 Ilya Enkovich > >> >>> > >> >>> * tree-vect-generic.c (expand_vector_operations_1): Check > >> >>> optab exists before use it. > >> >>> > >> >>> gcc/testsuite/ > >> >>> > >> >>> 2015-10-23 Ilya Enkovich > >> >>> > >> >>> * g++.dg/pr68062.C: New test. > >> >> > >> >> OK. > >> >> > >> >> Just curious, what was the tree code for which we couldn't find a > >> >> suitable > >> >> optab? > >> > > >> > Those are various comparison codes. > >> > >> Yeah, sorry for missing that check. Btw, I was curious to see that we miss > >> a way to query from optab_tag the "kind" (normal, conversion, etc.) so code > >> can decide what optab_handler function to call (optab_handler or > >> convert_optab_handler). So the code I added errs on the "simplistic" side > >> and hopes that matching lhs and rhs1 type always gets us a non-convert > >> optab... > > > > So probably better fix is > > > > diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c > > index d1fc0ba..73c5cc5 100644 > > --- a/gcc/tree-vect-generic.c > > +++ b/gcc/tree-vect-generic.c > > @@ -1533,7 +1533,8 @@ expand_vector_operations_1 (gimple_stmt_iterator *gsi) > >&& TYPE_MODE (TREE_TYPE (type)) == TYPE_MODE (TREE_TYPE (srhs1))) > > { > >op = optab_for_tree_code (code, TREE_TYPE (type), optab_scalar); > > - if (optab_handler (op, TYPE_MODE (TREE_TYPE (type))) != > > CODE_FOR_nothing) > > + if (op >= FIRST_NORM_OPTAB && op <= LAST_NORM_OPTAB > > + && optab_handler (op, TYPE_MODE (TREE_TYPE (type))) != > > CODE_FOR_nothing) > > { > > tree slhs = make_ssa_name (TREE_TYPE (srhs1)); > > gimple *repl = gimple_build_assign (slhs, code, srhs1, srhs2); > > > > ? > > Ah, didn't know we have those constants - yes, that's a better fix. > After all we want > optab_handler to return sth sensible for it. I'll install it then. Here is a version rebased on trunk. Thanks, Ilya -- gcc/ 2015-10-26 Ilya Enkovich * tree-vect-generic.c (expand_vector_operations_1): Check optab type before using it. diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c index 9c59402..a376ca2 100644 --- a/gcc/tree-vect-generic.c +++ b/gcc/tree-vect-generic.c @@ -1533,7 +1533,7 @@ expand_vector_operations_1 (gimple_stmt_iterator *gsi) && TYPE_MODE (TREE_TYPE (type)) == TYPE_MODE (TREE_TYPE (srhs1))) { op = optab_for_tree_code (code, TREE_TYPE (type), optab_scalar); - if (op != unknown_optab + if (op >= FIRST_NORM_OPTAB && op <= LAST_NORM_OPTAB && optab_handler (op, TYPE_MODE (TREE_TYPE (type))) != CODE_FOR_nothing) { tree slhs = make_ssa_name (TREE_TYPE (srhs1));
Re: [PATCH, PR68062] Fix uniform vector operation lowering
On 26 Oct 10:09, Richard Biener wrote: > On Sat, Oct 24, 2015 at 12:29 AM, Ilya Enkovich> wrote: > > 2015-10-24 0:32 GMT+03:00 Jeff Law : > >> On 10/23/2015 09:26 AM, Ilya Enkovich wrote: > >>> > >>> Hi, > >>> > >>> This patch checks optab exists before using it vector vector statement > >>> lowering. It fixes compilation of test from PR68062 with -funsigned-char > >>> option added (doesn't fix original testcase). Bootstrapped for > >>> x86_64-unknown-linux-gnu. OK for trunk if no regressions? > >>> > >>> Thanks, > >>> Ilya > >>> -- > >>> gcc/ > >>> > >>> 2015-10-23 Ilya Enkovich > >>> > >>> * tree-vect-generic.c (expand_vector_operations_1): Check > >>> optab exists before use it. > >>> > >>> gcc/testsuite/ > >>> > >>> 2015-10-23 Ilya Enkovich > >>> > >>> * g++.dg/pr68062.C: New test. > >> > >> OK. > >> > >> Just curious, what was the tree code for which we couldn't find a suitable > >> optab? > > > > Those are various comparison codes. > > Yeah, sorry for missing that check. Btw, I was curious to see that we miss > a way to query from optab_tag the "kind" (normal, conversion, etc.) so code > can decide what optab_handler function to call (optab_handler or > convert_optab_handler). So the code I added errs on the "simplistic" side > and hopes that matching lhs and rhs1 type always gets us a non-convert > optab... So probably better fix is diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c index d1fc0ba..73c5cc5 100644 --- a/gcc/tree-vect-generic.c +++ b/gcc/tree-vect-generic.c @@ -1533,7 +1533,8 @@ expand_vector_operations_1 (gimple_stmt_iterator *gsi) && TYPE_MODE (TREE_TYPE (type)) == TYPE_MODE (TREE_TYPE (srhs1))) { op = optab_for_tree_code (code, TREE_TYPE (type), optab_scalar); - if (optab_handler (op, TYPE_MODE (TREE_TYPE (type))) != CODE_FOR_nothing) + if (op >= FIRST_NORM_OPTAB && op <= LAST_NORM_OPTAB + && optab_handler (op, TYPE_MODE (TREE_TYPE (type))) != CODE_FOR_nothing) { tree slhs = make_ssa_name (TREE_TYPE (srhs1)); gimple *repl = gimple_build_assign (slhs, code, srhs1, srhs2); ? Ilya > > Richard. > > > Ilya > > > >> > >> jeff > >>
Re: [PATCH, PR68062] Fix uniform vector operation lowering
On Mon, Oct 26, 2015 at 10:35 AM, Ilya Enkovichwrote: > On 26 Oct 10:09, Richard Biener wrote: >> On Sat, Oct 24, 2015 at 12:29 AM, Ilya Enkovich >> wrote: >> > 2015-10-24 0:32 GMT+03:00 Jeff Law : >> >> On 10/23/2015 09:26 AM, Ilya Enkovich wrote: >> >>> >> >>> Hi, >> >>> >> >>> This patch checks optab exists before using it vector vector statement >> >>> lowering. It fixes compilation of test from PR68062 with -funsigned-char >> >>> option added (doesn't fix original testcase). Bootstrapped for >> >>> x86_64-unknown-linux-gnu. OK for trunk if no regressions? >> >>> >> >>> Thanks, >> >>> Ilya >> >>> -- >> >>> gcc/ >> >>> >> >>> 2015-10-23 Ilya Enkovich >> >>> >> >>> * tree-vect-generic.c (expand_vector_operations_1): Check >> >>> optab exists before use it. >> >>> >> >>> gcc/testsuite/ >> >>> >> >>> 2015-10-23 Ilya Enkovich >> >>> >> >>> * g++.dg/pr68062.C: New test. >> >> >> >> OK. >> >> >> >> Just curious, what was the tree code for which we couldn't find a suitable >> >> optab? >> > >> > Those are various comparison codes. >> >> Yeah, sorry for missing that check. Btw, I was curious to see that we miss >> a way to query from optab_tag the "kind" (normal, conversion, etc.) so code >> can decide what optab_handler function to call (optab_handler or >> convert_optab_handler). So the code I added errs on the "simplistic" side >> and hopes that matching lhs and rhs1 type always gets us a non-convert >> optab... > > So probably better fix is > > diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c > index d1fc0ba..73c5cc5 100644 > --- a/gcc/tree-vect-generic.c > +++ b/gcc/tree-vect-generic.c > @@ -1533,7 +1533,8 @@ expand_vector_operations_1 (gimple_stmt_iterator *gsi) >&& TYPE_MODE (TREE_TYPE (type)) == TYPE_MODE (TREE_TYPE (srhs1))) > { >op = optab_for_tree_code (code, TREE_TYPE (type), optab_scalar); > - if (optab_handler (op, TYPE_MODE (TREE_TYPE (type))) != > CODE_FOR_nothing) > + if (op >= FIRST_NORM_OPTAB && op <= LAST_NORM_OPTAB > + && optab_handler (op, TYPE_MODE (TREE_TYPE (type))) != > CODE_FOR_nothing) > { > tree slhs = make_ssa_name (TREE_TYPE (srhs1)); > gimple *repl = gimple_build_assign (slhs, code, srhs1, srhs2); > > ? Ah, didn't know we have those constants - yes, that's a better fix. After all we want optab_handler to return sth sensible for it. Thanks, Richard. > Ilya > >> >> Richard. >> >> > Ilya >> > >> >> >> >> jeff >> >>
Re: [PATCH, PR68062] Fix uniform vector operation lowering
On Sat, Oct 24, 2015 at 12:29 AM, Ilya Enkovichwrote: > 2015-10-24 0:32 GMT+03:00 Jeff Law : >> On 10/23/2015 09:26 AM, Ilya Enkovich wrote: >>> >>> Hi, >>> >>> This patch checks optab exists before using it vector vector statement >>> lowering. It fixes compilation of test from PR68062 with -funsigned-char >>> option added (doesn't fix original testcase). Bootstrapped for >>> x86_64-unknown-linux-gnu. OK for trunk if no regressions? >>> >>> Thanks, >>> Ilya >>> -- >>> gcc/ >>> >>> 2015-10-23 Ilya Enkovich >>> >>> * tree-vect-generic.c (expand_vector_operations_1): Check >>> optab exists before use it. >>> >>> gcc/testsuite/ >>> >>> 2015-10-23 Ilya Enkovich >>> >>> * g++.dg/pr68062.C: New test. >> >> OK. >> >> Just curious, what was the tree code for which we couldn't find a suitable >> optab? > > Those are various comparison codes. Yeah, sorry for missing that check. Btw, I was curious to see that we miss a way to query from optab_tag the "kind" (normal, conversion, etc.) so code can decide what optab_handler function to call (optab_handler or convert_optab_handler). So the code I added errs on the "simplistic" side and hopes that matching lhs and rhs1 type always gets us a non-convert optab... Richard. > Ilya > >> >> jeff >>
Re: [PATCH, PR68062] Fix uniform vector operation lowering
On 10/23/2015 09:26 AM, Ilya Enkovich wrote: Hi, This patch checks optab exists before using it vector vector statement lowering. It fixes compilation of test from PR68062 with -funsigned-char option added (doesn't fix original testcase). Bootstrapped for x86_64-unknown-linux-gnu. OK for trunk if no regressions? Thanks, Ilya -- gcc/ 2015-10-23 Ilya Enkovich* tree-vect-generic.c (expand_vector_operations_1): Check optab exists before use it. gcc/testsuite/ 2015-10-23 Ilya Enkovich * g++.dg/pr68062.C: New test. OK. Just curious, what was the tree code for which we couldn't find a suitable optab? jeff
Re: [PATCH, PR68062] Fix uniform vector operation lowering
2015-10-24 0:32 GMT+03:00 Jeff Law: > On 10/23/2015 09:26 AM, Ilya Enkovich wrote: >> >> Hi, >> >> This patch checks optab exists before using it vector vector statement >> lowering. It fixes compilation of test from PR68062 with -funsigned-char >> option added (doesn't fix original testcase). Bootstrapped for >> x86_64-unknown-linux-gnu. OK for trunk if no regressions? >> >> Thanks, >> Ilya >> -- >> gcc/ >> >> 2015-10-23 Ilya Enkovich >> >> * tree-vect-generic.c (expand_vector_operations_1): Check >> optab exists before use it. >> >> gcc/testsuite/ >> >> 2015-10-23 Ilya Enkovich >> >> * g++.dg/pr68062.C: New test. > > OK. > > Just curious, what was the tree code for which we couldn't find a suitable > optab? Those are various comparison codes. Ilya > > jeff >