value-range: Fix handling of POLY_INT_CST anti-ranges [PR96146]

The range infrastructure has code to decompose POLY_INT_CST ranges
to worst-case integer bounds.  However, it had the fundamental flaw
(obvious in hindsight) that it applied to anti-ranges too, meaning
that a range 2+2X would end up with a range of ~[2, +INF], i.e.
[-INF, 1].  This patch decays to varying in that case instead.

I'm still a bit uneasy about this.  ISTM that in terms of
generality:

  SSA_NAME => POLY_INT_CST => INTEGER_CST
           => ADDR_EXPR

I.e. an SSA_NAME could store a POLY_INT_CST and a POLY_INT_CST
could store an INTEGER_CST (before canonicalisation).  POLY_INT_CST
is also “as constant as” ADDR_EXPR (well, OK, only some ADDR_EXPRs
are run-time rather than link-time constants, whereas all POLY_INT_CSTs
are, but still).  So it seems like we should at least be able to treat
POLY_INT_CST as symbolic.  On the other hand, I don't have any examples
in which that would be useful.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK for trunk and
GCC 10?

Richard


gcc/
	PR tree-optimization/96146
	* value-range.cc (value_range::set): Only decompose POLY_INT_CST
	bounds to integers for VR_RANGE.  Decay to VR_VARYING for anti-ranges
	involving POLY_INT_CSTs.

gcc/testsuite/
	PR tree-optimization/96146
	* gcc.target/aarch64/sve/acle/general/pr96146.c: New test.
---
 .../aarch64/sve/acle/general/pr96146.c        | 20 ++++++++
 gcc/value-range.cc                            | 47 +++++++++++--------
 2 files changed, 48 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96146.c

On July 11, 2020 10:01:41 AM GMT+02:00, Richard Sandiford <richard.sandiford@arm.com> wrote:
>The range infrastructure has code to decompose POLY_INT_CST ranges
>to worst-case integer bounds.  However, it had the fundamental flaw
>(obvious in hindsight) that it applied to anti-ranges too, meaning
>that a range 2+2X would end up with a range of ~[2, +INF], i.e.
>[-INF, 1].  This patch decays to varying in that case instead.
>
>I'm still a bit uneasy about this.  ISTM that in terms of
>generality:
>
>  SSA_NAME => POLY_INT_CST => INTEGER_CST
>           => ADDR_EXPR
>
>I.e. an SSA_NAME could store a POLY_INT_CST and a POLY_INT_CST
>could store an INTEGER_CST (before canonicalisation).  POLY_INT_CST
>is also “as constant as” ADDR_EXPR (well, OK, only some ADDR_EXPRs
>are run-time rather than link-time constants, whereas all POLY_INT_CSTs
>are, but still).  So it seems like we should at least be able to treat
>POLY_INT_CST as symbolic.  On the other hand, I don't have any examples
>in which that would be useful.
>
>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK for trunk and
>GCC 10?

OK. 

Thanks, 
Richard. 

>Richard
>
>
>gcc/
>	PR tree-optimization/96146
>	* value-range.cc (value_range::set): Only decompose POLY_INT_CST
>	bounds to integers for VR_RANGE.  Decay to VR_VARYING for anti-ranges
>	involving POLY_INT_CSTs.
>
>gcc/testsuite/
>	PR tree-optimization/96146
>	* gcc.target/aarch64/sve/acle/general/pr96146.c: New test.
>---
> .../aarch64/sve/acle/general/pr96146.c        | 20 ++++++++
> gcc/value-range.cc                            | 47 +++++++++++--------
> 2 files changed, 48 insertions(+), 19 deletions(-)
>create mode 100644
>gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96146.c
>
>diff --git
>a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96146.c
>b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96146.c
>new file mode 100644
>index 00000000000..b05fac4918d
>--- /dev/null
>+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96146.c
>@@ -0,0 +1,20 @@
>+/* { dg-do run { target aarch64_sve_hw } } */
>+/* { dg-options "-O3" } */
>+
>+#include <arm_sve.h>
>+
>+void __attribute__ ((noipa))
>+f (volatile int *x)
>+{
>+  int i;
>+  for (int i = 0; i < svcntd (); ++i)
>+    *x = i;
>+}
>+
>+int
>+main (void)
>+{
>+  volatile int x;
>+  f (&x);
>+  return 0;
>+}
>diff --git a/gcc/value-range.cc b/gcc/value-range.cc
>index 929a6b59aaf..bc4b061da57 100644
>--- a/gcc/value-range.cc
>+++ b/gcc/value-range.cc
>@@ -86,7 +86,34 @@ value_range::set (tree min, tree max,
>value_range_kind kind)
>       set_undefined ();
>       return;
>     }
>-  else if (kind == VR_VARYING)
>+
>+  if (kind == VR_RANGE)
>+    {
>+      /* Convert POLY_INT_CST bounds into worst-case INTEGER_CST
>bounds.  */
>+      if (POLY_INT_CST_P (min))
>+	{
>+	  tree type_min = vrp_val_min (TREE_TYPE (min));
>+	  widest_int lb
>+	    = constant_lower_bound_with_limit (wi::to_poly_widest (min),
>+					       wi::to_widest (type_min));
>+	  min = wide_int_to_tree (TREE_TYPE (min), lb);
>+	}
>+      if (POLY_INT_CST_P (max))
>+	{
>+	  tree type_max = vrp_val_max (TREE_TYPE (max));
>+	  widest_int ub
>+	    = constant_upper_bound_with_limit (wi::to_poly_widest (max),
>+					       wi::to_widest (type_max));
>+	  max = wide_int_to_tree (TREE_TYPE (max), ub);
>+	}
>+    }
>+  else if (kind != VR_VARYING)
>+    {
>+      if (POLY_INT_CST_P (min) || POLY_INT_CST_P (max))
>+	kind = VR_VARYING;
>+    }
>+
>+  if (kind == VR_VARYING)
>     {
>       gcc_assert (TREE_TYPE (min) == TREE_TYPE (max));
>       tree typ = TREE_TYPE (min);
>@@ -99,24 +126,6 @@ value_range::set (tree min, tree max,
>value_range_kind kind)
>       return;
>     }
> 
>-  /* Convert POLY_INT_CST bounds into worst-case INTEGER_CST bounds. 
>*/
>-  if (POLY_INT_CST_P (min))
>-    {
>-      tree type_min = vrp_val_min (TREE_TYPE (min));
>-      widest_int lb
>-	= constant_lower_bound_with_limit (wi::to_poly_widest (min),
>-					   wi::to_widest (type_min));
>-      min = wide_int_to_tree (TREE_TYPE (min), lb);
>-    }
>-  if (POLY_INT_CST_P (max))
>-    {
>-      tree type_max = vrp_val_max (TREE_TYPE (max));
>-      widest_int ub
>-	= constant_upper_bound_with_limit (wi::to_poly_widest (max),
>-					   wi::to_widest (type_max));
>-      max = wide_int_to_tree (TREE_TYPE (max), ub);
>-    }
>-
>   /* Nothing to canonicalize for symbolic ranges.  */
>   if (TREE_CODE (min) != INTEGER_CST
>       || TREE_CODE (max) != INTEGER_CST)