Re: [PATCH] vrp: Fix up gcc.target/aarch64/pr90838.c [PR97312, PR94801]

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Oct 08, 2020 at 04:28:37PM +0200, Aldy Hernandez wrote:
> On 10/8/20 3:54 PM, Jakub Jelinek wrote:
> > On Thu, Oct 08, 2020 at 12:22:11PM +0200, Jakub Jelinek via Gcc-patches wrote:
> > > Perhaps another way out of this would be document and enforce that
> > > __builtin_c[lt]z{,l,ll} etc calls are undefined at zero, but C[TL]Z ifn
> > > calls are defined there based on *_DEFINED_VALUE_AT_ZERO (*) == 2
> 
> Huh, that magic 2 is not obvious.  I guess we should document the values of
> this macro in the source somewhere:

The 2 is documented in gccint documentation.

> BTW.  There's no reason why the vr-values can't just call the
> gimple_ranger::range_of_builtin_call.  In the original implementation we had
> vr-values call the ranger version and trap if they differed.  I'm pretty
> sure you can have vr-values call range_of_builtin_call with a value_range,
> and things will get squashed down appropriately.  We should really only have
> one version of this.  I'm not suggesting you do it, but I wouldn't object to
> it ;-).

Will defer that to you or Andrew ;).

> > --- gcc/gimple-range.cc.jj	2020-10-08 11:55:25.498313173 +0200
> > +++ gcc/gimple-range.cc	2020-10-08 15:36:14.926945183 +0200
> 
> > @@ -714,8 +730,14 @@ gimple_ranger::range_of_builtin_call (ir
> >   	  // the maximum.
> >   	  wide_int max = r.upper_bound ();
> >   	  if (max == 0)
> > -	    break;
> > -	  maxi = wi::floor_log2 (max);
> > +	    {
> > +	      if (mini == -1)
> > +		maxi = -1;
> > +	      else if (maxi == prec)
> > +		mini = prec;
> > +	    }
> > +	  else if (maxi != prec)
> > +	    maxi = wi::floor_log2 (max);
> 
> Hmmm, if max == 0, that means the range is [0, 0], because if the highest
> bound of r is 0, there's nothing left on the bottom but another 0 since R is
> unsigned.  Is that what you meant?

Yes, for max == 0 aka [0, 0] I wanted:
1) if mini == -1, i.e. the DEFINED_VALUE_AT_ZERO == 2 VALUE is -1, return [-1, -1]
2) if maxi == prec, i.e. DEFINED_VALUE_AT_ZERO == 2 VALUE is prec, return [prec, prec]
3) otherwise it is an UB case, ignore the argument range, so either [0, prec-1] or
   VARYING (the latter for the mini == -2 case)
The 1) and 2) cases would be well defined, and for 3) I'm worried that e.g.
during VRP iteration if at one point we see range of argument say [0, 1] and
determine for that [0, prec-1] range, then in another iteration the argument
range is narrowed to just [0, 0] and all of sudden the result would become
VARYING, I'd be afraid that would be against the rules.  Perhaps the right
thing is to set range to UNDEFINED in the 3) case.

	Jakub