[PATCH 4/6] Fix computation of precision.

Tobias Grosser tobias@grosser.es
Thu Jun 30 15:03:00 GMT 2011


On 06/30/2011 09:50 AM, Sebastian Pop wrote:
> 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".

OK. From my side this patch is fine.

Tobi



More information about the Gcc-patches mailing list