* Avoid generating useless range info @ 2017-06-14 16:41 Aldy Hernandez 2017-06-16 8:00 ` Richard Biener 0 siblings, 1 reply; 11+ messages in thread From: Aldy Hernandez @ 2017-06-14 16:41 UTC (permalink / raw) To: Richard Biener; +Cc: Andrew MacLeod, gcc-patches [-- Attachment #1: Type: text/plain, Size: 525 bytes --] Hi! As discovered in my range class work, we seem to generate a significant amount of useless range info out of VRP. Is there any reason why we can't avoid generating any range info that spans the entire domain, and yet contains nothing in the non-zero bitmask? The attached patch passes bootstrap, and the one regression it causes is because now the -Walloca-larger-than= pass is better able to determine that there is no range information at all, and the testcase is unbounded. So...win, win. OK for trunk? Aldy [-- Attachment #2: curr --] [-- Type: text/plain, Size: 1751 bytes --] gcc/ * tree-vrp.c (vrp_finalize): Do not expose any useless range information. gcc/testsuite/ * gcc.dg/Walloca-14.c: Adapt test to recognize new complaint of unbounded use. diff --git a/gcc/testsuite/gcc.dg/Walloca-14.c b/gcc/testsuite/gcc.dg/Walloca-14.c index 723dbe5..f3e3f57 100644 --- a/gcc/testsuite/gcc.dg/Walloca-14.c +++ b/gcc/testsuite/gcc.dg/Walloca-14.c @@ -9,5 +9,6 @@ g (int *p) extern void f (void *); void *q = __builtin_alloca (p); /* { dg-warning "passing argument 1" } */ + /* { dg-warning "unbounded use of 'alloca'" "unbounded" { target *-*-* } 11 } */ f (q); } diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 716a7c2..8442b1f 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -10708,8 +10708,24 @@ vrp_finalize (bool warn_array_bounds_p) vr_value[i]->max) == 1))) set_ptr_nonnull (name); else if (!POINTER_TYPE_P (TREE_TYPE (name))) - set_range_info (name, vr_value[i]->type, vr_value[i]->min, - vr_value[i]->max); + { + range_info_def *ri = SSA_NAME_RANGE_INFO (name); + tree type = TREE_TYPE (name); + unsigned precision = TYPE_PRECISION (type); + /* If the range covers the entire domain and there is + nothing in the non-zero bits mask, there is no sense in + storing anything. */ + if (vr_value[i]->min == TYPE_MIN_VALUE (type) + && vr_value[i]->max == TYPE_MAX_VALUE (type) + && vr_value[i]->type == VR_RANGE + && (!ri + || ri->get_nonzero_bits () == wi::shwi (-1, precision))) + SSA_NAME_RANGE_INFO (name) = NULL; + else + set_range_info (name, vr_value[i]->type, vr_value[i]->min, + vr_value[i]->max); + } +} } substitute_and_fold (op_with_constant_singleton_value_range, vrp_fold_stmt); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Avoid generating useless range info 2017-06-14 16:41 Avoid generating useless range info Aldy Hernandez @ 2017-06-16 8:00 ` Richard Biener 2017-06-23 9:02 ` Aldy Hernandez [not found] ` <CAGm3qMXOYa3Km6FGiji0j2txeZJfyiLTR7V6EDMTDEDQo0RWBA@mail.gmail.com> 0 siblings, 2 replies; 11+ messages in thread From: Richard Biener @ 2017-06-16 8:00 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Andrew MacLeod, gcc-patches On Wed, Jun 14, 2017 at 6:41 PM, Aldy Hernandez <aldyh@redhat.com> wrote: > Hi! > > As discovered in my range class work, we seem to generate a significant > amount of useless range info out of VRP. > > Is there any reason why we can't avoid generating any range info that spans > the entire domain, and yet contains nothing in the non-zero bitmask? > > The attached patch passes bootstrap, and the one regression it causes is > because now the -Walloca-larger-than= pass is better able to determine that > there is no range information at all, and the testcase is unbounded. > So...win, win. > > OK for trunk? Can you please do this in set_range_info itself? Thus, if min == wi::min_value && max == wi::max_value simply return? (do not use TYPE_MIN?MAX_VALUE please) Thanks, Richard. > Aldy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Avoid generating useless range info 2017-06-16 8:00 ` Richard Biener @ 2017-06-23 9:02 ` Aldy Hernandez [not found] ` <CAGm3qMXOYa3Km6FGiji0j2txeZJfyiLTR7V6EDMTDEDQo0RWBA@mail.gmail.com> 1 sibling, 0 replies; 11+ messages in thread From: Aldy Hernandez @ 2017-06-23 9:02 UTC (permalink / raw) To: Richard Biener; +Cc: Andrew MacLeod, gcc-patches [one more time, but without sending html which the list refuses :-/] On Fri, Jun 16, 2017 at 4:00 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Wed, Jun 14, 2017 at 6:41 PM, Aldy Hernandez <aldyh@redhat.com> wrote: >> Hi! >> >> As discovered in my range class work, we seem to generate a significant >> amount of useless range info out of VRP. >> >> Is there any reason why we can't avoid generating any range info that spans >> the entire domain, and yet contains nothing in the non-zero bitmask? >> >> The attached patch passes bootstrap, and the one regression it causes is >> because now the -Walloca-larger-than= pass is better able to determine that >> there is no range information at all, and the testcase is unbounded. >> So...win, win. >> >> OK for trunk? > > Can you please do this in set_range_info itself? Thus, if min == > wi::min_value && max == wi::max_value > simply return? (do not use TYPE_MIN?MAX_VALUE please) The reason I did it in vrp_finalize is because if you do it in set_range_info, you break set_nonzero_bits when setting bits on an SSA that currently has no range info: void set_nonzero_bits (tree name, const wide_int_ref &mask) { gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); if (SSA_NAME_RANGE_INFO (name) == NULL) set_range_info (name, VR_RANGE, TYPE_MIN_VALUE (TREE_TYPE (name)), TYPE_MAX_VALUE (TREE_TYPE (name))); range_info_def *ri = SSA_NAME_RANGE_INFO (name); ri->set_nonzero_bits (mask); } Let me know how you'd like me to proceed. Aldy > > Thanks, > Richard. > >> Aldy ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <CAGm3qMXOYa3Km6FGiji0j2txeZJfyiLTR7V6EDMTDEDQo0RWBA@mail.gmail.com>]
* Re: Avoid generating useless range info [not found] ` <CAGm3qMXOYa3Km6FGiji0j2txeZJfyiLTR7V6EDMTDEDQo0RWBA@mail.gmail.com> @ 2017-06-23 10:24 ` Richard Biener 2017-06-23 10:32 ` Jakub Jelinek 2017-06-27 10:26 ` Aldy Hernandez 0 siblings, 2 replies; 11+ messages in thread From: Richard Biener @ 2017-06-23 10:24 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Andrew MacLeod, gcc-patches On Fri, Jun 23, 2017 at 10:59 AM, Aldy Hernandez <aldyh@redhat.com> wrote: > > > On Fri, Jun 16, 2017 at 4:00 AM, Richard Biener <richard.guenther@gmail.com> > wrote: >> >> On Wed, Jun 14, 2017 at 6:41 PM, Aldy Hernandez <aldyh@redhat.com> wrote: >> > Hi! >> > >> > As discovered in my range class work, we seem to generate a significant >> > amount of useless range info out of VRP. >> > >> > Is there any reason why we can't avoid generating any range info that >> > spans >> > the entire domain, and yet contains nothing in the non-zero bitmask? >> > >> > The attached patch passes bootstrap, and the one regression it causes is >> > because now the -Walloca-larger-than= pass is better able to determine >> > that >> > there is no range information at all, and the testcase is unbounded. >> > So...win, win. >> > >> > OK for trunk? >> >> Can you please do this in set_range_info itself? Thus, if min == >> wi::min_value && max == wi::max_value >> simply return? (do not use TYPE_MIN?MAX_VALUE please) > > > The reason I did it in vrp_finalize is because if you do it in > set_range_info, you break set_nonzero_bits when setting bits on an SSA that > currently has no range info: > > void > set_nonzero_bits (tree name, const wide_int_ref &mask) > { > gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); > if (SSA_NAME_RANGE_INFO (name) == NULL) > set_range_info (name, VR_RANGE, > TYPE_MIN_VALUE (TREE_TYPE (name)), > TYPE_MAX_VALUE (TREE_TYPE (name))); > range_info_def *ri = SSA_NAME_RANGE_INFO (name); > ri->set_nonzero_bits (mask); > } > > Let me know how you'd like me to proceed. Just factor out a set_range_info_raw and call that then from here. Richard. > Aldy > >> >> Thanks, >> Richard. >> >> > Aldy > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Avoid generating useless range info 2017-06-23 10:24 ` Richard Biener @ 2017-06-23 10:32 ` Jakub Jelinek 2017-06-23 11:01 ` Richard Biener 2017-06-27 10:26 ` Aldy Hernandez 1 sibling, 1 reply; 11+ messages in thread From: Jakub Jelinek @ 2017-06-23 10:32 UTC (permalink / raw) To: Richard Biener; +Cc: Aldy Hernandez, Andrew MacLeod, gcc-patches On Fri, Jun 23, 2017 at 12:24:25PM +0200, Richard Biener wrote: > > void > > set_nonzero_bits (tree name, const wide_int_ref &mask) > > { > > gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); > > if (SSA_NAME_RANGE_INFO (name) == NULL) > > set_range_info (name, VR_RANGE, > > TYPE_MIN_VALUE (TREE_TYPE (name)), > > TYPE_MAX_VALUE (TREE_TYPE (name))); > > range_info_def *ri = SSA_NAME_RANGE_INFO (name); > > ri->set_nonzero_bits (mask); > > } > > > > Let me know how you'd like me to proceed. > > Just factor out a set_range_info_raw and call that then from here. And don't call it if the mask is all ones. Perhaps set_range_info and set_nonzero_bits even should ggc_free and clear earlier range_info_def if the range is all values and nonzero bit mask is all ones. Or do we share range_info_def between multiple SSA_NAMEs? If yes, of course we shouldn't use ggc_free. Jakub ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Avoid generating useless range info 2017-06-23 10:32 ` Jakub Jelinek @ 2017-06-23 11:01 ` Richard Biener 0 siblings, 0 replies; 11+ messages in thread From: Richard Biener @ 2017-06-23 11:01 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Aldy Hernandez, Andrew MacLeod, gcc-patches On Fri, Jun 23, 2017 at 12:32 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Fri, Jun 23, 2017 at 12:24:25PM +0200, Richard Biener wrote: >> > void >> > set_nonzero_bits (tree name, const wide_int_ref &mask) >> > { >> > gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); >> > if (SSA_NAME_RANGE_INFO (name) == NULL) >> > set_range_info (name, VR_RANGE, >> > TYPE_MIN_VALUE (TREE_TYPE (name)), >> > TYPE_MAX_VALUE (TREE_TYPE (name))); >> > range_info_def *ri = SSA_NAME_RANGE_INFO (name); >> > ri->set_nonzero_bits (mask); >> > } >> > >> > Let me know how you'd like me to proceed. >> >> Just factor out a set_range_info_raw and call that then from here. > > And don't call it if the mask is all ones. Perhaps set_range_info > and set_nonzero_bits even should ggc_free and clear earlier range_info_def > if the range is all values and nonzero bit mask is all ones. > Or do we share range_info_def between multiple SSA_NAMEs? If yes, of course > we shouldn't use ggc_free. We shouldn't as we don't copy on change. We do for points-to but only for the bitmap pointer IIRC. Richard, > > Jakub ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Avoid generating useless range info 2017-06-23 10:24 ` Richard Biener 2017-06-23 10:32 ` Jakub Jelinek @ 2017-06-27 10:26 ` Aldy Hernandez 2017-06-27 10:38 ` Jakub Jelinek 1 sibling, 1 reply; 11+ messages in thread From: Aldy Hernandez @ 2017-06-27 10:26 UTC (permalink / raw) To: Richard Biener; +Cc: Andrew MacLeod, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1832 bytes --] On Fri, Jun 23, 2017 at 6:24 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Fri, Jun 23, 2017 at 10:59 AM, Aldy Hernandez <aldyh@redhat.com> wrote: >> >> >> On Fri, Jun 16, 2017 at 4:00 AM, Richard Biener <richard.guenther@gmail.com> >> wrote: >>> >>> On Wed, Jun 14, 2017 at 6:41 PM, Aldy Hernandez <aldyh@redhat.com> wrote: >>> > Hi! >>> > >>> > As discovered in my range class work, we seem to generate a significant >>> > amount of useless range info out of VRP. >>> > >>> > Is there any reason why we can't avoid generating any range info that >>> > spans >>> > the entire domain, and yet contains nothing in the non-zero bitmask? >>> > >>> > The attached patch passes bootstrap, and the one regression it causes is >>> > because now the -Walloca-larger-than= pass is better able to determine >>> > that >>> > there is no range information at all, and the testcase is unbounded. >>> > So...win, win. >>> > >>> > OK for trunk? >>> >>> Can you please do this in set_range_info itself? Thus, if min == >>> wi::min_value && max == wi::max_value >>> simply return? (do not use TYPE_MIN?MAX_VALUE please) >> >> >> The reason I did it in vrp_finalize is because if you do it in >> set_range_info, you break set_nonzero_bits when setting bits on an SSA that >> currently has no range info: >> >> void >> set_nonzero_bits (tree name, const wide_int_ref &mask) >> { >> gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); >> if (SSA_NAME_RANGE_INFO (name) == NULL) >> set_range_info (name, VR_RANGE, >> TYPE_MIN_VALUE (TREE_TYPE (name)), >> TYPE_MAX_VALUE (TREE_TYPE (name))); >> range_info_def *ri = SSA_NAME_RANGE_INFO (name); >> ri->set_nonzero_bits (mask); >> } >> >> Let me know how you'd like me to proceed. > > Just factor out a set_range_info_raw and call that then from here. How about this? Aldy [-- Attachment #2: curr --] [-- Type: application/octet-stream, Size: 3548 bytes --] gcc/ * tree-ssanames.c (set_range_info_raw): Abstract from ... (set_range_info): ...here. Only call set_range_info_raw if domain is useful. (set_nonzero_bits): Call set_range_info_raw. * tree-ssanames.h (set_range_info_raw): New. gcc/testsuite/ * gcc.dg/Walloca-14.c: Adapt test to recognize new complaint of unbounded use. diff --git a/gcc/testsuite/gcc.dg/Walloca-14.c b/gcc/testsuite/gcc.dg/Walloca-14.c index 723dbe5..f3e3f57 100644 --- a/gcc/testsuite/gcc.dg/Walloca-14.c +++ b/gcc/testsuite/gcc.dg/Walloca-14.c @@ -9,5 +9,6 @@ g (int *p) extern void f (void *); void *q = __builtin_alloca (p); /* { dg-warning "passing argument 1" } */ + /* { dg-warning "unbounded use of 'alloca'" "unbounded" { target *-*-* } 11 } */ f (q); } diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c index 353c7b1..9886a9d 100644 --- a/gcc/tree-ssanames.c +++ b/gcc/tree-ssanames.c @@ -320,11 +320,14 @@ make_ssa_name_fn (struct function *fn, tree var, gimple *stmt, return t; } -/* Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name NAME. */ +/* Helper function for set_range_info. + + Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name + NAME. */ void -set_range_info (tree name, enum value_range_type range_type, - const wide_int_ref &min, const wide_int_ref &max) +set_range_info_raw (tree name, enum value_range_type range_type, + const wide_int_ref &min, const wide_int_ref &max) { gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); gcc_assert (range_type == VR_RANGE || range_type == VR_ANTI_RANGE); @@ -360,6 +363,22 @@ set_range_info (tree name, enum value_range_type range_type, } } +/* Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name + NAME while making sure we don't store useless range info. */ + +void +set_range_info (tree name, enum value_range_type range_type, + const wide_int_ref &min, const wide_int_ref &max) +{ + /* A range of the entire domain is really no range at all. */ + tree type = TREE_TYPE (name); + if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type)) + && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type))) + return; + + set_range_info_raw (name, range_type, min, max); +} + /* Gets range information MIN, MAX and returns enum value_range_type corresponding to tree ssa_name NAME. enum value_range_type returned @@ -419,9 +438,13 @@ set_nonzero_bits (tree name, const wide_int_ref &mask) { gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); if (SSA_NAME_RANGE_INFO (name) == NULL) - set_range_info (name, VR_RANGE, - TYPE_MIN_VALUE (TREE_TYPE (name)), - TYPE_MAX_VALUE (TREE_TYPE (name))); + { + if (mask == -1) + return; + set_range_info_raw (name, VR_RANGE, + TYPE_MIN_VALUE (TREE_TYPE (name)), + TYPE_MAX_VALUE (TREE_TYPE (name))); + } range_info_def *ri = SSA_NAME_RANGE_INFO (name); ri->set_nonzero_bits (mask); } diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h index 9a18394..f7e032f 100644 --- a/gcc/tree-ssanames.h +++ b/gcc/tree-ssanames.h @@ -69,6 +69,9 @@ struct GTY ((variable_size)) range_info_def { /* Sets the value range to SSA. */ extern void set_range_info (tree, enum value_range_type, const wide_int_ref &, const wide_int_ref &); +extern void set_range_info_raw (tree, enum value_range_type, + const wide_int_ref &, + const wide_int_ref &); /* Gets the value range from SSA. */ extern enum value_range_type get_range_info (const_tree, wide_int *, wide_int *); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Avoid generating useless range info 2017-06-27 10:26 ` Aldy Hernandez @ 2017-06-27 10:38 ` Jakub Jelinek 2017-06-28 7:56 ` Aldy Hernandez 0 siblings, 1 reply; 11+ messages in thread From: Jakub Jelinek @ 2017-06-27 10:38 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Richard Biener, Andrew MacLeod, gcc-patches On Tue, Jun 27, 2017 at 06:26:46AM -0400, Aldy Hernandez wrote: > How about this? @@ -360,6 +363,22 @@ set_range_info (tree name, enum value_range_type range_type, } } +/* Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name + NAME while making sure we don't store useless range info. */ + +void +set_range_info (tree name, enum value_range_type range_type, + const wide_int_ref &min, const wide_int_ref &max) +{ + /* A range of the entire domain is really no range at all. */ + tree type = TREE_TYPE (name); + if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type)) + && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type))) + return; + + set_range_info_raw (name, range_type, min, max); +} + Won't this misbehave if we have a narrower range on some SSA_NAME and call set_range_info to make it VARYING? In that case (i.e. SSA_NAME_RANGE_INFO (name) != NULL), we should either set_range_info_raw too (if nonzero_bits is not all ones) or clear SSA_NAME_RANGE_INFO (otherwise). /* Gets range information MIN, MAX and returns enum value_range_type corresponding to tree ssa_name NAME. enum value_range_type returned @@ -419,9 +438,13 @@ set_nonzero_bits (tree name, const wide_int_ref &mask) { gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); if (SSA_NAME_RANGE_INFO (name) == NULL) - set_range_info (name, VR_RANGE, - TYPE_MIN_VALUE (TREE_TYPE (name)), - TYPE_MAX_VALUE (TREE_TYPE (name))); + { + if (mask == -1) + return; + set_range_info_raw (name, VR_RANGE, + TYPE_MIN_VALUE (TREE_TYPE (name)), + TYPE_MAX_VALUE (TREE_TYPE (name))); + } range_info_def *ri = SSA_NAME_RANGE_INFO (name); ri->set_nonzero_bits (mask); Similarly, if SSA_NAME_RANGE_INFO is previously non-NULL, but min/max are VARYING and the new mask is -1, shouldn't we free it rather than set it to the default? If we consider the cases rare enough to worry about, at least your above if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type)) && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type))) should be gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); if (SSA_NAME_RANGE_INFO (name) == NULL && min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type)) && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type))) We'd then not misbehave, just might in some rare cases keep SSA_NAME_RANGE_INFO non-NULL even if it contains the default stuff. Jakub ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Avoid generating useless range info 2017-06-27 10:38 ` Jakub Jelinek @ 2017-06-28 7:56 ` Aldy Hernandez 2017-06-29 9:53 ` Richard Biener 2017-08-02 13:29 ` [testsuite, committed] Use relative line number in gcc.dg/Walloca-14.c Tom de Vries 0 siblings, 2 replies; 11+ messages in thread From: Aldy Hernandez @ 2017-06-28 7:56 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Richard Biener, Andrew MacLeod, gcc-patches [-- Attachment #1: Type: text/plain, Size: 2357 bytes --] On 06/27/2017 06:38 AM, Jakub Jelinek wrote: > On Tue, Jun 27, 2017 at 06:26:46AM -0400, Aldy Hernandez wrote: >> How about this? > > @@ -360,6 +363,22 @@ set_range_info (tree name, enum value_range_type range_type, > } > } > > +/* Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name > + NAME while making sure we don't store useless range info. */ > + > +void > +set_range_info (tree name, enum value_range_type range_type, > + const wide_int_ref &min, const wide_int_ref &max) > +{ > + /* A range of the entire domain is really no range at all. */ > + tree type = TREE_TYPE (name); > + if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type)) > + && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type))) > + return; > + > + set_range_info_raw (name, range_type, min, max); > +} > + > > Won't this misbehave if we have a narrower range on some SSA_NAME and > call set_range_info to make it VARYING? > In that case (i.e. SSA_NAME_RANGE_INFO (name) != NULL), we should either > set_range_info_raw too (if nonzero_bits is not all ones) or clear > SSA_NAME_RANGE_INFO (otherwise). Good point. Fixed. > > /* Gets range information MIN, MAX and returns enum value_range_type > corresponding to tree ssa_name NAME. enum value_range_type returned > @@ -419,9 +438,13 @@ set_nonzero_bits (tree name, const wide_int_ref &mask) > { > gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); > if (SSA_NAME_RANGE_INFO (name) == NULL) > - set_range_info (name, VR_RANGE, > - TYPE_MIN_VALUE (TREE_TYPE (name)), > - TYPE_MAX_VALUE (TREE_TYPE (name))); > + { > + if (mask == -1) > + return; > + set_range_info_raw (name, VR_RANGE, > + TYPE_MIN_VALUE (TREE_TYPE (name)), > + TYPE_MAX_VALUE (TREE_TYPE (name))); > + } > range_info_def *ri = SSA_NAME_RANGE_INFO (name); > ri->set_nonzero_bits (mask); > > Similarly, if SSA_NAME_RANGE_INFO is previously non-NULL, but min/max > are VARYING and the new mask is -1, shouldn't we free it rather than > set it to the default? Here, if SSA_NAME_RANGE_INFO is previously non-NULL then we proceed as always-- just set the nonzero bits to whatever was specified (without clearning SSA_NAME_RANGE_INFO). A mask of -1 and an SSA_NAME_RANGE_INFO of non-NULL can coexist just fine. How about this? Aldy [-- Attachment #2: curr --] [-- Type: text/plain, Size: 3812 bytes --] gcc/ * tree-ssanames.c (set_range_info_raw): Abstract from ... (set_range_info): ...here. Only call set_range_info_raw if domain is useful. (set_nonzero_bits): Call set_range_info_raw. * tree-ssanames.h (set_range_info_raw): New. gcc/testsuite/ * gcc.dg/Walloca-14.c: Adapt test to recognize new complaint of unbounded use. diff --git a/gcc/testsuite/gcc.dg/Walloca-14.c b/gcc/testsuite/gcc.dg/Walloca-14.c index 723dbe5..f3e3f57 100644 --- a/gcc/testsuite/gcc.dg/Walloca-14.c +++ b/gcc/testsuite/gcc.dg/Walloca-14.c @@ -9,5 +9,6 @@ g (int *p) extern void f (void *); void *q = __builtin_alloca (p); /* { dg-warning "passing argument 1" } */ + /* { dg-warning "unbounded use of 'alloca'" "unbounded" { target *-*-* } 11 } */ f (q); } diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c index 353c7b1..0053b01 100644 --- a/gcc/tree-ssanames.c +++ b/gcc/tree-ssanames.c @@ -320,11 +320,14 @@ make_ssa_name_fn (struct function *fn, tree var, gimple *stmt, return t; } -/* Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name NAME. */ +/* Helper function for set_range_info. + + Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name + NAME. */ void -set_range_info (tree name, enum value_range_type range_type, - const wide_int_ref &min, const wide_int_ref &max) +set_range_info_raw (tree name, enum value_range_type range_type, + const wide_int_ref &min, const wide_int_ref &max) { gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); gcc_assert (range_type == VR_RANGE || range_type == VR_ANTI_RANGE); @@ -360,6 +363,34 @@ set_range_info (tree name, enum value_range_type range_type, } } +/* Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name + NAME while making sure we don't store useless range info. */ + +void +set_range_info (tree name, enum value_range_type range_type, + const wide_int_ref &min, const wide_int_ref &max) +{ + gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); + + /* A range of the entire domain is really no range at all. */ + tree type = TREE_TYPE (name); + if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type)) + && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type))) + { + range_info_def *ri = SSA_NAME_RANGE_INFO (name); + if (ri == NULL) + return; + if (ri->get_nonzero_bits () == -1) + { + ggc_free (ri); + SSA_NAME_RANGE_INFO (name) = NULL; + return; + } + } + + set_range_info_raw (name, range_type, min, max); +} + /* Gets range information MIN, MAX and returns enum value_range_type corresponding to tree ssa_name NAME. enum value_range_type returned @@ -419,9 +450,13 @@ set_nonzero_bits (tree name, const wide_int_ref &mask) { gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); if (SSA_NAME_RANGE_INFO (name) == NULL) - set_range_info (name, VR_RANGE, - TYPE_MIN_VALUE (TREE_TYPE (name)), - TYPE_MAX_VALUE (TREE_TYPE (name))); + { + if (mask == -1) + return; + set_range_info_raw (name, VR_RANGE, + TYPE_MIN_VALUE (TREE_TYPE (name)), + TYPE_MAX_VALUE (TREE_TYPE (name))); + } range_info_def *ri = SSA_NAME_RANGE_INFO (name); ri->set_nonzero_bits (mask); } diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h index 9a18394..f7e032f 100644 --- a/gcc/tree-ssanames.h +++ b/gcc/tree-ssanames.h @@ -69,6 +69,9 @@ struct GTY ((variable_size)) range_info_def { /* Sets the value range to SSA. */ extern void set_range_info (tree, enum value_range_type, const wide_int_ref &, const wide_int_ref &); +extern void set_range_info_raw (tree, enum value_range_type, + const wide_int_ref &, + const wide_int_ref &); /* Gets the value range from SSA. */ extern enum value_range_type get_range_info (const_tree, wide_int *, wide_int *); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Avoid generating useless range info 2017-06-28 7:56 ` Aldy Hernandez @ 2017-06-29 9:53 ` Richard Biener 2017-08-02 13:29 ` [testsuite, committed] Use relative line number in gcc.dg/Walloca-14.c Tom de Vries 1 sibling, 0 replies; 11+ messages in thread From: Richard Biener @ 2017-06-29 9:53 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Jakub Jelinek, Andrew MacLeod, gcc-patches On Wed, Jun 28, 2017 at 9:56 AM, Aldy Hernandez <aldyh@redhat.com> wrote: > > > On 06/27/2017 06:38 AM, Jakub Jelinek wrote: >> >> On Tue, Jun 27, 2017 at 06:26:46AM -0400, Aldy Hernandez wrote: >>> >>> How about this? >> >> >> @@ -360,6 +363,22 @@ set_range_info (tree name, enum value_range_type >> range_type, >> } >> } >> +/* Store range information RANGE_TYPE, MIN, and MAX to tree ssa_name >> + NAME while making sure we don't store useless range info. */ >> + >> +void >> +set_range_info (tree name, enum value_range_type range_type, >> + const wide_int_ref &min, const wide_int_ref &max) >> +{ >> + /* A range of the entire domain is really no range at all. */ >> + tree type = TREE_TYPE (name); >> + if (min == wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type)) >> + && max == wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type))) >> + return; >> + >> + set_range_info_raw (name, range_type, min, max); >> +} >> + >> >> Won't this misbehave if we have a narrower range on some SSA_NAME and >> call set_range_info to make it VARYING? >> In that case (i.e. SSA_NAME_RANGE_INFO (name) != NULL), we should either >> set_range_info_raw too (if nonzero_bits is not all ones) or clear >> SSA_NAME_RANGE_INFO (otherwise). > > > Good point. Fixed. > >> /* Gets range information MIN, MAX and returns enum value_range_type >> corresponding to tree ssa_name NAME. enum value_range_type returned >> @@ -419,9 +438,13 @@ set_nonzero_bits (tree name, const wide_int_ref >> &mask) >> { >> gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name))); >> if (SSA_NAME_RANGE_INFO (name) == NULL) >> - set_range_info (name, VR_RANGE, >> - TYPE_MIN_VALUE (TREE_TYPE (name)), >> - TYPE_MAX_VALUE (TREE_TYPE (name))); >> + { >> + if (mask == -1) >> + return; >> + set_range_info_raw (name, VR_RANGE, >> + TYPE_MIN_VALUE (TREE_TYPE (name)), >> + TYPE_MAX_VALUE (TREE_TYPE (name))); >> + } >> range_info_def *ri = SSA_NAME_RANGE_INFO (name); >> ri->set_nonzero_bits (mask); >> >> Similarly, if SSA_NAME_RANGE_INFO is previously non-NULL, but min/max >> are VARYING and the new mask is -1, shouldn't we free it rather than >> set it to the default? > > > Here, if SSA_NAME_RANGE_INFO is previously non-NULL then we proceed as > always-- just set the nonzero bits to whatever was specified (without > clearning SSA_NAME_RANGE_INFO). A mask of -1 and an SSA_NAME_RANGE_INFO of > non-NULL can coexist just fine. > > How about this? Ok. Thanks, Richard. > Aldy ^ permalink raw reply [flat|nested] 11+ messages in thread
* [testsuite, committed] Use relative line number in gcc.dg/Walloca-14.c 2017-06-28 7:56 ` Aldy Hernandez 2017-06-29 9:53 ` Richard Biener @ 2017-08-02 13:29 ` Tom de Vries 1 sibling, 0 replies; 11+ messages in thread From: Tom de Vries @ 2017-08-02 13:29 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Jakub Jelinek, Richard Biener, Andrew MacLeod, gcc-patches [-- Attachment #1: Type: text/plain, Size: 338 bytes --] [ was: Re: Avoid generating useless range info ] On 06/28/2017 09:56 AM, Aldy Hernandez wrote: > void *q = __builtin_alloca (p); /* { dg-warning "passing argument 1" } */ > + /* { dg-warning "unbounded use of 'alloca'" "unbounded" { target *-*-* } 11 } */ Hi, Please use relative line number next time. Committed. Thanks, - Tom [-- Attachment #2: 0001-Use-relative-line-number-in-gcc.dg-Walloca-14.c.patch --] [-- Type: text/x-patch, Size: 752 bytes --] Use relative line number in gcc.dg/Walloca-14.c 2017-08-02 Tom de Vries <tom@codesourcery.com> * gcc.dg/Walloca-14.c: Use relative line number. --- gcc/testsuite/gcc.dg/Walloca-14.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.dg/Walloca-14.c b/gcc/testsuite/gcc.dg/Walloca-14.c index f3e3f57..ea48227 100644 --- a/gcc/testsuite/gcc.dg/Walloca-14.c +++ b/gcc/testsuite/gcc.dg/Walloca-14.c @@ -9,6 +9,6 @@ g (int *p) extern void f (void *); void *q = __builtin_alloca (p); /* { dg-warning "passing argument 1" } */ - /* { dg-warning "unbounded use of 'alloca'" "unbounded" { target *-*-* } 11 } */ + /* { dg-warning "unbounded use of 'alloca'" "unbounded" { target *-*-* } .-1 } */ f (q); } ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-08-02 13:29 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-14 16:41 Avoid generating useless range info Aldy Hernandez
2017-06-16 8:00 ` Richard Biener
2017-06-23 9:02 ` Aldy Hernandez
[not found] ` <CAGm3qMXOYa3Km6FGiji0j2txeZJfyiLTR7V6EDMTDEDQo0RWBA@mail.gmail.com>
2017-06-23 10:24 ` Richard Biener
2017-06-23 10:32 ` Jakub Jelinek
2017-06-23 11:01 ` Richard Biener
2017-06-27 10:26 ` Aldy Hernandez
2017-06-27 10:38 ` Jakub Jelinek
2017-06-28 7:56 ` Aldy Hernandez
2017-06-29 9:53 ` Richard Biener
2017-08-02 13:29 ` [testsuite, committed] Use relative line number in gcc.dg/Walloca-14.c Tom de Vries
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).