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

[Aldy Hernandez]

On 10/8/20 4:39 PM, Jakub Jelinek wrote:
> 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 ;).

I can do it once the dust settles.

> 
>>> --- 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]

Ah, I see.  Do you mind commenting that?  Or perhaps you could spell it 
out obviously like:

if (max == 0) {
	...
	if (DEFINED_VALUE_AT_ZERO)
		// do special things
	...
}

But whatever is fine.  I hope to never look at these bits ever again :).

Aldy