[PATCH] Add support for vrange streaming.

[PATCH] Add support for vrange streaming.

[Aldy Hernandez]

I think it's time for the ranger folk to start owning range streaming
instead of passes (IPA, etc) doing their own thing.  I have plans for
overhauling the IPA code later this cycle to support generic ranges,
and I'd like to start cleaning up the streaming and hashing interface.

This patch adds generic streaming support for vrange.

I'd appreciate another set of eyes.

Thoughts?

gcc/ChangeLog:

	* data-streamer-in.cc (streamer_read_real_value): New.
	(streamer_read_value_range): New.
	* data-streamer-out.cc (streamer_write_real_value): New.
	(streamer_write_vrange): New.
	* data-streamer.h (streamer_write_vrange): New.
	(streamer_read_value_range): New.
	* value-range.h (class frange): Mark streamer_write_vrange and
	streamer_read_value_range as friends.
---
 gcc/data-streamer-in.cc  | 56 ++++++++++++++++++++++++++++++++++++++++
 gcc/data-streamer-out.cc | 48 ++++++++++++++++++++++++++++++++++
 gcc/data-streamer.h      |  3 +++
 gcc/value-range.h        |  4 +++
 4 files changed, 111 insertions(+)

diff --git a/gcc/data-streamer-in.cc b/gcc/data-streamer-in.cc
index 8ebcac43479..76954b12c95 100644
--- a/gcc/data-streamer-in.cc
+++ b/gcc/data-streamer-in.cc
@@ -28,6 +28,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple.h"
 #include "cgraph.h"
 #include "data-streamer.h"
+#include "value-range.h"
+#include "streamer-hooks.h"
 
 /* Read a string from the string table in DATA_IN using input block
    IB.  Write the length to RLEN.  */
@@ -206,6 +208,60 @@ streamer_read_gcov_count (class lto_input_block *ib)
   return ret;
 }
 
+/* Read REAL_VALUE_TYPE from IB.  */
+
+void
+streamer_read_real_value (class lto_input_block *ib, REAL_VALUE_TYPE *r)
+{
+  struct bitpack_d bp = streamer_read_bitpack (ib);
+  bp_unpack_real_value (&bp, r);
+}
+
+void
+streamer_read_value_range (class lto_input_block *ib, data_in *data_in,
+			   Value_Range &vr)
+{
+  // Read the common fields to all vranges.
+  value_range_kind kind = streamer_read_enum (ib, value_range_kind, VR_LAST);
+  gcc_checking_assert (kind != VR_UNDEFINED);
+  tree type = stream_read_tree (ib, data_in);
+
+  // Initialize the Value_Range to the correct type.
+  vr.set_type (type);
+
+  if (is_a <irange> (vr))
+    {
+      irange &r = as_a <irange> (vr);
+      r.set_undefined ();
+      unsigned HOST_WIDE_INT num_pairs = streamer_read_uhwi (ib);
+      for (unsigned i = 0; i < num_pairs; ++i)
+	{
+	  wide_int lb = streamer_read_wide_int (ib);
+	  wide_int ub = streamer_read_wide_int (ib);
+	  int_range<2> tmp (type, lb, ub);
+	  r.union_ (tmp);
+	}
+      wide_int nz = streamer_read_wide_int (ib);
+      r.set_nonzero_bits (nz);
+      return;
+    }
+  if (is_a <frange> (vr))
+    {
+      frange &r = as_a <frange> (vr);
+
+      r.m_type = type;
+      streamer_read_real_value (ib, &r.m_min);
+      streamer_read_real_value (ib, &r.m_max);
+      struct bitpack_d bp = streamer_read_bitpack (ib);
+      r.m_pos_nan = (bool) bp_unpack_value (&bp, 1);
+      r.m_neg_nan = (bool) bp_unpack_value (&bp, 1);
+      if (flag_checking)
+	r.verify_range ();
+      return;
+    }
+  gcc_unreachable ();
+}
+
 /* Read the physical representation of a wide_int val from
    input block IB.  */
 
diff --git a/gcc/data-streamer-out.cc b/gcc/data-streamer-out.cc
index cd25745b8dc..888fc395d0d 100644
--- a/gcc/data-streamer-out.cc
+++ b/gcc/data-streamer-out.cc
@@ -28,6 +28,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple.h"
 #include "cgraph.h"
 #include "data-streamer.h"
+#include "value-range.h"
+#include "streamer-hooks.h"
 
 
 /* Adds a new block to output stream OBS.  */
@@ -392,6 +394,52 @@ streamer_write_data_stream (struct lto_output_stream *obs, const void *data,
     }
 }
 
+/* Write REAL_VALUE_TYPE into OB.  */
+
+void
+streamer_write_real_value (struct output_block *ob, const REAL_VALUE_TYPE *r)
+{
+  bitpack_d bp = bitpack_create (ob->main_stream);
+  bp_pack_real_value (&bp, r);
+  streamer_write_bitpack (&bp);
+}
+
+void
+streamer_write_vrange (struct output_block *ob, const vrange &v)
+{
+  gcc_checking_assert (!v.undefined_p ());
+
+  // Write the common fields to all vranges.
+  value_range_kind kind = v.varying_p () ? VR_VARYING : VR_RANGE;
+  streamer_write_enum (ob->main_stream, value_range_kind, VR_LAST, kind);
+  stream_write_tree (ob, v.type (), true);
+
+  if (is_a <irange> (v))
+    {
+      const irange &r = as_a <irange> (v);
+      streamer_write_uhwi (ob, r.num_pairs ());
+      for (unsigned i = 0; i < r.num_pairs (); ++i)
+	{
+	  streamer_write_wide_int (ob, r.lower_bound (i));
+	  streamer_write_wide_int (ob, r.upper_bound (i));
+	}
+      streamer_write_wide_int (ob, r.get_nonzero_bits ());
+      return;
+    }
+  if (is_a <frange> (v))
+    {
+      const frange &r = as_a <frange> (v);
+      streamer_write_real_value (ob, &r.m_min);
+      streamer_write_real_value (ob, &r.m_max);
+      bitpack_d bp = bitpack_create (ob->main_stream);
+      bp_pack_value (&bp, r.m_pos_nan, 1);
+      bp_pack_value (&bp, r.m_neg_nan, 1);
+      streamer_write_bitpack (&bp);
+      return;
+    }
+  gcc_unreachable ();
+}
+
 /* Emit the physical representation of wide_int VAL to output block OB.  */
 
 void
diff --git a/gcc/data-streamer.h b/gcc/data-streamer.h
index 19c9d6ea606..7e69eb9992b 100644
--- a/gcc/data-streamer.h
+++ b/gcc/data-streamer.h
@@ -75,6 +75,7 @@ void streamer_write_data_stream (struct lto_output_stream *, const void *,
 				 size_t);
 void streamer_write_wide_int (struct output_block *, const wide_int &);
 void streamer_write_widest_int (struct output_block *, const widest_int &);
+void streamer_write_vrange (struct output_block *, const class vrange &);
 
 /* In data-streamer-in.cc  */
 const char *streamer_read_string (class data_in *, class lto_input_block *);
@@ -91,6 +92,8 @@ poly_int64 streamer_read_poly_int64 (class lto_input_block *);
 gcov_type streamer_read_gcov_count (class lto_input_block *);
 wide_int streamer_read_wide_int (class lto_input_block *);
 widest_int streamer_read_widest_int (class lto_input_block *);
+void streamer_read_value_range (class lto_input_block *, class data_in *,
+				class Value_Range &);
 
 /* Returns a new bit-packing context for bit-packing into S.  */
 inline struct bitpack_d
diff --git a/gcc/value-range.h b/gcc/value-range.h
index f8aa0ca7bec..e56df9db901 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -328,6 +328,10 @@ class frange : public vrange
 {
   friend class frange_storage_slot;
   friend class vrange_printer;
+  friend void streamer_write_vrange (struct output_block *, const vrange &);
+  friend void streamer_read_value_range (class lto_input_block *,
+					 class data_in *,
+					 class Value_Range &);
 public:
   frange ();
   frange (const frange &);
-- 
2.39.2

[PATCH] Add inchash support for vrange.

[Aldy Hernandez]

This patch provides inchash support for vrange.  It is along the lines
of the streaming support I just posted and will be used for IPA
hashing of ranges.

Thoughts?

gcc/ChangeLog:

	* inchash.cc (hash::add_real_value): New.
	* inchash.h (class hash): Add add_real_value.
	* value-range.cc (add_vrange): New.
	* value-range.h (inchash::add_vrange): New.
---
 gcc/inchash.cc     | 20 +++++++++++++++++++
 gcc/inchash.h      |  2 ++
 gcc/value-range.cc | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 gcc/value-range.h  |  6 ++++++
 4 files changed, 76 insertions(+)

diff --git a/gcc/inchash.cc b/gcc/inchash.cc
index a30662b97fe..914e3cc92cd 100644
--- a/gcc/inchash.cc
+++ b/gcc/inchash.cc
@@ -24,3 +24,23 @@ along with GCC; see the file COPYING3.  If not see
 #endif
 #include "system.h"
 #include "coretypes.h"
+#include "real.h"
+#include "inchash.h"
+
+namespace inchash
+{
+
+/* This is here instead of inchash.h to keep us from having to put
+   real.h in coretypes.h.  */
+void
+hash::add_real_value (const real_value &v)
+{
+  add_int (v.sign);
+  add_int (v.uexp);
+  for (unsigned i = 0; i < SIGSZ; ++i)
+    add_hwi (v.sig[i]);
+  /* Ignore the rest of the flags, as sign, exponent, and
+     significant bits should be enough.  */
+}
+
+} // namespace inchash
diff --git a/gcc/inchash.h b/gcc/inchash.h
index bf76308431d..41ae153d1c5 100644
--- a/gcc/inchash.h
+++ b/gcc/inchash.h
@@ -88,6 +88,8 @@ class hash
       add_hwi (x.sext_elt (i));
   }
 
+  void add_real_value (const class real_value &v);
+
   /* Hash in pointer PTR.  */
   void add_ptr (const void *ptr)
   {
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 963330eed79..31ec832e185 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -232,6 +232,54 @@ vrange::dump (FILE *file) const
   pp_flush (&buffer);
 }
 
+namespace inchash
+{
+
+void
+add_vrange (const vrange &v, inchash::hash &hstate,
+	     unsigned int)
+{
+  if (v.undefined_p ())
+    {
+      hstate.add_int (VR_UNDEFINED);
+      return;
+    }
+  // Types are ignored throughout to inhibit two ranges being equal
+  // but having different hash values.  This can happen when two
+  // ranges are equal types_compatible_p is true.
+  if (is_a <irange> (v))
+    {
+      const irange &r = as_a <irange> (v);
+      if (r.varying_p ())
+	hstate.add_int (VR_VARYING);
+      else
+	hstate.add_int (VR_RANGE);
+      for (unsigned i = 0; i < r.num_pairs (); ++i)
+	{
+	  hstate.add_wide_int (r.lower_bound (i));
+	  hstate.add_wide_int (r.upper_bound (i));
+	}
+      hstate.add_wide_int (r.get_nonzero_bits ());
+      return;
+    }
+  if (is_a <frange> (v))
+    {
+      const frange &r = as_a <frange> (v);
+      if (r.varying_p ())
+	hstate.add_int (VR_VARYING);
+      else
+	hstate.add_int (VR_RANGE);
+      hstate.add_real_value (r.m_min);
+      hstate.add_real_value (r.m_max);
+      hstate.add_int (r.m_pos_nan);
+      hstate.add_int (r.m_neg_nan);
+      return;
+    }
+  gcc_unreachable ();
+}
+
+} //namespace inchash
+
 bool
 irange::supports_type_p (const_tree type) const
 {
diff --git a/gcc/value-range.h b/gcc/value-range.h
index e56df9db901..cb7418208d4 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -109,6 +109,11 @@ protected:
   const ENUM_BITFIELD(value_range_discriminator) m_discriminator : 4;
 };
 
+namespace inchash
+{
+  extern void add_vrange (const vrange &, hash &, unsigned flags = 0);
+}
+
 // An integer range without any storage.
 
 class GTY((user)) irange : public vrange
@@ -332,6 +337,7 @@ class frange : public vrange
   friend void streamer_read_value_range (class lto_input_block *,
 					 class data_in *,
 					 class Value_Range &);
+  friend void inchash::add_vrange (const vrange &, inchash::hash &, unsigned);
 public:
   frange ();
   frange (const frange &);
-- 
2.39.2

Re: [PATCH] Add inchash support for vrange.

[Aldy Hernandez]

> +namespace inchash
> +{
> +
> +void
> +add_vrange (const vrange &v, inchash::hash &hstate,
> +	     unsigned int)
> +{
> +  if (v.undefined_p ())
> +    {
> +      hstate.add_int (VR_UNDEFINED);
> +      return;
> +    }
> +  // Types are ignored throughout to inhibit two ranges being equal
> +  // but having different hash values.  This can happen when two
> +  // ranges are equal types_compatible_p is true.

Sorry, this comment should read:

+  // Types are ignored throughout to inhibit two ranges being equal
+  // but having different hash values.  This can happen when two
+  // ranges are equal and their types are different (but
+  // types_compatible_p is true).

Re: [PATCH] Add inchash support for vrange.

[Jakub Jelinek]

On Tue, Apr 18, 2023 at 11:06:38AM +0200, Aldy Hernandez via Gcc-patches wrote:
> This patch provides inchash support for vrange.  It is along the lines
> of the streaming support I just posted and will be used for IPA
> hashing of ranges.
> 
> Thoughts?
> 
> gcc/ChangeLog:
> 
> 	* inchash.cc (hash::add_real_value): New.
> 	* inchash.h (class hash): Add add_real_value.
> 	* value-range.cc (add_vrange): New.
> 	* value-range.h (inchash::add_vrange): New.
> ---
>  gcc/inchash.cc     | 20 +++++++++++++++++++
>  gcc/inchash.h      |  2 ++
>  gcc/value-range.cc | 48 ++++++++++++++++++++++++++++++++++++++++++++++
>  gcc/value-range.h  |  6 ++++++
>  4 files changed, 76 insertions(+)
> 
> diff --git a/gcc/inchash.cc b/gcc/inchash.cc
> index a30662b97fe..914e3cc92cd 100644
> --- a/gcc/inchash.cc
> +++ b/gcc/inchash.cc
> @@ -24,3 +24,23 @@ along with GCC; see the file COPYING3.  If not see
>  #endif
>  #include "system.h"
>  #include "coretypes.h"
> +#include "real.h"
> +#include "inchash.h"
> +
> +namespace inchash
> +{
> +
> +/* This is here instead of inchash.h to keep us from having to put
> +   real.h in coretypes.h.  */
> +void
> +hash::add_real_value (const real_value &v)
> +{
> +  add_int (v.sign);
> +  add_int (v.uexp);
> +  for (unsigned i = 0; i < SIGSZ; ++i)
> +    add_hwi (v.sig[i]);
> +  /* Ignore the rest of the flags, as sign, exponent, and
> +     significant bits should be enough.  */

I don't think that's the case.
At least cl, decimal and signalling are essential flags as well.
Dunno about canonical.
How do you otherwise differentiate between Inf and +0.0 or (canonical)
qNaN or (canonical) sNaN?
They have the same sign, uexp and sig.

	Jakub

Re: [PATCH] Add inchash support for vrange.

[Jakub Jelinek]

On Tue, Apr 18, 2023 at 11:33:05AM +0200, Jakub Jelinek wrote:
> On Tue, Apr 18, 2023 at 11:06:38AM +0200, Aldy Hernandez via Gcc-patches wrote:
> > This patch provides inchash support for vrange.  It is along the lines
> > of the streaming support I just posted and will be used for IPA
> > hashing of ranges.
> > 
> > Thoughts?
> > 
> > gcc/ChangeLog:
> > 
> > 	* inchash.cc (hash::add_real_value): New.
> > 	* inchash.h (class hash): Add add_real_value.
> > 	* value-range.cc (add_vrange): New.
> > 	* value-range.h (inchash::add_vrange): New.
> > ---
> >  gcc/inchash.cc     | 20 +++++++++++++++++++
> >  gcc/inchash.h      |  2 ++
> >  gcc/value-range.cc | 48 ++++++++++++++++++++++++++++++++++++++++++++++
> >  gcc/value-range.h  |  6 ++++++
> >  4 files changed, 76 insertions(+)
> > 
> > diff --git a/gcc/inchash.cc b/gcc/inchash.cc
> > index a30662b97fe..914e3cc92cd 100644
> > --- a/gcc/inchash.cc
> > +++ b/gcc/inchash.cc
> > @@ -24,3 +24,23 @@ along with GCC; see the file COPYING3.  If not see
> >  #endif
> >  #include "system.h"
> >  #include "coretypes.h"
> > +#include "real.h"
> > +#include "inchash.h"
> > +
> > +namespace inchash
> > +{
> > +
> > +/* This is here instead of inchash.h to keep us from having to put
> > +   real.h in coretypes.h.  */
> > +void
> > +hash::add_real_value (const real_value &v)
> > +{
> > +  add_int (v.sign);
> > +  add_int (v.uexp);
> > +  for (unsigned i = 0; i < SIGSZ; ++i)
> > +    add_hwi (v.sig[i]);
> > +  /* Ignore the rest of the flags, as sign, exponent, and
> > +     significant bits should be enough.  */
> 
> I don't think that's the case.
> At least cl, decimal and signalling are essential flags as well.
> Dunno about canonical.
> How do you otherwise differentiate between Inf and +0.0 or (canonical)
> qNaN or (canonical) sNaN?
> They have the same sign, uexp and sig.

I'd say it is best to follow real_identical that is used for the
comparisons, that one always compares cl and sign and then differentiates
based on cl:
1) for rvc_zero/rvc_inf, everything else is ignored
2) for rvc_normal, decimal, uexp and sig are compared
3) for rvc_nan, signalling and canonical are compared and if !canonical,
   sig is also compared
So, perhaps:
  add_int (v.cl);
  add_int (v.sign);
  switch (v.cl)
    {
    case rvc_zero:
    case rvc_inf:
      return;
    case rvc_normal:
      add_int (v.decimal);
      add_int (REAL_EXP (&v));
      break;
    case rvc_nan:
      add_int (v.signalling);
      add_int (v.canonical);
      if (v.canonical)
	return;
      break;
    default:
      gcc_unreachable ();
    }
 for (unsigned i = 0; i < SIGSZ; ++i)
    add_hwi (v.sig[i]);

	Jakub

Re: [PATCH] Add inchash support for vrange.

[Aldy Hernandez]

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



On 4/18/23 12:32, Jakub Jelinek wrote:
> On Tue, Apr 18, 2023 at 11:33:05AM +0200, Jakub Jelinek wrote:
>> On Tue, Apr 18, 2023 at 11:06:38AM +0200, Aldy Hernandez via Gcc-patches wrote:
>>> This patch provides inchash support for vrange.  It is along the lines
>>> of the streaming support I just posted and will be used for IPA
>>> hashing of ranges.
>>>
>>> Thoughts?
>>>
>>> gcc/ChangeLog:
>>>
>>> 	* inchash.cc (hash::add_real_value): New.
>>> 	* inchash.h (class hash): Add add_real_value.
>>> 	* value-range.cc (add_vrange): New.
>>> 	* value-range.h (inchash::add_vrange): New.
>>> ---
>>>   gcc/inchash.cc     | 20 +++++++++++++++++++
>>>   gcc/inchash.h      |  2 ++
>>>   gcc/value-range.cc | 48 ++++++++++++++++++++++++++++++++++++++++++++++
>>>   gcc/value-range.h  |  6 ++++++
>>>   4 files changed, 76 insertions(+)
>>>
>>> diff --git a/gcc/inchash.cc b/gcc/inchash.cc
>>> index a30662b97fe..914e3cc92cd 100644
>>> --- a/gcc/inchash.cc
>>> +++ b/gcc/inchash.cc
>>> @@ -24,3 +24,23 @@ along with GCC; see the file COPYING3.  If not see
>>>   #endif
>>>   #include "system.h"
>>>   #include "coretypes.h"
>>> +#include "real.h"
>>> +#include "inchash.h"
>>> +
>>> +namespace inchash
>>> +{
>>> +
>>> +/* This is here instead of inchash.h to keep us from having to put
>>> +   real.h in coretypes.h.  */
>>> +void
>>> +hash::add_real_value (const real_value &v)
>>> +{
>>> +  add_int (v.sign);
>>> +  add_int (v.uexp);
>>> +  for (unsigned i = 0; i < SIGSZ; ++i)
>>> +    add_hwi (v.sig[i]);
>>> +  /* Ignore the rest of the flags, as sign, exponent, and
>>> +     significant bits should be enough.  */
>>
>> I don't think that's the case.
>> At least cl, decimal and signalling are essential flags as well.
>> Dunno about canonical.
>> How do you otherwise differentiate between Inf and +0.0 or (canonical)
>> qNaN or (canonical) sNaN?
>> They have the same sign, uexp and sig.
> 
> I'd say it is best to follow real_identical that is used for the
> comparisons, that one always compares cl and sign and then differentiates
> based on cl:
> 1) for rvc_zero/rvc_inf, everything else is ignored
> 2) for rvc_normal, decimal, uexp and sig are compared
> 3) for rvc_nan, signalling and canonical are compared and if !canonical,
>     sig is also compared
> So, perhaps:
>    add_int (v.cl);
>    add_int (v.sign);
>    switch (v.cl)
>      {
>      case rvc_zero:
>      case rvc_inf:
>        return;
>      case rvc_normal:
>        add_int (v.decimal);
>        add_int (REAL_EXP (&v));
>        break;
>      case rvc_nan:
>        add_int (v.signalling);
>        add_int (v.canonical);
>        if (v.canonical)
> 	return;
>        break;
>      default:
>        gcc_unreachable ();
>      }
>   for (unsigned i = 0; i < SIGSZ; ++i)
>      add_hwi (v.sig[i]);
> 
> 	Jakub
> 

Sounds good.  Thanks for the patch.

OK pending tests?

p.s. Cleaned up the need for declaring add_vrange() a friend of frange 
now that we have a way of getting the nan_state elegantly.

[-- Attachment #2: 0001-Add-inchash-support-for-vrange.patch --]
[-- Type: text/x-patch, Size: 4053 bytes --]

From b02b48a46a6b1a2f37e00f42a8e374298e745c0c Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Thu, 23 Feb 2023 08:48:28 +0100
Subject: [PATCH] Add inchash support for vrange.

This patch provides inchash support for vrange.  It is along the lines
of the streaming support I just posted and will be used for IPA
hashing of ranges.

gcc/ChangeLog:

	* inchash.cc (hash::add_real_value): New.
	* inchash.h (class hash): Add add_real_value.
	* value-range.cc (add_vrange): New.
	* value-range.h (inchash::add_vrange): New.
---
 gcc/inchash.cc     | 36 ++++++++++++++++++++++++++++++++
 gcc/inchash.h      |  2 ++
 gcc/value-range.cc | 52 ++++++++++++++++++++++++++++++++++++++++++++++
 gcc/value-range.h  |  5 +++++
 4 files changed, 95 insertions(+)

diff --git a/gcc/inchash.cc b/gcc/inchash.cc
index a30662b97fe..7890db04202 100644
--- a/gcc/inchash.cc
+++ b/gcc/inchash.cc
@@ -24,3 +24,39 @@ along with GCC; see the file COPYING3.  If not see
 #endif
 #include "system.h"
 #include "coretypes.h"
+#include "real.h"
+#include "inchash.h"
+
+namespace inchash
+{
+
+/* This is here instead of inchash.h to keep us from having to put
+   real.h in coretypes.h.  */
+void
+hash::add_real_value (const real_value &v)
+{
+  add_int (v.cl);
+  add_int (v.sign);
+  switch (v.cl)
+    {
+    case rvc_zero:
+    case rvc_inf:
+      return;
+    case rvc_normal:
+      add_int (v.decimal);
+      add_int (REAL_EXP (&v));
+      break;
+    case rvc_nan:
+      add_int (v.signalling);
+      add_int (v.canonical);
+      if (v.canonical)
+	return;
+      break;
+    default:
+      gcc_unreachable ();
+    }
+  for (unsigned i = 0; i < SIGSZ; ++i)
+    add_hwi (v.sig[i]);
+}
+
+} // namespace inchash
diff --git a/gcc/inchash.h b/gcc/inchash.h
index bf76308431d..41ae153d1c5 100644
--- a/gcc/inchash.h
+++ b/gcc/inchash.h
@@ -88,6 +88,8 @@ class hash
       add_hwi (x.sext_elt (i));
   }
 
+  void add_real_value (const class real_value &v);
+
   /* Hash in pointer PTR.  */
   void add_ptr (const void *ptr)
   {
diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index ec826c2fe1b..c14a27e23af 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -232,6 +232,58 @@ vrange::dump (FILE *file) const
   pp_flush (&buffer);
 }
 
+namespace inchash
+{
+
+void
+add_vrange (const vrange &v, inchash::hash &hstate,
+	     unsigned int)
+{
+  if (v.undefined_p ())
+    {
+      hstate.add_int (VR_UNDEFINED);
+      return;
+    }
+  // Types are ignored throughout to inhibit two ranges being equal
+  // but having different hash values.  This can happen when two
+  // ranges are equal and their types are different (but
+  // types_compatible_p is true).
+  if (is_a <irange> (v))
+    {
+      const irange &r = as_a <irange> (v);
+      if (r.varying_p ())
+	hstate.add_int (VR_VARYING);
+      else
+	hstate.add_int (VR_RANGE);
+      for (unsigned i = 0; i < r.num_pairs (); ++i)
+	{
+	  hstate.add_wide_int (r.lower_bound (i));
+	  hstate.add_wide_int (r.upper_bound (i));
+	}
+      hstate.add_wide_int (r.get_nonzero_bits ());
+      return;
+    }
+  if (is_a <frange> (v))
+    {
+      const frange &r = as_a <frange> (v);
+      if (r.varying_p ())
+	hstate.add_int (VR_VARYING);
+      else
+	hstate.add_int (VR_RANGE);
+
+      hstate.add_real_value (r.lower_bound ());
+      hstate.add_real_value (r.upper_bound ());
+
+      nan_state nan = r.get_nan_state ();
+      hstate.add_int (nan.pos_p ());
+      hstate.add_int (nan.neg_p ());
+      return;
+    }
+  gcc_unreachable ();
+}
+
+} //namespace inchash
+
 bool
 irange::supports_type_p (const_tree type) const
 {
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 969b2b68418..fb90f66d9de 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -108,6 +108,11 @@ protected:
   ENUM_BITFIELD(value_range_discriminator) m_discriminator : 4;
 };
 
+namespace inchash
+{
+  extern void add_vrange (const vrange &, hash &, unsigned flags = 0);
+}
+
 // An integer range without any storage.
 
 class GTY((user)) irange : public vrange
-- 
2.39.2

Re: [PATCH] Add inchash support for vrange.

[Jakub Jelinek]

On Tue, Apr 18, 2023 at 12:50:58PM +0200, Aldy Hernandez wrote:
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -232,6 +232,58 @@ vrange::dump (FILE *file) const
>    pp_flush (&buffer);
>  }
>  
> +namespace inchash
> +{
> +
> +void
> +add_vrange (const vrange &v, inchash::hash &hstate,
> +	     unsigned int)
> +{
> +  if (v.undefined_p ())
> +    {
> +      hstate.add_int (VR_UNDEFINED);
> +      return;
> +    }
> +  // Types are ignored throughout to inhibit two ranges being equal
> +  // but having different hash values.  This can happen when two
> +  // ranges are equal and their types are different (but
> +  // types_compatible_p is true).
> +  if (is_a <irange> (v))
> +    {
> +      const irange &r = as_a <irange> (v);
> +      if (r.varying_p ())
> +	hstate.add_int (VR_VARYING);
> +      else
> +	hstate.add_int (VR_RANGE);

Shouldn't this also
      hstate.add_int (r.num_pairs ());
?
Or is that unnecessary because different number of add_wide_int
calls will likely result in different hashes then?

Otherwise LGTM.

> +      for (unsigned i = 0; i < r.num_pairs (); ++i)
> +	{
> +	  hstate.add_wide_int (r.lower_bound (i));
> +	  hstate.add_wide_int (r.upper_bound (i));
> +	}
> +      hstate.add_wide_int (r.get_nonzero_bits ());
> +      return;
> +    }

	Jakub

Re: [PATCH] Add inchash support for vrange.

[Aldy Hernandez]

On 4/18/23 12:59, Jakub Jelinek wrote:
> On Tue, Apr 18, 2023 at 12:50:58PM +0200, Aldy Hernandez wrote:
>> --- a/gcc/value-range.cc
>> +++ b/gcc/value-range.cc
>> @@ -232,6 +232,58 @@ vrange::dump (FILE *file) const
>>     pp_flush (&buffer);
>>   }
>>   
>> +namespace inchash
>> +{
>> +
>> +void
>> +add_vrange (const vrange &v, inchash::hash &hstate,
>> +	     unsigned int)
>> +{
>> +  if (v.undefined_p ())
>> +    {
>> +      hstate.add_int (VR_UNDEFINED);
>> +      return;
>> +    }
>> +  // Types are ignored throughout to inhibit two ranges being equal
>> +  // but having different hash values.  This can happen when two
>> +  // ranges are equal and their types are different (but
>> +  // types_compatible_p is true).
>> +  if (is_a <irange> (v))
>> +    {
>> +      const irange &r = as_a <irange> (v);
>> +      if (r.varying_p ())
>> +	hstate.add_int (VR_VARYING);
>> +      else
>> +	hstate.add_int (VR_RANGE);
> 
> Shouldn't this also
>        hstate.add_int (r.num_pairs ());
> ?
> Or is that unnecessary because different number of add_wide_int
> calls will likely result in different hashes then?

That was my thinking, and we could save one write.

I can add the num_pairs() if you prefer.  I don't have a strong opinion.

Aldy

Re: [PATCH] Add inchash support for vrange.

[Jakub Jelinek]

On Tue, Apr 18, 2023 at 01:33:47PM +0200, Aldy Hernandez wrote:
> > > +      const irange &r = as_a <irange> (v);
> > > +      if (r.varying_p ())
> > > +	hstate.add_int (VR_VARYING);
> > > +      else
> > > +	hstate.add_int (VR_RANGE);
> > 
> > Shouldn't this also
> >        hstate.add_int (r.num_pairs ());
> > ?
> > Or is that unnecessary because different number of add_wide_int
> > calls will likely result in different hashes then?
> 
> That was my thinking, and we could save one write.
> 
> I can add the num_pairs() if you prefer.  I don't have a strong opinion.

Me neither.  Let's go with your version then.

	Jakub

Re: [PATCH] Add support for vrange streaming.

[Aldy Hernandez]

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



On 4/18/23 11:06, Aldy Hernandez wrote:
> I think it's time for the ranger folk to start owning range streaming
> instead of passes (IPA, etc) doing their own thing.  I have plans for
> overhauling the IPA code later this cycle to support generic ranges,
> and I'd like to start cleaning up the streaming and hashing interface.
> 
> This patch adds generic streaming support for vrange.
> 
> I'd appreciate another set of eyes.
> 
> Thoughts?

We recently added support for querying and storing an frange's NAN 
without the need to be friends with the class.

Adjusted patch in testing...

Aldy

[-- Attachment #2: 0001-Add-support-for-vrange-streaming.patch --]
[-- Type: text/x-patch, Size: 6442 bytes --]

From ad69f19f80b719880bcad659a07f00724522019d Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Tue, 18 Apr 2023 07:57:43 +0200
Subject: [PATCH] Add support for vrange streaming.

I think it's time for the ranger folk to start owning range streaming
instead of passes (IPA, etc) doing their own thing.  I have plans for
overhauling the IPA code later this cycle to support generic ranges,
and I'd like to start cleaning up the streaming and hashing interface.

This patch adds generic streaming support for vrange.

gcc/ChangeLog:

	* data-streamer-in.cc (streamer_read_real_value): New.
	(streamer_read_value_range): New.
	* data-streamer-out.cc (streamer_write_real_value): New.
	(streamer_write_vrange): New.
	* data-streamer.h (streamer_write_vrange): New.
	(streamer_read_value_range): New.
---
 gcc/data-streamer-in.cc  | 55 ++++++++++++++++++++++++++++++++++++++++
 gcc/data-streamer-out.cc | 51 +++++++++++++++++++++++++++++++++++++
 gcc/data-streamer.h      |  3 +++
 3 files changed, 109 insertions(+)

diff --git a/gcc/data-streamer-in.cc b/gcc/data-streamer-in.cc
index 8ebcac43479..07728bef413 100644
--- a/gcc/data-streamer-in.cc
+++ b/gcc/data-streamer-in.cc
@@ -28,6 +28,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple.h"
 #include "cgraph.h"
 #include "data-streamer.h"
+#include "value-range.h"
+#include "streamer-hooks.h"
 
 /* Read a string from the string table in DATA_IN using input block
    IB.  Write the length to RLEN.  */
@@ -206,6 +208,59 @@ streamer_read_gcov_count (class lto_input_block *ib)
   return ret;
 }
 
+/* Read REAL_VALUE_TYPE from IB.  */
+
+void
+streamer_read_real_value (class lto_input_block *ib, REAL_VALUE_TYPE *r)
+{
+  struct bitpack_d bp = streamer_read_bitpack (ib);
+  bp_unpack_real_value (&bp, r);
+}
+
+void
+streamer_read_value_range (class lto_input_block *ib, data_in *data_in,
+			   Value_Range &vr)
+{
+  // Read the common fields to all vranges.
+  value_range_kind kind = streamer_read_enum (ib, value_range_kind, VR_LAST);
+  gcc_checking_assert (kind != VR_UNDEFINED);
+  tree type = stream_read_tree (ib, data_in);
+
+  // Initialize the Value_Range to the correct type.
+  vr.set_type (type);
+
+  if (is_a <irange> (vr))
+    {
+      irange &r = as_a <irange> (vr);
+      r.set_undefined ();
+      unsigned HOST_WIDE_INT num_pairs = streamer_read_uhwi (ib);
+      for (unsigned i = 0; i < num_pairs; ++i)
+	{
+	  wide_int lb = streamer_read_wide_int (ib);
+	  wide_int ub = streamer_read_wide_int (ib);
+	  int_range<2> tmp (type, lb, ub);
+	  r.union_ (tmp);
+	}
+      wide_int nz = streamer_read_wide_int (ib);
+      r.set_nonzero_bits (nz);
+      return;
+    }
+  if (is_a <frange> (vr))
+    {
+      frange &r = as_a <frange> (vr);
+      REAL_VALUE_TYPE lb, ub;
+      streamer_read_real_value (ib, &lb);
+      streamer_read_real_value (ib, &ub);
+      struct bitpack_d bp = streamer_read_bitpack (ib);
+      bool pos_nan = (bool) bp_unpack_value (&bp, 1);
+      bool neg_nan = (bool) bp_unpack_value (&bp, 1);
+      nan_state nan (pos_nan, neg_nan);
+      r.set (type, lb, ub, nan);
+      return;
+    }
+  gcc_unreachable ();
+}
+
 /* Read the physical representation of a wide_int val from
    input block IB.  */
 
diff --git a/gcc/data-streamer-out.cc b/gcc/data-streamer-out.cc
index cd25745b8dc..afc9862062b 100644
--- a/gcc/data-streamer-out.cc
+++ b/gcc/data-streamer-out.cc
@@ -28,6 +28,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple.h"
 #include "cgraph.h"
 #include "data-streamer.h"
+#include "value-range.h"
+#include "streamer-hooks.h"
 
 
 /* Adds a new block to output stream OBS.  */
@@ -392,6 +394,55 @@ streamer_write_data_stream (struct lto_output_stream *obs, const void *data,
     }
 }
 
+/* Write REAL_VALUE_TYPE into OB.  */
+
+void
+streamer_write_real_value (struct output_block *ob, const REAL_VALUE_TYPE *r)
+{
+  bitpack_d bp = bitpack_create (ob->main_stream);
+  bp_pack_real_value (&bp, r);
+  streamer_write_bitpack (&bp);
+}
+
+void
+streamer_write_vrange (struct output_block *ob, const vrange &v)
+{
+  gcc_checking_assert (!v.undefined_p ());
+
+  // Write the common fields to all vranges.
+  value_range_kind kind = v.varying_p () ? VR_VARYING : VR_RANGE;
+  streamer_write_enum (ob->main_stream, value_range_kind, VR_LAST, kind);
+  stream_write_tree (ob, v.type (), true);
+
+  if (is_a <irange> (v))
+    {
+      const irange &r = as_a <irange> (v);
+      streamer_write_uhwi (ob, r.num_pairs ());
+      for (unsigned i = 0; i < r.num_pairs (); ++i)
+	{
+	  streamer_write_wide_int (ob, r.lower_bound (i));
+	  streamer_write_wide_int (ob, r.upper_bound (i));
+	}
+      streamer_write_wide_int (ob, r.get_nonzero_bits ());
+      return;
+    }
+  if (is_a <frange> (v))
+    {
+      const frange &r = as_a <frange> (v);
+      REAL_VALUE_TYPE lb = r.lower_bound ();
+      REAL_VALUE_TYPE ub = r.upper_bound ();
+      streamer_write_real_value (ob, &lb);
+      streamer_write_real_value (ob, &ub);
+      bitpack_d bp = bitpack_create (ob->main_stream);
+      nan_state nan = r.get_nan_state ();
+      bp_pack_value (&bp, nan.pos_p (), 1);
+      bp_pack_value (&bp, nan.neg_p (), 1);
+      streamer_write_bitpack (&bp);
+      return;
+    }
+  gcc_unreachable ();
+}
+
 /* Emit the physical representation of wide_int VAL to output block OB.  */
 
 void
diff --git a/gcc/data-streamer.h b/gcc/data-streamer.h
index 19c9d6ea606..7e69eb9992b 100644
--- a/gcc/data-streamer.h
+++ b/gcc/data-streamer.h
@@ -75,6 +75,7 @@ void streamer_write_data_stream (struct lto_output_stream *, const void *,
 				 size_t);
 void streamer_write_wide_int (struct output_block *, const wide_int &);
 void streamer_write_widest_int (struct output_block *, const widest_int &);
+void streamer_write_vrange (struct output_block *, const class vrange &);
 
 /* In data-streamer-in.cc  */
 const char *streamer_read_string (class data_in *, class lto_input_block *);
@@ -91,6 +92,8 @@ poly_int64 streamer_read_poly_int64 (class lto_input_block *);
 gcov_type streamer_read_gcov_count (class lto_input_block *);
 wide_int streamer_read_wide_int (class lto_input_block *);
 widest_int streamer_read_widest_int (class lto_input_block *);
+void streamer_read_value_range (class lto_input_block *, class data_in *,
+				class Value_Range &);
 
 /* Returns a new bit-packing context for bit-packing into S.  */
 inline struct bitpack_d
-- 
2.39.2

Re: [PATCH] Add support for vrange streaming.

[Richard Biener]

On Tue, Apr 18, 2023 at 2:48 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
>
>
> On 4/18/23 11:06, Aldy Hernandez wrote:
> > I think it's time for the ranger folk to start owning range streaming
> > instead of passes (IPA, etc) doing their own thing.  I have plans for
> > overhauling the IPA code later this cycle to support generic ranges,
> > and I'd like to start cleaning up the streaming and hashing interface.
> >
> > This patch adds generic streaming support for vrange.
> >
> > I'd appreciate another set of eyes.
> >
> > Thoughts?
>
> We recently added support for querying and storing an frange's NAN
> without the need to be friends with the class.
>
> Adjusted patch in testing...

I think this is reasonable once you find use for it.

Thanks,
Richard.

> Aldy

Re: [PATCH] Add support for vrange streaming.

[Aldy Hernandez]

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

Thanks. I will put it aside until I start posting the IPA patches.

Aldy

On Thu, Apr 27, 2023, 13:02 Richard Biener <richard.guenther@gmail.com>
wrote:

> On Tue, Apr 18, 2023 at 2:48 PM Aldy Hernandez <aldyh@redhat.com> wrote:
> >
> >
> >
> > On 4/18/23 11:06, Aldy Hernandez wrote:
> > > I think it's time for the ranger folk to start owning range streaming
> > > instead of passes (IPA, etc) doing their own thing.  I have plans for
> > > overhauling the IPA code later this cycle to support generic ranges,
> > > and I'd like to start cleaning up the streaming and hashing interface.
> > >
> > > This patch adds generic streaming support for vrange.
> > >
> > > I'd appreciate another set of eyes.
> > >
> > > Thoughts?
> >
> > We recently added support for querying and storing an frange's NAN
> > without the need to be friends with the class.
> >
> > Adjusted patch in testing...
>
> I think this is reasonable once you find use for it.
>
> Thanks,
> Richard.
>
> > Aldy
>
>

Re: [PATCH] Add support for vrange streaming.

[Aldy Hernandez]

I'm pushing this in preparation for further changes in this area later today.

Aldy

On Thu, Apr 27, 2023 at 1:36 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> Thanks. I will put it aside until I start posting the IPA patches.
>
> Aldy
>
> On Thu, Apr 27, 2023, 13:02 Richard Biener <richard.guenther@gmail.com> wrote:
>>
>> On Tue, Apr 18, 2023 at 2:48 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>> >
>> >
>> >
>> > On 4/18/23 11:06, Aldy Hernandez wrote:
>> > > I think it's time for the ranger folk to start owning range streaming
>> > > instead of passes (IPA, etc) doing their own thing.  I have plans for
>> > > overhauling the IPA code later this cycle to support generic ranges,
>> > > and I'd like to start cleaning up the streaming and hashing interface.
>> > >
>> > > This patch adds generic streaming support for vrange.
>> > >
>> > > I'd appreciate another set of eyes.
>> > >
>> > > Thoughts?
>> >
>> > We recently added support for querying and storing an frange's NAN
>> > without the need to be friends with the class.
>> >
>> > Adjusted patch in testing...
>>
>> I think this is reasonable once you find use for it.
>>
>> Thanks,
>> Richard.
>>
>> > Aldy
>>