This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 4/6] Fix computation of precision.
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