This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [1/4] [patch, middle-end] Make mpz_set/get_double_int functions non-static.


Hello,

> I'm not entirely certain that this is the best way this handles 
> get_type_bounds, though it's the best I could think of.  This is 
> currently a static function that returns the upper and lower bounds of 
> an integer type, in mpz_t form -- or, more accurately, what it returns 
> are the maximum and minimum representable values for an unsigned integer 
> or a two's-complement signed integer using the full bitfield used by the 
> type; there is of course no guarantee that these are in fact within the 
> legal range of the type.  It's used in mpz_to_double_int to determine if 
> a value is in range or not; it's also used elsewhere in 
> tree-ssa-loop-niter.c.  Because of the caveats about it not really 
> returning the upper and lower bounds, I don't think it's a useful 
> function to export generally, so I simply duplicated it as a static 
> function in double-int.c and left the original in tree-ssa-loop-niter.c.

now, this looks wrong to me.  I do not really want to argue whether it is
"useful function to export generally", however, obviously you need the
functionality at at least two places.  If you prefer, change the name of
the function/improve the comments to reflect your doubts (i.e., that it
does not really return bounds of the type, but rather minimum and
maximum value of the realization of the type), but just cloning the
function whenever you need it somewhere is IMHO not a good idea.

> +double_int
> +mpz_get_double_int (tree type, mpz_t val, bool wrap)
> +{
> +  mpz_t min, max;
> +  unsigned HOST_WIDE_INT *vp;
> +  size_t count, numb;
> +  double_int res;
> +
> +  if (!wrap)
> +    {  
> +      mpz_init (min);
> +      mpz_init (max);
> +      get_type_bounds (type, min, max);
> +
> +      if (mpz_cmp (val, min) < 0)
> +	mpz_set (val, min);
> +      else if (mpz_cmp (val, max) > 0)
> +	mpz_set (val, max);
> +    }
> +
...
> +  if (!wrap)
> +    {
> +      gcc_assert (count <= 2);
> +      mpz_clear (min);
> +      mpz_clear (max);
> +    }

Although this is correct, calling mpz_clear already inside the first if
seems more natural.

Zdenek


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]