[COMMITTED] Make irange dependency explicit for range_of_ssa_name_with_loop_info.

[COMMITTED] Make irange dependency explicit for range_of_ssa_name_with_loop_info.

[Aldy Hernandez]

Even though ranger is type agnostic, SCEV seems to only work with
integers.  This patch removes some FIXME notes making it explicit that
bounds_of_var_in_loop only works with iranges.

Tested on x86-64 Linux.

gcc/ChangeLog:

	* gimple-range-fold.cc (fold_using_range::range_of_phi): Only
	query SCEV for integers.
	(fold_using_range::range_of_ssa_name_with_loop_info): Remove
	irange check.
---
 gcc/gimple-range-fold.cc | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 6f907df5bf5..7665c954f2b 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -853,12 +853,14 @@ fold_using_range::range_of_phi (vrange &r, gphi *phi, fur_source &src)
       }
 
   // If SCEV is available, query if this PHI has any knonwn values.
-  if (scev_initialized_p () && !POINTER_TYPE_P (TREE_TYPE (phi_def)))
+  if (scev_initialized_p ()
+      && !POINTER_TYPE_P (TREE_TYPE (phi_def))
+      && irange::supports_p (TREE_TYPE (phi_def)))
     {
-      value_range loop_range;
       class loop *l = loop_containing_stmt (phi);
       if (l && loop_outer (l))
 	{
+	  int_range_max loop_range;
 	  range_of_ssa_name_with_loop_info (loop_range, phi_def, l, phi, src);
 	  if (!loop_range.varying_p ())
 	    {
@@ -1337,9 +1339,7 @@ fold_using_range::range_of_ssa_name_with_loop_info (irange &r, tree name,
 {
   gcc_checking_assert (TREE_CODE (name) == SSA_NAME);
   tree min, max, type = TREE_TYPE (name);
-  // FIXME: Remove the supports_p() once all this can handle floats, etc.
-  if (irange::supports_p (type)
-      && bounds_of_var_in_loop (&min, &max, src.query (), l, phi, name))
+  if (bounds_of_var_in_loop (&min, &max, src.query (), l, phi, name))
     {
       if (TREE_CODE (min) != INTEGER_CST)
 	{
-- 
2.37.1

[COMMITTED] const_tree conversion of vrange::supports_*

[Aldy Hernandez]

Make all vrange::supports_*_p methods const_tree as they can end up
being called from functions that are const_tree.

Tested on x86-64 Linux.

gcc/ChangeLog:

	* value-range.cc (vrange::supports_type_p): Use const_tree.
	(irange::supports_type_p): Same.
	(frange::supports_type_p): Same.
	* value-range.h (Value_Range::supports_type_p): Same.
	(irange::supports_p): Same.
---
 gcc/value-range.cc |  6 +++---
 gcc/value-range.h  | 16 ++++++++--------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 2923f4f5a0e..7adbf55c6a6 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -105,7 +105,7 @@ vrange::type () const
 }
 
 bool
-vrange::supports_type_p (tree) const
+vrange::supports_type_p (const_tree) const
 {
   return false;
 }
@@ -229,7 +229,7 @@ vrange::dump (FILE *file) const
 }
 
 bool
-irange::supports_type_p (tree type) const
+irange::supports_type_p (const_tree type) const
 {
   return supports_p (type);
 }
@@ -416,7 +416,7 @@ frange::operator== (const frange &src) const
 }
 
 bool
-frange::supports_type_p (tree type) const
+frange::supports_type_p (const_tree type) const
 {
   return supports_p (type);
 }
diff --git a/gcc/value-range.h b/gcc/value-range.h
index e43fbe30f27..c6ab955c407 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -78,7 +78,7 @@ public:
   virtual void accept (const class vrange_visitor &v) const = 0;
   virtual void set (tree, tree, value_range_kind = VR_RANGE);
   virtual tree type () const;
-  virtual bool supports_type_p (tree type) const;
+  virtual bool supports_type_p (const_tree type) const;
   virtual void set_varying (tree type);
   virtual void set_undefined ();
   virtual bool union_ (const vrange &);
@@ -122,8 +122,8 @@ public:
   virtual void set_undefined () override;
 
   // Range types.
-  static bool supports_p (tree type);
-  virtual bool supports_type_p (tree type) const override;
+  static bool supports_p (const_tree type);
+  virtual bool supports_type_p (const_tree type) const override;
   virtual tree type () const override;
 
   // Iteration over sub-ranges.
@@ -336,7 +336,7 @@ class frange : public vrange
 public:
   frange ();
   frange (const frange &);
-  static bool supports_p (tree type)
+  static bool supports_p (const_tree type)
   {
     // Disabled until floating point range-ops come live.
     return 0 && SCALAR_FLOAT_TYPE_P (type);
@@ -347,7 +347,7 @@ public:
   virtual void set_undefined () override;
   virtual bool union_ (const vrange &) override;
   virtual bool intersect (const vrange &) override;
-  virtual bool supports_type_p (tree type) const override;
+  virtual bool supports_type_p (const_tree type) const override;
   virtual void accept (const vrange_visitor &v) const override;
   frange& operator= (const frange &);
   bool operator== (const frange &) const;
@@ -457,7 +457,7 @@ public:
   operator vrange &();
   operator const vrange &() const;
   void dump (FILE *) const;
-  static bool supports_type_p (tree type);
+  static bool supports_type_p (const_tree type);
 
   // Convenience methods for vrange compatability.
   void set (tree min, tree max, value_range_kind kind = VR_RANGE)
@@ -588,7 +588,7 @@ Value_Range::operator const vrange &() const
 // Return TRUE if TYPE is supported by the vrange infrastructure.
 
 inline bool
-Value_Range::supports_type_p (tree type)
+Value_Range::supports_type_p (const_tree type)
 {
   return irange::supports_p (type) || frange::supports_p (type);
 }
@@ -730,7 +730,7 @@ irange::nonzero_p () const
 }
 
 inline bool
-irange::supports_p (tree type)
+irange::supports_p (const_tree type)
 {
   return INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type);
 }
-- 
2.37.1

[COMMITTED] Cleanups to frange.

[Aldy Hernandez]

These are some assorted cleanups to the frange class to make it easier
to drop in an implementation with FP endpoints:

* frange::set() had some asserts limiting the type of arguments
  passed.  There's no reason why we can't handle all the variants.
  Worse comes to worse, we can always return a VARYING which is
  conservative and correct.

* frange::normalize_kind() now returns a boolean that can be used in
  union and intersection to indicate that the range changed.

* Implement vrp_val_max and vrp_val_min for floats.  Also, move them
  earlier in the header file so frange can use them.

Tested on x86-64 Linux.

gcc/ChangeLog:

	* value-range.cc (tree_compare): New.
	(frange::set): Make more general.
	(frange::normalize_kind): Cleanup and return bool.
	(frange::union_): Use normalize_kind return value.
	(frange::intersect): Same.
	(frange::verify_range): Remove unnecessary else.
	* value-range.h (vrp_val_max): Move before frange class.
	(vrp_val_min): Same.
	(frange::frange): Remove set to m_type.
---
 gcc/value-range.cc | 102 +++++++++++++++++++++++++++++----------------
 gcc/value-range.h  |  70 ++++++++++++++++++-------------
 2 files changed, 105 insertions(+), 67 deletions(-)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 7adbf55c6a6..dc06f8b0078 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -260,66 +260,93 @@ frange::accept (const vrange_visitor &v) const
   v.visit (*this);
 }
 
-// Setter for franges.  Currently only singletons are supported.
+// Helper function to compare floats.  Returns TRUE if op1 .CODE. op2
+// is nonzero.
+
+static inline bool
+tree_compare (tree_code code, tree op1, tree op2)
+{
+  return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2));
+}
+
+// Setter for franges.
 
 void
 frange::set (tree min, tree max, value_range_kind kind)
 {
-  gcc_checking_assert (kind == VR_RANGE);
-  gcc_checking_assert (operand_equal_p (min, max));
   gcc_checking_assert (TREE_CODE (min) == REAL_CST);
+  gcc_checking_assert (TREE_CODE (max) == REAL_CST);
+
+  if (kind == VR_UNDEFINED)
+    {
+      set_undefined ();
+      return;
+    }
+
+  // Treat VR_ANTI_RANGE and VR_VARYING as varying.
+  if (kind != VR_RANGE)
+    {
+      set_varying (TREE_TYPE (min));
+      return;
+    }
 
   m_kind = kind;
   m_type = TREE_TYPE (min);
 
-  REAL_VALUE_TYPE *const cst = TREE_REAL_CST_PTR (min);
-  if (real_isnan (cst))
-    m_props.nan_set_yes ();
-  else
-    m_props.nan_set_no ();
-
-  if (real_isinf (cst))
+  // Mark NANness.
+  if (real_isnan (TREE_REAL_CST_PTR (min))
+      || real_isnan (TREE_REAL_CST_PTR (max)))
     {
-      if (real_isneg (cst))
-	{
-	  m_props.inf_set_no ();
-	  m_props.ninf_set_yes ();
-	}
-      else
-	{
-	  m_props.inf_set_yes ();
-	  m_props.ninf_set_no ();
-	}
+      gcc_checking_assert (operand_equal_p (min, max));
+      m_props.nan_set_yes ();
     }
   else
+    m_props.nan_set_no ();
+
+  bool is_min = vrp_val_is_min (min);
+  bool is_max = vrp_val_is_max (max);
+
+  // Mark when the endpoints can't be INF.
+  if (!is_min)
+    m_props.ninf_set_no ();
+  if (!is_max)
+    m_props.inf_set_no ();
+
+  // Mark when the endpoints are definitely INF.
+  if (operand_equal_p (min, max))
     {
-      m_props.inf_set_no ();
-      m_props.ninf_set_no ();
+      if (is_min)
+	m_props.ninf_set_yes ();
+      else if (is_max)
+	m_props.inf_set_yes ();
     }
 
+  // Check for swapped ranges.
+  gcc_checking_assert (m_props.nan_yes_p ()
+		       || tree_compare (LE_EXPR, min, max));
+
   if (flag_checking)
     verify_range ();
 }
 
-// Normalize range to VARYING or UNDEFINED, or vice versa.
+// Normalize range to VARYING or UNDEFINED, or vice versa.  Return
+// TRUE if anything changed.
 //
 // A range with no known properties can be dropped to VARYING.
 // Similarly, a VARYING with any properties should be dropped to a
 // VR_RANGE.  Normalizing ranges upon changing them ensures there is
 // only one representation for a given range.
 
-void
+bool
 frange::normalize_kind ()
 {
   if (m_kind == VR_RANGE)
     {
       // No FP properties set means varying.
-      if (m_props.nan_varying_p ()
-	  && m_props.inf_varying_p ()
-	  && m_props.ninf_varying_p ())
+      if (m_props.varying_p ())
 	{
 	  set_varying (m_type);
-	  return;
+	  return true;
 	}
       // Undefined is viral.
       if (m_props.nan_undefined_p ()
@@ -327,17 +354,19 @@ frange::normalize_kind ()
 	  || m_props.ninf_undefined_p ())
 	{
 	  set_undefined ();
-	  return;
+	  return true;
 	}
     }
   else if (m_kind == VR_VARYING)
     {
       // If a VARYING has any FP properties, it's no longer VARYING.
-      if (!m_props.nan_varying_p ()
-	  || !m_props.inf_varying_p ()
-	  || !m_props.ninf_varying_p ())
-	m_kind = VR_RANGE;
+      if (!m_props.varying_p ())
+	{
+	  m_kind = VR_RANGE;
+	  return true;
+	}
     }
+  return false;
 }
 
 bool
@@ -354,7 +383,7 @@ frange::union_ (const vrange &v)
     }
 
   bool ret = m_props.union_ (r.m_props);
-  normalize_kind ();
+  ret |= normalize_kind ();
 
   if (flag_checking)
     verify_range ();
@@ -380,7 +409,7 @@ frange::intersect (const vrange &v)
     }
 
   bool ret = m_props.intersect (r.m_props);
-  normalize_kind ();
+  ret |= normalize_kind ();
 
   if (flag_checking)
     verify_range ();
@@ -429,12 +458,11 @@ frange::verify_range ()
       gcc_checking_assert (m_props.undefined_p ());
       return;
     }
-  else if (varying_p ())
+  if (varying_p ())
     {
       gcc_checking_assert (m_props.varying_p ());
       return;
     }
-
   gcc_checking_assert (m_kind == VR_RANGE);
   gcc_checking_assert (!m_props.varying_p () && !m_props.undefined_p ());
 }
diff --git a/gcc/value-range.h b/gcc/value-range.h
index c6ab955c407..390fcb8fd99 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -359,7 +359,7 @@ public:
   FRANGE_PROP_ACCESSOR(ninf)
 private:
   void verify_range ();
-  void normalize_kind ();
+  bool normalize_kind ();
 
   frange_props m_props;
   tree m_type;
@@ -1010,6 +1010,45 @@ irange::normalize_kind ()
     }
 }
 
+// Return the maximum value for TYPE.
+
+inline tree
+vrp_val_max (const_tree type)
+{
+  if (INTEGRAL_TYPE_P (type))
+    return TYPE_MAX_VALUE (type);
+  if (POINTER_TYPE_P (type))
+    {
+      wide_int max = wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type));
+      return wide_int_to_tree (const_cast<tree> (type), max);
+    }
+  if (frange::supports_p (type))
+    {
+      REAL_VALUE_TYPE real;
+      real_inf (&real);
+      return build_real (const_cast <tree> (type), real);
+    }
+  return NULL_TREE;
+}
+
+// Return the minimum value for TYPE.
+
+inline tree
+vrp_val_min (const_tree type)
+{
+  if (INTEGRAL_TYPE_P (type))
+    return TYPE_MIN_VALUE (type);
+  if (POINTER_TYPE_P (type))
+    return build_zero_cst (const_cast<tree> (type));
+  if (frange::supports_p (type))
+    {
+      REAL_VALUE_TYPE real, real_ninf;
+      real_inf (&real);
+      real_ninf = real_value_negate (&real);
+      return build_real (const_cast <tree> (type), real_ninf);
+    }
+  return NULL_TREE;
+}
 
 // Supporting methods for frange.
 
@@ -1039,7 +1078,6 @@ inline
 frange::frange ()
 {
   m_discriminator = VR_FRANGE;
-  m_type = nullptr;
   set_undefined ();
 }
 
@@ -1072,32 +1110,4 @@ frange::set_undefined ()
   m_props.set_undefined ();
 }
 
-
-// Return the maximum value for TYPE.
-
-inline tree
-vrp_val_max (const_tree type)
-{
-  if (INTEGRAL_TYPE_P (type))
-    return TYPE_MAX_VALUE (type);
-  if (POINTER_TYPE_P (type))
-    {
-      wide_int max = wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type));
-      return wide_int_to_tree (const_cast<tree> (type), max);
-    }
-  return NULL_TREE;
-}
-
-// Return the minimum value for TYPE.
-
-inline tree
-vrp_val_min (const_tree type)
-{
-  if (INTEGRAL_TYPE_P (type))
-    return TYPE_MIN_VALUE (type);
-  if (POINTER_TYPE_P (type))
-    return build_zero_cst (const_cast<tree> (type));
-  return NULL_TREE;
-}
-
 #endif // GCC_VALUE_RANGE_H
-- 
2.37.1

Re: [COMMITTED] Make irange dependency explicit for range_of_ssa_name_with_loop_info.

[Richard Biener]

On Mon, Aug 1, 2022 at 8:17 AM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Even though ranger is type agnostic, SCEV seems to only work with
> integers.  This patch removes some FIXME notes making it explicit that
> bounds_of_var_in_loop only works with iranges.

SCEV also handles floats, where do you see this failing?  Yes, support is
somewhat limited, but still.

> Tested on x86-64 Linux.
>
> gcc/ChangeLog:
>
>         * gimple-range-fold.cc (fold_using_range::range_of_phi): Only
>         query SCEV for integers.
>         (fold_using_range::range_of_ssa_name_with_loop_info): Remove
>         irange check.
> ---
>  gcc/gimple-range-fold.cc | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
> index 6f907df5bf5..7665c954f2b 100644
> --- a/gcc/gimple-range-fold.cc
> +++ b/gcc/gimple-range-fold.cc
> @@ -853,12 +853,14 @@ fold_using_range::range_of_phi (vrange &r, gphi *phi, fur_source &src)
>        }
>
>    // If SCEV is available, query if this PHI has any knonwn values.
> -  if (scev_initialized_p () && !POINTER_TYPE_P (TREE_TYPE (phi_def)))
> +  if (scev_initialized_p ()
> +      && !POINTER_TYPE_P (TREE_TYPE (phi_def))
> +      && irange::supports_p (TREE_TYPE (phi_def)))
>      {
> -      value_range loop_range;
>        class loop *l = loop_containing_stmt (phi);
>        if (l && loop_outer (l))
>         {
> +         int_range_max loop_range;
>           range_of_ssa_name_with_loop_info (loop_range, phi_def, l, phi, src);
>           if (!loop_range.varying_p ())
>             {
> @@ -1337,9 +1339,7 @@ fold_using_range::range_of_ssa_name_with_loop_info (irange &r, tree name,
>  {
>    gcc_checking_assert (TREE_CODE (name) == SSA_NAME);
>    tree min, max, type = TREE_TYPE (name);
> -  // FIXME: Remove the supports_p() once all this can handle floats, etc.
> -  if (irange::supports_p (type)
> -      && bounds_of_var_in_loop (&min, &max, src.query (), l, phi, name))
> +  if (bounds_of_var_in_loop (&min, &max, src.query (), l, phi, name))
>      {
>        if (TREE_CODE (min) != INTEGER_CST)
>         {
> --
> 2.37.1
>

Re: [COMMITTED] Make irange dependency explicit for range_of_ssa_name_with_loop_info.

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

On Tue, Aug 2, 2022 at 9:19 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Mon, Aug 1, 2022 at 8:17 AM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Even though ranger is type agnostic, SCEV seems to only work with
> > integers.  This patch removes some FIXME notes making it explicit that
> > bounds_of_var_in_loop only works with iranges.
>
> SCEV also handles floats, where do you see this failing?  Yes, support is
> somewhat limited, but still.

Oh, it does?  We could convert  range_of_ssa_name_with_loop_info to
the type agnostic vrange if so... (see attached untested patch).

I'm looking at the testcase for 24021 with the attached patch, and
bounds_of_var_in_loop() is returning false because SCEV couldn't
figure out the direction of the loop:

  dir = scev_direction (chrec);
  if (/* Do not adjust ranges if we do not know whether the iv increases
     or decreases,  ... */
      dir == EV_DIR_UNKNOWN
      /* ... or if it may wrap.  */
      || scev_probably_wraps_p (NULL_TREE, init, step, stmt,
                get_chrec_loop (chrec), true))
    return false;

The chrec in question is rather simple... a loop increasing by 0.01:

(gdb) p debug(chrec)
{1.00000000000000002081668171172168513294309377670288085938e-2, +,
1.00000001490116119384765625e-1}_1

I also see that bounds_of_var_in_loop() calls niter's
max_loop_iterations(), which if I understand things correctly, bails
at several points if the step is not integral.

I'd be happy to adapt our code, if SCEV can give me useful information.

Aldy

[-- Attachment #2: 0009-Make-range_of_ssa_name_with_loop_info-type-agnostic.patch --]
[-- Type: text/x-patch, Size: 4161 bytes --]

From 86752bb57b0534b496b51559ec8e28e56fb3ea4e Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Tue, 2 Aug 2022 13:27:16 +0200
Subject: [PATCH] Make range_of_ssa_name_with_loop_info type agnostic.

gcc/ChangeLog:

	* gimple-range-fold.cc (fold_using_range::range_of_phi): Remove
	irange check.
	(tree_lower_bound): New.
	(tree_upper_bound): New.
	(fold_using_range::range_of_ssa_name_with_loop_info): Convert to
	vrange.
	* gimple-range-fold.h (range_of_ssa_name_with_loop_info): Change
	argument to vrange.
---
 gcc/gimple-range-fold.cc | 46 ++++++++++++++++++++++++++++++----------
 gcc/gimple-range-fold.h  |  2 +-
 2 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index 7665c954f2b..923094abd62 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -854,13 +854,12 @@ fold_using_range::range_of_phi (vrange &r, gphi *phi, fur_source &src)
 
   // If SCEV is available, query if this PHI has any knonwn values.
   if (scev_initialized_p ()
-      && !POINTER_TYPE_P (TREE_TYPE (phi_def))
-      && irange::supports_p (TREE_TYPE (phi_def)))
+      && !POINTER_TYPE_P (TREE_TYPE (phi_def)))
     {
       class loop *l = loop_containing_stmt (phi);
       if (l && loop_outer (l))
 	{
-	  int_range_max loop_range;
+	  Value_Range loop_range (type);
 	  range_of_ssa_name_with_loop_info (loop_range, phi_def, l, phi, src);
 	  if (!loop_range.varying_p ())
 	    {
@@ -1330,10 +1329,32 @@ fold_using_range::range_of_cond_expr  (vrange &r, gassign *s, fur_source &src)
   return true;
 }
 
+// Return the lower bound of R as a tree.
+
+static inline tree
+tree_lower_bound (const vrange &r, tree type)
+{
+  if (is_a <irange> (r))
+    return wide_int_to_tree (type, as_a <irange> (r).lower_bound ());
+  // ?? Handle floats when they contain endpoints.
+  return NULL;
+}
+
+// Return the upper bound of R as a tree.
+
+static inline tree
+tree_upper_bound (const vrange &r, tree type)
+{
+  if (is_a <irange> (r))
+    return wide_int_to_tree (type, as_a <irange> (r).upper_bound ());
+  // ?? Handle floats when they contain endpoints.
+  return NULL;
+}
+
 // If SCEV has any information about phi node NAME, return it as a range in R.
 
 void
-fold_using_range::range_of_ssa_name_with_loop_info (irange &r, tree name,
+fold_using_range::range_of_ssa_name_with_loop_info (vrange &r, tree name,
 						    class loop *l, gphi *phi,
 						    fur_source &src)
 {
@@ -1341,24 +1362,27 @@ fold_using_range::range_of_ssa_name_with_loop_info (irange &r, tree name,
   tree min, max, type = TREE_TYPE (name);
   if (bounds_of_var_in_loop (&min, &max, src.query (), l, phi, name))
     {
-      if (TREE_CODE (min) != INTEGER_CST)
+      if (!is_gimple_constant (min))
 	{
 	  if (src.query ()->range_of_expr (r, min, phi) && !r.undefined_p ())
-	    min = wide_int_to_tree (type, r.lower_bound ());
+	    min = tree_lower_bound (r, type);
 	  else
 	    min = vrp_val_min (type);
 	}
-      if (TREE_CODE (max) != INTEGER_CST)
+      if (!is_gimple_constant (max))
 	{
 	  if (src.query ()->range_of_expr (r, max, phi) && !r.undefined_p ())
-	    max = wide_int_to_tree (type, r.upper_bound ());
+	    max = tree_upper_bound (r, type);
 	  else
 	    max = vrp_val_max (type);
 	}
-      r.set (min, max);
+      if (min && max)
+	{
+	  r.set (min, max);
+	  return;
+	}
     }
-  else
-    r.set_varying (type);
+  r.set_varying (type);
 }
 
 // -----------------------------------------------------------------------
diff --git a/gcc/gimple-range-fold.h b/gcc/gimple-range-fold.h
index fbf66275f74..c2f381dffec 100644
--- a/gcc/gimple-range-fold.h
+++ b/gcc/gimple-range-fold.h
@@ -173,7 +173,7 @@ protected:
   void range_of_builtin_ubsan_call (irange &r, gcall *call, tree_code code,
 				    fur_source &src);
   bool range_of_phi (vrange &r, gphi *phi, fur_source &src);
-  void range_of_ssa_name_with_loop_info (irange &, tree, class loop *, gphi *,
+  void range_of_ssa_name_with_loop_info (vrange &, tree, class loop *, gphi *,
 					 fur_source &src);
   void relation_fold_and_or (irange& lhs_range, gimple *s, fur_source &src);
 };
-- 
2.37.1

Re: [COMMITTED] Make irange dependency explicit for range_of_ssa_name_with_loop_info.

[Richard Biener]

On Tue, Aug 2, 2022 at 1:41 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> On Tue, Aug 2, 2022 at 9:19 AM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Mon, Aug 1, 2022 at 8:17 AM Aldy Hernandez via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > >
> > > Even though ranger is type agnostic, SCEV seems to only work with
> > > integers.  This patch removes some FIXME notes making it explicit that
> > > bounds_of_var_in_loop only works with iranges.
> >
> > SCEV also handles floats, where do you see this failing?  Yes, support is
> > somewhat limited, but still.
>
> Oh, it does?  We could convert  range_of_ssa_name_with_loop_info to
> the type agnostic vrange if so... (see attached untested patch).
>
> I'm looking at the testcase for 24021 with the attached patch, and
> bounds_of_var_in_loop() is returning false because SCEV couldn't
> figure out the direction of the loop:
>
>   dir = scev_direction (chrec);
>   if (/* Do not adjust ranges if we do not know whether the iv increases
>      or decreases,  ... */
>       dir == EV_DIR_UNKNOWN
>       /* ... or if it may wrap.  */
>       || scev_probably_wraps_p (NULL_TREE, init, step, stmt,
>                 get_chrec_loop (chrec), true))
>     return false;
>
> The chrec in question is rather simple... a loop increasing by 0.01:
>
> (gdb) p debug(chrec)
> {1.00000000000000002081668171172168513294309377670288085938e-2, +,
> 1.00000001490116119384765625e-1}_1

Well, yeah - I meant it "supports" floats in that it can analyze the CHRECs (you
quote that) and it shouldn't ICE anywhere.  But of course some things may
return "I don't know" when not implemented.  Like scev_direction which has

  step = CHREC_RIGHT (chrec);
  if (TREE_CODE (step) != INTEGER_CST)
    return EV_DIR_UNKNOWN;

  if (tree_int_cst_sign_bit (step))
    return EV_DIR_DECREASES;
  else
    return EV_DIR_GROWS;

so it lacks support for REAL_CST step.  Would be interesting to see what happens
with a loop with -0.0 "increment" and honoring signed zeros.  That said,
inspecting the sign bit of a REAL_CST and handling zero (of any sign) as
EV_DIR_UNKNOWN would probably work.

> I also see that bounds_of_var_in_loop() calls niter's
> max_loop_iterations(), which if I understand things correctly, bails
> at several points if the step is not integral.

Yes, a lot of code is "simplified" by not considering FP IVs.  But it "works"
in terms of "it doesn't ICE" ;)

> I'd be happy to adapt our code, if SCEV can give me useful information.
>
> Aldy

Re: [COMMITTED] Make irange dependency explicit for range_of_ssa_name_with_loop_info.

[Aldy Hernandez]

On Tue, Aug 2, 2022 at 2:07 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Aug 2, 2022 at 1:41 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> > On Tue, Aug 2, 2022 at 9:19 AM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> > >
> > > On Mon, Aug 1, 2022 at 8:17 AM Aldy Hernandez via Gcc-patches
> > > <gcc-patches@gcc.gnu.org> wrote:
> > > >
> > > > Even though ranger is type agnostic, SCEV seems to only work with
> > > > integers.  This patch removes some FIXME notes making it explicit that
> > > > bounds_of_var_in_loop only works with iranges.
> > >
> > > SCEV also handles floats, where do you see this failing?  Yes, support is
> > > somewhat limited, but still.
> >
> > Oh, it does?  We could convert  range_of_ssa_name_with_loop_info to
> > the type agnostic vrange if so... (see attached untested patch).
> >
> > I'm looking at the testcase for 24021 with the attached patch, and
> > bounds_of_var_in_loop() is returning false because SCEV couldn't
> > figure out the direction of the loop:
> >
> >   dir = scev_direction (chrec);
> >   if (/* Do not adjust ranges if we do not know whether the iv increases
> >      or decreases,  ... */
> >       dir == EV_DIR_UNKNOWN
> >       /* ... or if it may wrap.  */
> >       || scev_probably_wraps_p (NULL_TREE, init, step, stmt,
> >                 get_chrec_loop (chrec), true))
> >     return false;
> >
> > The chrec in question is rather simple... a loop increasing by 0.01:
> >
> > (gdb) p debug(chrec)
> > {1.00000000000000002081668171172168513294309377670288085938e-2, +,
> > 1.00000001490116119384765625e-1}_1
>
> Well, yeah - I meant it "supports" floats in that it can analyze the CHRECs (you
> quote that) and it shouldn't ICE anywhere.  But of course some things may
> return "I don't know" when not implemented.  Like scev_direction which has
>
>   step = CHREC_RIGHT (chrec);
>   if (TREE_CODE (step) != INTEGER_CST)
>     return EV_DIR_UNKNOWN;
>
>   if (tree_int_cst_sign_bit (step))
>     return EV_DIR_DECREASES;
>   else
>     return EV_DIR_GROWS;
>
> so it lacks support for REAL_CST step.  Would be interesting to see what happens
> with a loop with -0.0 "increment" and honoring signed zeros.  That said,
> inspecting the sign bit of a REAL_CST and handling zero (of any sign) as
> EV_DIR_UNKNOWN would probably work.
>
> > I also see that bounds_of_var_in_loop() calls niter's
> > max_loop_iterations(), which if I understand things correctly, bails
> > at several points if the step is not integral.
>
> Yes, a lot of code is "simplified" by not considering FP IVs.  But it "works"
> in terms of "it doesn't ICE" ;)

That's a very generous interpretation of "works" :-).

In that case I will make our code type agnostic, with the previously
attached patch (after testing).  Then whenever SCEV and
bounds_of_var_in_loop (which also seems to be integer centric) support
floats, everything will just work.

Thanks.
Aldy

Re: [COMMITTED] Make irange dependency explicit for range_of_ssa_name_with_loop_info.

[Richard Biener]

On Tue, Aug 2, 2022 at 2:11 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> On Tue, Aug 2, 2022 at 2:07 PM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Tue, Aug 2, 2022 at 1:41 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> > >
> > > On Tue, Aug 2, 2022 at 9:19 AM Richard Biener
> > > <richard.guenther@gmail.com> wrote:
> > > >
> > > > On Mon, Aug 1, 2022 at 8:17 AM Aldy Hernandez via Gcc-patches
> > > > <gcc-patches@gcc.gnu.org> wrote:
> > > > >
> > > > > Even though ranger is type agnostic, SCEV seems to only work with
> > > > > integers.  This patch removes some FIXME notes making it explicit that
> > > > > bounds_of_var_in_loop only works with iranges.
> > > >
> > > > SCEV also handles floats, where do you see this failing?  Yes, support is
> > > > somewhat limited, but still.
> > >
> > > Oh, it does?  We could convert  range_of_ssa_name_with_loop_info to
> > > the type agnostic vrange if so... (see attached untested patch).
> > >
> > > I'm looking at the testcase for 24021 with the attached patch, and
> > > bounds_of_var_in_loop() is returning false because SCEV couldn't
> > > figure out the direction of the loop:
> > >
> > >   dir = scev_direction (chrec);
> > >   if (/* Do not adjust ranges if we do not know whether the iv increases
> > >      or decreases,  ... */
> > >       dir == EV_DIR_UNKNOWN
> > >       /* ... or if it may wrap.  */
> > >       || scev_probably_wraps_p (NULL_TREE, init, step, stmt,
> > >                 get_chrec_loop (chrec), true))
> > >     return false;
> > >
> > > The chrec in question is rather simple... a loop increasing by 0.01:
> > >
> > > (gdb) p debug(chrec)
> > > {1.00000000000000002081668171172168513294309377670288085938e-2, +,
> > > 1.00000001490116119384765625e-1}_1
> >
> > Well, yeah - I meant it "supports" floats in that it can analyze the CHRECs (you
> > quote that) and it shouldn't ICE anywhere.  But of course some things may
> > return "I don't know" when not implemented.  Like scev_direction which has
> >
> >   step = CHREC_RIGHT (chrec);
> >   if (TREE_CODE (step) != INTEGER_CST)
> >     return EV_DIR_UNKNOWN;
> >
> >   if (tree_int_cst_sign_bit (step))
> >     return EV_DIR_DECREASES;
> >   else
> >     return EV_DIR_GROWS;
> >
> > so it lacks support for REAL_CST step.  Would be interesting to see what happens
> > with a loop with -0.0 "increment" and honoring signed zeros.  That said,
> > inspecting the sign bit of a REAL_CST and handling zero (of any sign) as
> > EV_DIR_UNKNOWN would probably work.
> >
> > > I also see that bounds_of_var_in_loop() calls niter's
> > > max_loop_iterations(), which if I understand things correctly, bails
> > > at several points if the step is not integral.
> >
> > Yes, a lot of code is "simplified" by not considering FP IVs.  But it "works"
> > in terms of "it doesn't ICE" ;)
>
> That's a very generous interpretation of "works" :-).

;)

> In that case I will make our code type agnostic, with the previously
> attached patch (after testing).  Then whenever SCEV and
> bounds_of_var_in_loop (which also seems to be integer centric) support
> floats, everything will just work.

Sounds good.

Richard.

> Thanks.
> Aldy
>