Re: Avoid generating useless range info

[Aldy Hernandez <aldyh@redhat.com>]

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