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.