Avoid generating useless range info

Avoid generating useless range info

[Aldy Hernandez]

[-- 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);

Re: Avoid generating useless range info

[Richard Biener]

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

Re: Avoid generating useless range info

[Aldy Hernandez]

[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

Re: Avoid generating useless range info

[Richard Biener]

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
>
>

Re: Avoid generating useless range info

[Jakub Jelinek]

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

Re: Avoid generating useless range info

[Richard Biener]

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

Re: Avoid generating useless range info

[Aldy Hernandez]

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

Re: Avoid generating useless range info

[Jakub Jelinek]

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

Re: Avoid generating useless range info

[Aldy Hernandez]

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

Re: Avoid generating useless range info

[Richard Biener]

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

[testsuite, committed] Use relative line number in gcc.dg/Walloca-14.c

[Tom de Vries]

[-- 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);
 }