Re: [PATCH] [PR tree-optimization/18639] Compare nonzero bits in irange with widest_int.

[Jakub Jelinek <jakub@redhat.com>]

On Fri, Feb 03, 2023 at 11:23:28AM -0500, Andrew MacLeod wrote:
> 
> On 2/3/23 04:16, Jakub Jelinek wrote:
> > On Fri, Feb 03, 2023 at 09:50:43AM +0100, Aldy Hernandez wrote:
> > > [PR tree-optimization/18639] Compare nonzero bits in irange with widest_int.
> > 0 missing in the bug number in the subject line, though the current
> > recommended formatting of the subject is I think:
> > value-range: Compare nonzero bits in irange with widest_int [PR180639]
> >          PR 108639/tree-optimization
> > 
> > Reversed component and number
> > 
> > > --- a/gcc/value-range.cc
> > > +++ b/gcc/value-range.cc
> > > @@ -1259,7 +1259,10 @@ irange::legacy_equal_p (const irange &other) const
> > >   			       other.tree_lower_bound (0))
> > >   	  && vrp_operand_equal_p (tree_upper_bound (0),
> > >   				  other.tree_upper_bound (0))
> > > -	  && get_nonzero_bits () == other.get_nonzero_bits ());
> > > +	  && (widest_int::from (get_nonzero_bits (),
> > > +				TYPE_SIGN (type ()))
> > > +	      == widest_int::from (other.get_nonzero_bits (),
> > > +				   TYPE_SIGN (other.type ()))));
> > >   }
> > >   bool
> > > @@ -1294,7 +1297,11 @@ irange::operator== (const irange &other) const
> > >   	  || !operand_equal_p (ub, ub_other, 0))
> > >   	return false;
> > >       }
> > > -  return get_nonzero_bits () == other.get_nonzero_bits ();
> > > +  widest_int nz1 = widest_int::from (get_nonzero_bits (),
> > > +				     TYPE_SIGN (type ()));
> > > +  widest_int nz2 = widest_int::from (other.get_nonzero_bits (),
> > > +				     TYPE_SIGN (other.type ()));
> > > +  return nz1 == nz2;
> > >   }
> > While the above avoids the ICE (and would be certainly correct for
> > the bounds, depending on the sign of their type sign or zero extended
> > to widest int), but is the above what we want for non-zero bits
> > to be considered equal?  The wide_ints (which ought to have precision
> > of the corresponding type) don't represent normal numbers but bitmasks,
> > 0 - this bit is known to be zero, 1 - nothing is known about this bit).
> > So, if there are different precisions and the narrower value has 0
> > in the MSB of the bitmask (so MSB is known to be zero), the above requires
> > for equality that in the other range all upper bits are known to be zero
> > too for both signed and unsigned.  That is ok.  Similarly for MSB set
> > if TYPE_SIGN of the narrower is unsigned, the MSB value is unknown, but we
> > require on the wider to have all the upper bits cleared.  But for signed
> > narrower type with MSB set, i.e. it is unknown if it is positive or
> > negative, the above requires that all the above bits are unknown too.
> > And that is the case I'm not sure about, whether in that case the
> > upper bits of the wider wide_int should be checked at all.
> > Though, perhaps from the POV of nonzero bits derived from the sign-extended
> > values in the ranges sign bit copies (so all above bits 1) is what one would
> > get, so maybe it is ok.  Just food for thought.
> > 
> if the bits match exactly along with everything else, then we can be sure
> the ranges are truly equal.  If for some reason the numbers are all the same
> but the non-zero bits don't compare equal,  then I can't think of what harm
> it could cause to compare unequal..  Worst case is we dont perform some
> optimization in this extremely rare scenario of differing precisions.  And
> in fact they could actually be unequal...
> 
> So I suspect this is fine...

Ok then.

	Jakub