[PATCH 4/6] Fix computation of precision.

[Sebastian Pop]

On Wed, Jun 29, 2011 at 18:16, Tobias Grosser <tobias@grosser.es> wrote:
> why do we continue to call low 'low' and up 'up', if we actually just have
> two values v1 and v2 where we do not know which one is larger? I think this
> wrong and probably comes because we pass the lower loop bound to val_one and
> the upper loop bound to val_two.
>
> What about:
>
> +/* Return a type that could represent all values between VAL_ONE and
> +   VAL_TWO including VAL_ONE and VAL_TWO itself.  There is no
> +   constraint on which of the two values is larger.  */
>
>  static tree
> - gcc_type_for_interval (mpz_t low, mpz_t up)
> + gcc_type_for_interval (mpz_t val_one, mpz_t val_two)
>   {
>

Sounds good.  I will change the patch.

>> -  bool unsigned_p = true;
>> -  int precision, prec_up, prec_int;
>> +  bool unsigned_p;
>>    tree type;
>>    enum machine_mode mode;
>> -
>> -  gcc_assert (mpz_cmp (low, up)<= 0);
>> -
>> -  prec_up = precision_for_value (up);
>> -  prec_int = precision_for_interval (low, up);
>> -  precision = MAX (prec_up, prec_int);
>> +  int precision = MAX (mpz_sizeinbase (low, 2),
>> +                      mpz_sizeinbase (up, 2));
>>
>>    if (precision>  BITS_PER_WORD)
>>      {
>> @@ -452,14 +397,10 @@ gcc_type_for_interval (mpz_t low, mpz_t up)
>>        return integer_type_node;
>>      }
>>
>> -  if (mpz_sgn (low)<= 0)
>> -    unsigned_p = false;
>> -
>> -  else if (precision<  BITS_PER_WORD)
>> -    {
>> -      unsigned_p = false;
>> -      precision++;
>> -    }
>> +  if (mpz_cmp (low, up)<= 0)
>> +    unsigned_p = (mpz_sgn (low)>= 0);
>> +  else
>> +    unsigned_p = (mpz_sgn (up)>= 0);
>
> What about?
>
>       unsigned_p = value_min(low, up) >= 0;

Ok.

>
> (You need to move the implementation of value_min to this patch)
>
>>
>>    mode = smallest_mode_for_size (precision, MODE_INT);
>>    precision = GET_MODE_PRECISION (mode);
>
> In general the new implementation looks a lot more elegant as the old one.
> What was the problem with the old one? That low could be larger than up and

I don't think that could happen, given that we have a
gcc_assert (mpz_cmp (low, up)<= 0);

> that the calculation in precision_for_interval was incorrect (or at least
> not understandable for me)?

There was an off by one problem in the computation of precision exposed
by the patch "Compute LB and UB of a CLAST expression".

Sebastian