* [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).