On Wed, Jun 29, 2011 at 18:16, Tobias Grosser <tob...@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
