* [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode @ 2015-11-14 7:12 Senthil Kumar Selvaraj 2015-11-14 8:13 ` Marc Glisse 0 siblings, 1 reply; 13+ messages in thread From: Senthil Kumar Selvaraj @ 2015-11-14 7:12 UTC (permalink / raw) To: gcc-patches; +Cc: rguenther, law This patch came out of a discussion held in the gcc mailing list (https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). The patch restricts folding of conditional exprs with lhs previously set by a type conversion to occur only if the source of the type conversion's mode is word mode or smaller. Bootstrapped and reg tested on x86_64 (with --enable-languages=c,c++). If ok, could you commit please? I don't have commit access. Regards Senthil gcc/ChangeLog 2015-11-11 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> * tree-vrp.c (simplify_cond_using_ranges): Fold only if innerop's mode is word_mode or smaller. diff --git gcc/tree-vrp.c gcc/tree-vrp.c index e2393e4..c139bc6 100644 --- gcc/tree-vrp.c +++ gcc/tree-vrp.c @@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) innerop = gimple_assign_rhs1 (def_stmt); if (TREE_CODE (innerop) == SSA_NAME + && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) + <= GET_MODE_SIZE(word_mode)) && !POINTER_TYPE_P (TREE_TYPE (innerop))) { value_range *vr = get_value_range (innerop); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-14 7:12 [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode Senthil Kumar Selvaraj @ 2015-11-14 8:13 ` Marc Glisse 2015-11-14 8:50 ` Senthil Kumar Selvaraj 0 siblings, 1 reply; 13+ messages in thread From: Marc Glisse @ 2015-11-14 8:13 UTC (permalink / raw) To: Senthil Kumar Selvaraj; +Cc: gcc-patches, rguenther, law On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > This patch came out of a discussion held in the gcc mailing list > (https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). > > The patch restricts folding of conditional exprs with lhs previously > set by a type conversion to occur only if the source of the type > conversion's mode is word mode or smaller. > > Bootstrapped and reg tested on x86_64 (with --enable-languages=c,c++). > > If ok, could you commit please? I don't have commit access. > > Regards > Senthil > > gcc/ChangeLog > > 2015-11-11 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > * tree-vrp.c (simplify_cond_using_ranges): Fold only > if innerop's mode is word_mode or smaller. > > > diff --git gcc/tree-vrp.c gcc/tree-vrp.c > index e2393e4..c139bc6 100644 > --- gcc/tree-vrp.c > +++ gcc/tree-vrp.c > @@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > innerop = gimple_assign_rhs1 (def_stmt); > > if (TREE_CODE (innerop) == SSA_NAME > + && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) > + <= GET_MODE_SIZE(word_mode)) > && !POINTER_TYPE_P (TREE_TYPE (innerop))) > { > value_range *vr = get_value_range (innerop); I thought the result of the discussion was that the transformation is ok if either it is narrowing or it widens but to something no bigger than word_mode. So you should have 2 comparisons, or 1 with a max. -- Marc Glisse ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-14 8:13 ` Marc Glisse @ 2015-11-14 8:50 ` Senthil Kumar Selvaraj 2015-11-14 8:57 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Senthil Kumar Selvaraj @ 2015-11-14 8:50 UTC (permalink / raw) To: gcc-patches; +Cc: rguenther, law On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote: > On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > > >This patch came out of a discussion held in the gcc mailing list > >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). > > > >The patch restricts folding of conditional exprs with lhs previously > >set by a type conversion to occur only if the source of the type > >conversion's mode is word mode or smaller. > > > >Bootstrapped and reg tested on x86_64 (with --enable-languages=c,c++). > > > >If ok, could you commit please? I don't have commit access. > > > >Regards > >Senthil > > > >gcc/ChangeLog > > > >2015-11-11 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > > > * tree-vrp.c (simplify_cond_using_ranges): Fold only > > if innerop's mode is word_mode or smaller. > > > > > >diff --git gcc/tree-vrp.c gcc/tree-vrp.c > >index e2393e4..c139bc6 100644 > >--- gcc/tree-vrp.c > >+++ gcc/tree-vrp.c > >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > > innerop = gimple_assign_rhs1 (def_stmt); > > > > if (TREE_CODE (innerop) == SSA_NAME > >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) > >+ <= GET_MODE_SIZE(word_mode)) > > && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > { > > value_range *vr = get_value_range (innerop); > > I thought the result of the discussion was that the transformation is ok if > either it is narrowing or it widens but to something no bigger than > word_mode. So you should have 2 comparisons, or 1 with a max. Hmm, I came to the opposite conclusion - I thought Richard only okayed "widening upto word-mode", not the narrowing. Richard? Regards Senthil > > -- > Marc Glisse ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-14 8:50 ` Senthil Kumar Selvaraj @ 2015-11-14 8:57 ` Richard Biener 2015-11-14 18:11 ` Senthil Kumar Selvaraj 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2015-11-14 8:57 UTC (permalink / raw) To: Senthil Kumar Selvaraj, gcc-patches; +Cc: law On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote: >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote: >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: >> >> >This patch came out of a discussion held in the gcc mailing list >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). >> > >> >The patch restricts folding of conditional exprs with lhs previously >> >set by a type conversion to occur only if the source of the type >> >conversion's mode is word mode or smaller. >> > >> >Bootstrapped and reg tested on x86_64 (with >--enable-languages=c,c++). >> > >> >If ok, could you commit please? I don't have commit access. >> > >> >Regards >> >Senthil >> > >> >gcc/ChangeLog >> > >> >2015-11-11 Senthil Kumar Selvaraj ><senthil_kumar.selvaraj@atmel.com> >> > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only >> > if innerop's mode is word_mode or smaller. >> > >> > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c >> >index e2393e4..c139bc6 100644 >> >--- gcc/tree-vrp.c >> >+++ gcc/tree-vrp.c >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) >> > innerop = gimple_assign_rhs1 (def_stmt); >> > >> > if (TREE_CODE (innerop) == SSA_NAME >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) >> >+ <= GET_MODE_SIZE(word_mode)) >> > && !POINTER_TYPE_P (TREE_TYPE (innerop))) >> > { >> > value_range *vr = get_value_range (innerop); >> >> I thought the result of the discussion was that the transformation is >ok if >> either it is narrowing or it widens but to something no bigger than >> word_mode. So you should have 2 comparisons, or 1 with a max. > >Hmm, I came to the opposite conclusion - I thought Richard only okayed >"widening upto word-mode", not the narrowing. I didn't mean to suggest narrowing is not OK. In fact narrowing is always OK. Richard. >Richard? > >Regards >Senthil >> >> -- >> Marc Glisse ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-14 8:57 ` Richard Biener @ 2015-11-14 18:11 ` Senthil Kumar Selvaraj 2015-11-16 9:02 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Senthil Kumar Selvaraj @ 2015-11-14 18:11 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, law On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote: > On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote: > >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote: > >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > >> > >> >This patch came out of a discussion held in the gcc mailing list > >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). > >> > > >> >The patch restricts folding of conditional exprs with lhs previously > >> >set by a type conversion to occur only if the source of the type > >> >conversion's mode is word mode or smaller. > >> > > >> >Bootstrapped and reg tested on x86_64 (with > >--enable-languages=c,c++). > >> > > >> >If ok, could you commit please? I don't have commit access. > >> > > >> >Regards > >> >Senthil > >> > > >> >gcc/ChangeLog > >> > > >> >2015-11-11 Senthil Kumar Selvaraj > ><senthil_kumar.selvaraj@atmel.com> > >> > > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only > >> > if innerop's mode is word_mode or smaller. > >> > > >> > > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c > >> >index e2393e4..c139bc6 100644 > >> >--- gcc/tree-vrp.c > >> >+++ gcc/tree-vrp.c > >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > >> > innerop = gimple_assign_rhs1 (def_stmt); > >> > > >> > if (TREE_CODE (innerop) == SSA_NAME > >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) > >> >+ <= GET_MODE_SIZE(word_mode)) > >> > && !POINTER_TYPE_P (TREE_TYPE (innerop))) > >> > { > >> > value_range *vr = get_value_range (innerop); > >> > >> I thought the result of the discussion was that the transformation is > >ok if > >> either it is narrowing or it widens but to something no bigger than > >> word_mode. So you should have 2 comparisons, or 1 with a max. > > > >Hmm, I came to the opposite conclusion - I thought Richard only okayed > >"widening upto word-mode", not the narrowing. > > I didn't mean to suggest narrowing is not OK. In fact narrowing is always OK. My bad. Here's a revised patch that checks for both conditions, using max as Marc suggested to limit to word_mode or narrowing conversions. Bootstrapped and regtested for x86_64 with c and c++. Is this ok? If yes, would you commit it for me please? I don't have commit access. gcc/ChangeLog 2015-11-14 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> * tree-vrp.c (simplify_cond_using_ranges): Fold only if innerop's mode smaller or equal to word_mode or op0's mode. diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index e2393e4..cfd90e7 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -9467,7 +9467,10 @@ simplify_cond_using_ranges (gcond *stmt) innerop = gimple_assign_rhs1 (def_stmt); if (TREE_CODE (innerop) == SSA_NAME - && !POINTER_TYPE_P (TREE_TYPE (innerop))) + && !POINTER_TYPE_P (TREE_TYPE (innerop)) + && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (innerop))) + <= std::max (GET_MODE_SIZE (word_mode), + GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))))) { value_range *vr = get_value_range (innerop); Regards Senthil > > Richard. > > >Richard? > > > >Regards > >Senthil > >> > >> -- > >> Marc Glisse > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-14 18:11 ` Senthil Kumar Selvaraj @ 2015-11-16 9:02 ` Richard Biener 2015-11-17 19:53 ` Senthil Kumar Selvaraj 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2015-11-16 9:02 UTC (permalink / raw) To: Senthil Kumar Selvaraj; +Cc: gcc-patches, law On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote: > > On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote: > > >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote: > > >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > > >> > > >> >This patch came out of a discussion held in the gcc mailing list > > >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). > > >> > > > >> >The patch restricts folding of conditional exprs with lhs previously > > >> >set by a type conversion to occur only if the source of the type > > >> >conversion's mode is word mode or smaller. > > >> > > > >> >Bootstrapped and reg tested on x86_64 (with > > >--enable-languages=c,c++). > > >> > > > >> >If ok, could you commit please? I don't have commit access. > > >> > > > >> >Regards > > >> >Senthil > > >> > > > >> >gcc/ChangeLog > > >> > > > >> >2015-11-11 Senthil Kumar Selvaraj > > ><senthil_kumar.selvaraj@atmel.com> > > >> > > > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only > > >> > if innerop's mode is word_mode or smaller. > > >> > > > >> > > > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c > > >> >index e2393e4..c139bc6 100644 > > >> >--- gcc/tree-vrp.c > > >> >+++ gcc/tree-vrp.c > > >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > > >> > innerop = gimple_assign_rhs1 (def_stmt); > > >> > > > >> > if (TREE_CODE (innerop) == SSA_NAME > > >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) > > >> >+ <= GET_MODE_SIZE(word_mode)) > > >> > && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > >> > { > > >> > value_range *vr = get_value_range (innerop); > > >> > > >> I thought the result of the discussion was that the transformation is > > >ok if > > >> either it is narrowing or it widens but to something no bigger than > > >> word_mode. So you should have 2 comparisons, or 1 with a max. > > > > > >Hmm, I came to the opposite conclusion - I thought Richard only okayed > > >"widening upto word-mode", not the narrowing. > > > > I didn't mean to suggest narrowing is not OK. In fact narrowing is always OK. > > My bad. Here's a revised patch that checks for both conditions, using > max as Marc suggested to limit to word_mode or narrowing conversions. > > Bootstrapped and regtested for x86_64 with c and c++. > > Is this ok? If yes, would you commit it > for me please? I don't have commit access. > > gcc/ChangeLog > 2015-11-14 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > * tree-vrp.c (simplify_cond_using_ranges): Fold only > if innerop's mode smaller or equal to word_mode or op0's mode. > > > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c > index e2393e4..cfd90e7 100644 > --- a/gcc/tree-vrp.c > +++ b/gcc/tree-vrp.c > @@ -9467,7 +9467,10 @@ simplify_cond_using_ranges (gcond *stmt) > innerop = gimple_assign_rhs1 (def_stmt); > > if (TREE_CODE (innerop) == SSA_NAME > - && !POINTER_TYPE_P (TREE_TYPE (innerop))) > + && !POINTER_TYPE_P (TREE_TYPE (innerop)) > + && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (innerop))) > + <= std::max (GET_MODE_SIZE (word_mode), > + GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))))) Please use TYPE_PRECISION (...) and GET_MODE_PRECISION (word_mode) and add a comment as to what we are testing here and why. Btw, ideally we'd factor out a bool desired_pro_or_demotion_p (tree to_type, tree from_type) {} function somewhere as we have similar tests throughout the compiler that we might want to unify (and also have a central place to eventually add a target hook if ever desired). In fact in other places we also check that the type we promote/demote to matches its mode precision or the type we promote/demote from already does not. I'd suggest tree.[ch] for that function. Please also add a testcase. Thanks, Richard. > { > value_range *vr = get_value_range (innerop); > > > Regards > Senthil > > > > Richard. > > > > >Richard? > > > > > >Regards > > >Senthil > > >> > > >> -- > > >> Marc Glisse > > > > > > -- Richard Biener <rguenther@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-16 9:02 ` Richard Biener @ 2015-11-17 19:53 ` Senthil Kumar Selvaraj 2015-11-18 8:36 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Senthil Kumar Selvaraj @ 2015-11-17 19:53 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, law On Mon, Nov 16, 2015 at 10:02:15AM +0100, Richard Biener wrote: > On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > > > On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote: > > > On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote: > > > >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote: > > > >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > > > >> > > > >> >This patch came out of a discussion held in the gcc mailing list > > > >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). > > > >> > > > > >> >The patch restricts folding of conditional exprs with lhs previously > > > >> >set by a type conversion to occur only if the source of the type > > > >> >conversion's mode is word mode or smaller. > > > >> > > > > >> >Bootstrapped and reg tested on x86_64 (with > > > >--enable-languages=c,c++). > > > >> > > > > >> >If ok, could you commit please? I don't have commit access. > > > >> > > > > >> >Regards > > > >> >Senthil > > > >> > > > > >> >gcc/ChangeLog > > > >> > > > > >> >2015-11-11 Senthil Kumar Selvaraj > > > ><senthil_kumar.selvaraj@atmel.com> > > > >> > > > > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only > > > >> > if innerop's mode is word_mode or smaller. > > > >> > > > > >> > > > > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c > > > >> >index e2393e4..c139bc6 100644 > > > >> >--- gcc/tree-vrp.c > > > >> >+++ gcc/tree-vrp.c > > > >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > > > >> > innerop = gimple_assign_rhs1 (def_stmt); > > > >> > > > > >> > if (TREE_CODE (innerop) == SSA_NAME > > > >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) > > > >> >+ <= GET_MODE_SIZE(word_mode)) > > > >> > && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > > >> > { > > > >> > value_range *vr = get_value_range (innerop); > > > >> > > > >> I thought the result of the discussion was that the transformation is > > > >ok if > > > >> either it is narrowing or it widens but to something no bigger than > > > >> word_mode. So you should have 2 comparisons, or 1 with a max. > > > > > > > >Hmm, I came to the opposite conclusion - I thought Richard only okayed > > > >"widening upto word-mode", not the narrowing. > > > > > > I didn't mean to suggest narrowing is not OK. In fact narrowing is always OK. > > > > My bad. Here's a revised patch that checks for both conditions, using > > max as Marc suggested to limit to word_mode or narrowing conversions. > > > > Bootstrapped and regtested for x86_64 with c and c++. > > > > Is this ok? If yes, would you commit it > > for me please? I don't have commit access. > > > > gcc/ChangeLog > > 2015-11-14 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > > > * tree-vrp.c (simplify_cond_using_ranges): Fold only > > if innerop's mode smaller or equal to word_mode or op0's mode. > > > > > > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c > > index e2393e4..cfd90e7 100644 > > --- a/gcc/tree-vrp.c > > +++ b/gcc/tree-vrp.c > > @@ -9467,7 +9467,10 @@ simplify_cond_using_ranges (gcond *stmt) > > innerop = gimple_assign_rhs1 (def_stmt); > > > > if (TREE_CODE (innerop) == SSA_NAME > > - && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > + && !POINTER_TYPE_P (TREE_TYPE (innerop)) > > + && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (innerop))) > > + <= std::max (GET_MODE_SIZE (word_mode), > > + GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))))) > > Please use TYPE_PRECISION (...) and GET_MODE_PRECISION (word_mode) and > add a comment as to what we are testing here and why. > > Btw, ideally we'd factor out a > > bool > desired_pro_or_demotion_p (tree to_type, tree from_type) {} > > function somewhere as we have similar tests throughout the compiler > that we might want to unify (and also have a central place to > eventually add a target hook if ever desired). > > In fact in other places we also check that the type we promote/demote > to matches its mode precision or the type we promote/demote from > already does not. > > I'd suggest tree.[ch] for that function. > > Please also add a testcase. How does the below patch look? Bootstrapped, but not regtested yet. The testcase was rather tricky to write - I wasn't sure how to reliably get a type bigger than a word for all targets. I resorted to __int128, not sure it's a good idea though - I should probably add dg-skip-if for targets that don't support that. Do you know of a better way to write that? Regards Senthil diff --git gcc/testsuite/gcc.dg/tree-ssa/vrp98.c gcc/testsuite/gcc.dg/tree-ssa/vrp98.c new file mode 100644 index 0000000..448ceba --- /dev/null +++ gcc/testsuite/gcc.dg/tree-ssa/vrp98.c @@ -0,0 +1,40 @@ +/* { dg-do compile } */ +/* { dg-options "-Os -fdump-tree-vrp1-details" } */ + +#include <stdint.h> +#include <limits.h> + +typedef unsigned int word __attribute__((mode(word))); +typedef unsigned __int128 bigger_than_word; + +int +foo (bigger_than_word a, word b, uint8_t c) +{ + /* Must fold use of t1 into use of b, as b is no wider than word_mode. */ + const uint8_t t1 = b % UCHAR_MAX; + + /* Must NOT fold use of t2 into use of a, as a is wider than word_mode. */ + const uint8_t t2 = a % UCHAR_MAX; + + /* Must fold use of t3 into use of c, as c is narrower than t3. */ + const uint32_t t3 = (const uint32_t)(c >> 1); + + uint16_t ret = 0; + + if (t1 == 1) + ret = 20; + else if (t2 == 2) + ret = 30; + else if (t3 == 3) + ret = 40; + /* Th extra condition below is necessary to prevent a prior pass from + folding away the cast. Ignored in scan-tree-dump. */ + else if (t3 == 4) + ret = 50; + + return ret; +} + +/* { dg-final { scan-tree-dump "Folded into: if \\(_\[0-9\]+ == 1\\)" "vrp1" } } */ +/* { dg-final { scan-tree-dump-not "Folded into: if \\(_\[0-9\]+ == 2\\)" "vrp1" } } */ +/* { dg-final { scan-tree-dump "Folded into: if \\(_\[0-9\]+ == 3\\)" "vrp1" } } */ diff --git gcc/tree-vrp.c gcc/tree-vrp.c index 5d085b4..4513b88 100644 --- gcc/tree-vrp.c +++ gcc/tree-vrp.c @@ -9467,7 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) innerop = gimple_assign_rhs1 (def_stmt); if (TREE_CODE (innerop) == SSA_NAME - && !POINTER_TYPE_P (TREE_TYPE (innerop))) + && !POINTER_TYPE_P (TREE_TYPE (innerop)) + && desired_pro_or_demotion_p (innerop, op0)) { value_range *vr = get_value_range (innerop); diff --git gcc/tree.h gcc/tree.h index 0b9c3b9..91099ed 100644 --- gcc/tree.h +++ gcc/tree.h @@ -5316,4 +5316,18 @@ get_decl_source_range (tree decl) return get_range_from_loc (line_table, loc); } +/* Return true if it makes sense to promote/demote from_type to to_type. */ +inline bool +desired_pro_or_demotion_p (tree to_type, tree from_type) +{ + unsigned int to_type_precision = TYPE_PRECISION (TREE_TYPE (to_type)); + + /* OK to promote if to_type is no bigger than word_mode. */ + if (to_type_precision <= GET_MODE_PRECISION (word_mode)) + return true; + + /* Otherwise, allow only if narrowing or same precision conversions. */ + return to_type_precision <= TYPE_PRECISION (TREE_TYPE (from_type)); +} + #endif /* GCC_TREE_H */ > > Thanks, > Richard. > > > { > > value_range *vr = get_value_range (innerop); > > > > > > Regards > > Senthil > > > > > > Richard. > > > > > > >Richard? > > > > > > > >Regards > > > >Senthil > > > >> > > > >> -- > > > >> Marc Glisse > > > > > > > > > > > > -- > Richard Biener <rguenther@suse.de> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-17 19:53 ` Senthil Kumar Selvaraj @ 2015-11-18 8:36 ` Richard Biener 2015-11-19 6:20 ` Senthil Kumar Selvaraj 0 siblings, 1 reply; 13+ messages in thread From: Richard Biener @ 2015-11-18 8:36 UTC (permalink / raw) To: Senthil Kumar Selvaraj; +Cc: gcc-patches, law On Wed, 18 Nov 2015, Senthil Kumar Selvaraj wrote: > On Mon, Nov 16, 2015 at 10:02:15AM +0100, Richard Biener wrote: > > On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > > > > > On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote: > > > > On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote: > > > > >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote: > > > > >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > > > > >> > > > > >> >This patch came out of a discussion held in the gcc mailing list > > > > >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). > > > > >> > > > > > >> >The patch restricts folding of conditional exprs with lhs previously > > > > >> >set by a type conversion to occur only if the source of the type > > > > >> >conversion's mode is word mode or smaller. > > > > >> > > > > > >> >Bootstrapped and reg tested on x86_64 (with > > > > >--enable-languages=c,c++). > > > > >> > > > > > >> >If ok, could you commit please? I don't have commit access. > > > > >> > > > > > >> >Regards > > > > >> >Senthil > > > > >> > > > > > >> >gcc/ChangeLog > > > > >> > > > > > >> >2015-11-11 Senthil Kumar Selvaraj > > > > ><senthil_kumar.selvaraj@atmel.com> > > > > >> > > > > > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only > > > > >> > if innerop's mode is word_mode or smaller. > > > > >> > > > > > >> > > > > > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c > > > > >> >index e2393e4..c139bc6 100644 > > > > >> >--- gcc/tree-vrp.c > > > > >> >+++ gcc/tree-vrp.c > > > > >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > > > > >> > innerop = gimple_assign_rhs1 (def_stmt); > > > > >> > > > > > >> > if (TREE_CODE (innerop) == SSA_NAME > > > > >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) > > > > >> >+ <= GET_MODE_SIZE(word_mode)) > > > > >> > && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > > > >> > { > > > > >> > value_range *vr = get_value_range (innerop); > > > > >> > > > > >> I thought the result of the discussion was that the transformation is > > > > >ok if > > > > >> either it is narrowing or it widens but to something no bigger than > > > > >> word_mode. So you should have 2 comparisons, or 1 with a max. > > > > > > > > > >Hmm, I came to the opposite conclusion - I thought Richard only okayed > > > > >"widening upto word-mode", not the narrowing. > > > > > > > > I didn't mean to suggest narrowing is not OK. In fact narrowing is always OK. > > > > > > My bad. Here's a revised patch that checks for both conditions, using > > > max as Marc suggested to limit to word_mode or narrowing conversions. > > > > > > Bootstrapped and regtested for x86_64 with c and c++. > > > > > > Is this ok? If yes, would you commit it > > > for me please? I don't have commit access. > > > > > > gcc/ChangeLog > > > 2015-11-14 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > > > > > * tree-vrp.c (simplify_cond_using_ranges): Fold only > > > if innerop's mode smaller or equal to word_mode or op0's mode. > > > > > > > > > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c > > > index e2393e4..cfd90e7 100644 > > > --- a/gcc/tree-vrp.c > > > +++ b/gcc/tree-vrp.c > > > @@ -9467,7 +9467,10 @@ simplify_cond_using_ranges (gcond *stmt) > > > innerop = gimple_assign_rhs1 (def_stmt); > > > > > > if (TREE_CODE (innerop) == SSA_NAME > > > - && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > > + && !POINTER_TYPE_P (TREE_TYPE (innerop)) > > > + && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (innerop))) > > > + <= std::max (GET_MODE_SIZE (word_mode), > > > + GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))))) > > > > Please use TYPE_PRECISION (...) and GET_MODE_PRECISION (word_mode) and > > add a comment as to what we are testing here and why. > > > > Btw, ideally we'd factor out a > > > > bool > > desired_pro_or_demotion_p (tree to_type, tree from_type) {} > > > > function somewhere as we have similar tests throughout the compiler > > that we might want to unify (and also have a central place to > > eventually add a target hook if ever desired). > > > > In fact in other places we also check that the type we promote/demote > > to matches its mode precision or the type we promote/demote from > > already does not. > > > > I'd suggest tree.[ch] for that function. > > > > Please also add a testcase. > > How does the below patch look? Bootstrapped, but not regtested yet. > > The testcase was rather tricky to write - I wasn't sure how to reliably > get a type bigger than a word for all targets. I resorted to __int128, > not sure it's a good idea though - I should probably add dg-skip-if for > targets that don't support that. Do you know of a better way to write > that? Ah, I was hoping for an avr specific testcase (as you have that already) scanning assembler output - after all we want to check the final result, not just what VRP did. > diff --git gcc/tree-vrp.c gcc/tree-vrp.c > index 5d085b4..4513b88 100644 > --- gcc/tree-vrp.c > +++ gcc/tree-vrp.c > @@ -9467,7 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > innerop = gimple_assign_rhs1 (def_stmt); > > if (TREE_CODE (innerop) == SSA_NAME > - && !POINTER_TYPE_P (TREE_TYPE (innerop))) > + && !POINTER_TYPE_P (TREE_TYPE (innerop)) > + && desired_pro_or_demotion_p (innerop, op0)) > { > value_range *vr = get_value_range (innerop); > > diff --git gcc/tree.h gcc/tree.h > index 0b9c3b9..91099ed 100644 > --- gcc/tree.h > +++ gcc/tree.h > @@ -5316,4 +5316,18 @@ get_decl_source_range (tree decl) > return get_range_from_loc (line_table, loc); > } > > +/* Return true if it makes sense to promote/demote from_type to to_type. */ > +inline bool > +desired_pro_or_demotion_p (tree to_type, tree from_type) Please pass in a type here. Otherwise ok. Thanks, Richard. > +{ > + unsigned int to_type_precision = TYPE_PRECISION (TREE_TYPE (to_type)); > + > + /* OK to promote if to_type is no bigger than word_mode. */ > + if (to_type_precision <= GET_MODE_PRECISION (word_mode)) > + return true; > + > + /* Otherwise, allow only if narrowing or same precision conversions. */ > + return to_type_precision <= TYPE_PRECISION (TREE_TYPE (from_type)); > +} > + > #endif /* GCC_TREE_H */ > > > > > Thanks, > > Richard. > > > > > { > > > value_range *vr = get_value_range (innerop); > > > > > > > > > Regards > > > Senthil > > > > > > > > Richard. > > > > > > > > >Richard? > > > > > > > > > >Regards > > > > >Senthil > > > > >> > > > > >> -- > > > > >> Marc Glisse > > > > > > > > > > > > > > > > > > -- > > Richard Biener <rguenther@suse.de> > > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) > > -- Richard Biener <rguenther@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-18 8:36 ` Richard Biener @ 2015-11-19 6:20 ` Senthil Kumar Selvaraj 2015-11-19 17:31 ` Jeff Law 0 siblings, 1 reply; 13+ messages in thread From: Senthil Kumar Selvaraj @ 2015-11-19 6:20 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, law On Wed, Nov 18, 2015 at 09:36:21AM +0100, Richard Biener wrote: > On Wed, 18 Nov 2015, Senthil Kumar Selvaraj wrote: > > > On Mon, Nov 16, 2015 at 10:02:15AM +0100, Richard Biener wrote: > > > On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > > > > > > > On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote: > > > > > On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote: > > > > > >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote: > > > > > >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote: > > > > > >> > > > > > >> >This patch came out of a discussion held in the gcc mailing list > > > > > >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html). > > > > > >> > > > > > > >> >The patch restricts folding of conditional exprs with lhs previously > > > > > >> >set by a type conversion to occur only if the source of the type > > > > > >> >conversion's mode is word mode or smaller. > > > > > >> > > > > > > >> >Bootstrapped and reg tested on x86_64 (with > > > > > >--enable-languages=c,c++). > > > > > >> > > > > > > >> >If ok, could you commit please? I don't have commit access. > > > > > >> > > > > > > >> >Regards > > > > > >> >Senthil > > > > > >> > > > > > > >> >gcc/ChangeLog > > > > > >> > > > > > > >> >2015-11-11 Senthil Kumar Selvaraj > > > > > ><senthil_kumar.selvaraj@atmel.com> > > > > > >> > > > > > > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only > > > > > >> > if innerop's mode is word_mode or smaller. > > > > > >> > > > > > > >> > > > > > > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c > > > > > >> >index e2393e4..c139bc6 100644 > > > > > >> >--- gcc/tree-vrp.c > > > > > >> >+++ gcc/tree-vrp.c > > > > > >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > > > > > >> > innerop = gimple_assign_rhs1 (def_stmt); > > > > > >> > > > > > > >> > if (TREE_CODE (innerop) == SSA_NAME > > > > > >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop))) > > > > > >> >+ <= GET_MODE_SIZE(word_mode)) > > > > > >> > && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > > > > >> > { > > > > > >> > value_range *vr = get_value_range (innerop); > > > > > >> > > > > > >> I thought the result of the discussion was that the transformation is > > > > > >ok if > > > > > >> either it is narrowing or it widens but to something no bigger than > > > > > >> word_mode. So you should have 2 comparisons, or 1 with a max. > > > > > > > > > > > >Hmm, I came to the opposite conclusion - I thought Richard only okayed > > > > > >"widening upto word-mode", not the narrowing. > > > > > > > > > > I didn't mean to suggest narrowing is not OK. In fact narrowing is always OK. > > > > > > > > My bad. Here's a revised patch that checks for both conditions, using > > > > max as Marc suggested to limit to word_mode or narrowing conversions. > > > > > > > > Bootstrapped and regtested for x86_64 with c and c++. > > > > > > > > Is this ok? If yes, would you commit it > > > > for me please? I don't have commit access. > > > > > > > > gcc/ChangeLog > > > > 2015-11-14 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > > > > > > > * tree-vrp.c (simplify_cond_using_ranges): Fold only > > > > if innerop's mode smaller or equal to word_mode or op0's mode. > > > > > > > > > > > > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c > > > > index e2393e4..cfd90e7 100644 > > > > --- a/gcc/tree-vrp.c > > > > +++ b/gcc/tree-vrp.c > > > > @@ -9467,7 +9467,10 @@ simplify_cond_using_ranges (gcond *stmt) > > > > innerop = gimple_assign_rhs1 (def_stmt); > > > > > > > > if (TREE_CODE (innerop) == SSA_NAME > > > > - && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > > > + && !POINTER_TYPE_P (TREE_TYPE (innerop)) > > > > + && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (innerop))) > > > > + <= std::max (GET_MODE_SIZE (word_mode), > > > > + GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))))) > > > > > > Please use TYPE_PRECISION (...) and GET_MODE_PRECISION (word_mode) and > > > add a comment as to what we are testing here and why. > > > > > > Btw, ideally we'd factor out a > > > > > > bool > > > desired_pro_or_demotion_p (tree to_type, tree from_type) {} > > > > > > function somewhere as we have similar tests throughout the compiler > > > that we might want to unify (and also have a central place to > > > eventually add a target hook if ever desired). > > > > > > In fact in other places we also check that the type we promote/demote > > > to matches its mode precision or the type we promote/demote from > > > already does not. > > > > > > I'd suggest tree.[ch] for that function. > > > > > > Please also add a testcase. > > > > How does the below patch look? Bootstrapped, but not regtested yet. > > > > The testcase was rather tricky to write - I wasn't sure how to reliably > > get a type bigger than a word for all targets. I resorted to __int128, > > not sure it's a good idea though - I should probably add dg-skip-if for > > targets that don't support that. Do you know of a better way to write > > that? > > Ah, I was hoping for an avr specific testcase (as you have that already) > scanning assembler output - after all we want to check the final result, > not just what VRP did. > > > diff --git gcc/tree-vrp.c gcc/tree-vrp.c > > index 5d085b4..4513b88 100644 > > --- gcc/tree-vrp.c > > +++ gcc/tree-vrp.c > > @@ -9467,7 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt) > > innerop = gimple_assign_rhs1 (def_stmt); > > > > if (TREE_CODE (innerop) == SSA_NAME > > - && !POINTER_TYPE_P (TREE_TYPE (innerop))) > > + && !POINTER_TYPE_P (TREE_TYPE (innerop)) > > + && desired_pro_or_demotion_p (innerop, op0)) > > { > > value_range *vr = get_value_range (innerop); > > > > diff --git gcc/tree.h gcc/tree.h > > index 0b9c3b9..91099ed 100644 > > --- gcc/tree.h > > +++ gcc/tree.h > > @@ -5316,4 +5316,18 @@ get_decl_source_range (tree decl) > > return get_range_from_loc (line_table, loc); > > } > > > > +/* Return true if it makes sense to promote/demote from_type to to_type. */ > > +inline bool > > +desired_pro_or_demotion_p (tree to_type, tree from_type) > > Please pass in a type here. > > Otherwise ok. See modified patch below. If you think vrp98.c is unnecessary, feel free to dump it :). If ok, could you commit it for me please? I don't have commit access. Regards Senthil gcc/ChangeLog 2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> * tree.h (desired_pro_or_demotion_p): New function. * tree-vrp.c (simplify_cond_using_ranges): Call it. gcc/testsuite/ChangeLog 2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> * gcc.dg/tree-ssa/vrp98.c: New testcase. * gcc.target/avr/uint8-single-reg.c: New testcase. diff --git gcc/testsuite/gcc.dg/tree-ssa/vrp98.c gcc/testsuite/gcc.dg/tree-ssa/vrp98.c new file mode 100644 index 0000000..982f091 --- /dev/null +++ gcc/testsuite/gcc.dg/tree-ssa/vrp98.c @@ -0,0 +1,41 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target int128 } */ +/* { dg-options "-Os -fdump-tree-vrp1-details" } */ + +#include <stdint.h> +#include <limits.h> + +typedef unsigned int word __attribute__((mode(word))); +typedef unsigned __int128 bigger_than_word; + +int +foo (bigger_than_word a, word b, uint8_t c) +{ + /* Must fold use of t1 into use of b, as b is no wider than word_mode. */ + const uint8_t t1 = b % UCHAR_MAX; + + /* Must NOT fold use of t2 into use of a, as a is wider than word_mode. */ + const uint8_t t2 = a % UCHAR_MAX; + + /* Must fold use of t3 into use of c, as c is narrower than t3. */ + const uint32_t t3 = (const uint32_t)(c >> 1); + + uint16_t ret = 0; + + if (t1 == 1) + ret = 20; + else if (t2 == 2) + ret = 30; + else if (t3 == 3) + ret = 40; + /* Th extra condition below is necessary to prevent a prior pass from + folding away the cast. Ignored in scan-tree-dump. */ + else if (t3 == 4) + ret = 50; + + return ret; +} + +/* { dg-final { scan-tree-dump "Folded into: if \\(_\[0-9\]+ == 1\\)" "vrp1" } } */ +/* { dg-final { scan-tree-dump-not "Folded into: if \\(_\[0-9\]+ == 2\\)" "vrp1" } } */ +/* { dg-final { scan-tree-dump "Folded into: if \\(_\[0-9\]+ == 3\\)" "vrp1" } } */ diff --git gcc/testsuite/gcc.target/avr/uint8-single-reg.c gcc/testsuite/gcc.target/avr/uint8-single-reg.c new file mode 100644 index 0000000..291b56c --- /dev/null +++ gcc/testsuite/gcc.target/avr/uint8-single-reg.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-Os" } */ + +/* This testcase verifies that a uint8_t variable assigned from a wider variable + with the same range is held in a single register. VRP must not fold away the + conversion and use two regs to hold the uint16_t - widenings are ok only upto + word mode (1 byte for AVR). +*/ + +unsigned int foo(const unsigned int wvalue) +{ + const unsigned char type = (wvalue >> 8); + unsigned int size = 0; + + if (type == 1) + { + size = 20; + } + return size; +} + +/* { dg-final { scan-assembler "cpi r25,lo8\\(1\\)" } } */ +/* { dg-final { scan-assembler-not "cpc r\\d+,__zero_reg__" } } */ + diff --git gcc/tree-vrp.c gcc/tree-vrp.c index e67048e..6a4ff30 100644 --- gcc/tree-vrp.c +++ gcc/tree-vrp.c @@ -9459,7 +9459,8 @@ simplify_cond_using_ranges (gcond *stmt) innerop = gimple_assign_rhs1 (def_stmt); if (TREE_CODE (innerop) == SSA_NAME - && !POINTER_TYPE_P (TREE_TYPE (innerop))) + && !POINTER_TYPE_P (TREE_TYPE (innerop)) + && desired_pro_or_demotion_p (TREE_TYPE (innerop), TREE_TYPE (op0))) { value_range *vr = get_value_range (innerop); diff --git gcc/tree.h gcc/tree.h index 41c0f7c..cb52deb 100644 --- gcc/tree.h +++ gcc/tree.h @@ -5358,4 +5358,18 @@ get_decl_source_range (tree decl) return get_range_from_loc (line_table, loc); } +/* Return true if it makes sense to promote/demote from_type to to_type. */ +inline bool +desired_pro_or_demotion_p (const_tree to_type, const_tree from_type) +{ + unsigned int to_type_precision = TYPE_PRECISION (to_type); + + /* OK to promote if to_type is no bigger than word_mode. */ + if (to_type_precision <= GET_MODE_PRECISION (word_mode)) + return true; + + /* Otherwise, allow only if narrowing or same precision conversions. */ + return to_type_precision <= TYPE_PRECISION (from_type); +} + #endif /* GCC_TREE_H */ ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-19 6:20 ` Senthil Kumar Selvaraj @ 2015-11-19 17:31 ` Jeff Law 2015-11-20 17:04 ` Senthil Kumar Selvaraj 0 siblings, 1 reply; 13+ messages in thread From: Jeff Law @ 2015-11-19 17:31 UTC (permalink / raw) To: Senthil Kumar Selvaraj, Richard Biener; +Cc: gcc-patches On 11/18/2015 11:20 PM, Senthil Kumar Selvaraj wrote: > On Wed, Nov 18, 2015 at 09:36:21AM +0100, Richard Biener wrote: >> >> Otherwise ok. > > See modified patch below. If you think vrp98.c is unnecessary, feel free > to dump it :). > > If ok, could you commit it for me please? I don't have commit access. > > Regards > Senthil > > gcc/ChangeLog > 2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > * tree.h (desired_pro_or_demotion_p): New function. > * tree-vrp.c (simplify_cond_using_ranges): Call it. > > gcc/testsuite/ChangeLog > 2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > * gcc.dg/tree-ssa/vrp98.c: New testcase. > * gcc.target/avr/uint8-single-reg.c: New testcase. I went ahead and committed this as-is. I do think the vrp98 testcase is useful as it verifies that VRP is doing what we want in a target independent way. It's a good complement to the AVR specific testcase. Thanks, Jeff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-19 17:31 ` Jeff Law @ 2015-11-20 17:04 ` Senthil Kumar Selvaraj 2015-11-20 17:34 ` Jeff Law 0 siblings, 1 reply; 13+ messages in thread From: Senthil Kumar Selvaraj @ 2015-11-20 17:04 UTC (permalink / raw) To: Jeff Law; +Cc: Richard Biener, gcc-patches On Thu, Nov 19, 2015 at 10:31:41AM -0700, Jeff Law wrote: > On 11/18/2015 11:20 PM, Senthil Kumar Selvaraj wrote: > >On Wed, Nov 18, 2015 at 09:36:21AM +0100, Richard Biener wrote: > >> > >>Otherwise ok. > > > >See modified patch below. If you think vrp98.c is unnecessary, feel free > >to dump it :). > > > >If ok, could you commit it for me please? I don't have commit access. > > > >Regards > >Senthil > > > >gcc/ChangeLog > >2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > > > * tree.h (desired_pro_or_demotion_p): New function. > > * tree-vrp.c (simplify_cond_using_ranges): Call it. > > > >gcc/testsuite/ChangeLog > >2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > > > * gcc.dg/tree-ssa/vrp98.c: New testcase. > > * gcc.target/avr/uint8-single-reg.c: New testcase. > I went ahead and committed this as-is. > > I do think the vrp98 testcase is useful as it verifies that VRP is doing > what we want in a target independent way. It's a good complement to the AVR > specific testcase. I see the same problem on gcc-5-branch as well. Would it be ok to backport the fix to that branch as well? Regards Senthil > > Thanks, > Jeff > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-20 17:04 ` Senthil Kumar Selvaraj @ 2015-11-20 17:34 ` Jeff Law 2015-11-23 9:54 ` Richard Biener 0 siblings, 1 reply; 13+ messages in thread From: Jeff Law @ 2015-11-20 17:34 UTC (permalink / raw) To: Senthil Kumar Selvaraj; +Cc: Richard Biener, gcc-patches On 11/20/2015 10:04 AM, Senthil Kumar Selvaraj wrote: > On Thu, Nov 19, 2015 at 10:31:41AM -0700, Jeff Law wrote: >> On 11/18/2015 11:20 PM, Senthil Kumar Selvaraj wrote: >>> On Wed, Nov 18, 2015 at 09:36:21AM +0100, Richard Biener wrote: >>>> >>>> Otherwise ok. >>> >>> See modified patch below. If you think vrp98.c is unnecessary, feel free >>> to dump it :). >>> >>> If ok, could you commit it for me please? I don't have commit access. >>> >>> Regards >>> Senthil >>> >>> gcc/ChangeLog >>> 2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >>> >>> * tree.h (desired_pro_or_demotion_p): New function. >>> * tree-vrp.c (simplify_cond_using_ranges): Call it. >>> >>> gcc/testsuite/ChangeLog >>> 2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >>> >>> * gcc.dg/tree-ssa/vrp98.c: New testcase. >>> * gcc.target/avr/uint8-single-reg.c: New testcase. >> I went ahead and committed this as-is. >> >> I do think the vrp98 testcase is useful as it verifies that VRP is doing >> what we want in a target independent way. It's a good complement to the AVR >> specific testcase. > > I see the same problem on gcc-5-branch as well. Would it be ok to > backport the fix to that branch as well? That's a call for the release managers. I typically don't backport anything expect ICE or incorrect code generation fixes as I tend to be very conservative on what goes onto a release branch. Jakub, Richi or Joseph would need to ack into a release branch. jeff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode 2015-11-20 17:34 ` Jeff Law @ 2015-11-23 9:54 ` Richard Biener 0 siblings, 0 replies; 13+ messages in thread From: Richard Biener @ 2015-11-23 9:54 UTC (permalink / raw) To: Jeff Law; +Cc: Senthil Kumar Selvaraj, gcc-patches On Fri, 20 Nov 2015, Jeff Law wrote: > On 11/20/2015 10:04 AM, Senthil Kumar Selvaraj wrote: > > On Thu, Nov 19, 2015 at 10:31:41AM -0700, Jeff Law wrote: > > > On 11/18/2015 11:20 PM, Senthil Kumar Selvaraj wrote: > > > > On Wed, Nov 18, 2015 at 09:36:21AM +0100, Richard Biener wrote: > > > > > > > > > > Otherwise ok. > > > > > > > > See modified patch below. If you think vrp98.c is unnecessary, feel free > > > > to dump it :). > > > > > > > > If ok, could you commit it for me please? I don't have commit access. > > > > > > > > Regards > > > > Senthil > > > > > > > > gcc/ChangeLog > > > > 2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > > > > > > > * tree.h (desired_pro_or_demotion_p): New function. > > > > * tree-vrp.c (simplify_cond_using_ranges): Call it. > > > > > > > > gcc/testsuite/ChangeLog > > > > 2015-11-19 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > > > > > > > * gcc.dg/tree-ssa/vrp98.c: New testcase. > > > > * gcc.target/avr/uint8-single-reg.c: New testcase. > > > I went ahead and committed this as-is. > > > > > > I do think the vrp98 testcase is useful as it verifies that VRP is doing > > > what we want in a target independent way. It's a good complement to the > > > AVR > > > specific testcase. > > > > I see the same problem on gcc-5-branch as well. Would it be ok to > > backport the fix to that branch as well? > That's a call for the release managers. I typically don't backport anything > expect ICE or incorrect code generation fixes as I tend to be very > conservative on what goes onto a release branch. > > Jakub, Richi or Joseph would need to ack into a release branch. As this is fixes a regression it qualifies in principle. But as it is an optimization regression only I'd prefer to wait a bit to look for fallout. Richard. > jeff > > -- Richard Biener <rguenther@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-11-23 9:46 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-11-14 7:12 [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode Senthil Kumar Selvaraj 2015-11-14 8:13 ` Marc Glisse 2015-11-14 8:50 ` Senthil Kumar Selvaraj 2015-11-14 8:57 ` Richard Biener 2015-11-14 18:11 ` Senthil Kumar Selvaraj 2015-11-16 9:02 ` Richard Biener 2015-11-17 19:53 ` Senthil Kumar Selvaraj 2015-11-18 8:36 ` Richard Biener 2015-11-19 6:20 ` Senthil Kumar Selvaraj 2015-11-19 17:31 ` Jeff Law 2015-11-20 17:04 ` Senthil Kumar Selvaraj 2015-11-20 17:34 ` Jeff Law 2015-11-23 9:54 ` Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).