On 06/29/2011 12:35 PM, Sebastian Pop wrote:
2011-06-29 Sebastian Pop<sebastian....@amd.com>
* graphite-clast-to-gimple.c (precision_for_value): Removed.
(precision_for_interval): Removed.
(gcc_type_for_interval): Use mpz_sizeinbase.
-/* Return a type that could represent the integer value VAL. */
+/* Return a type that could represent the values between LOW and UP.
+ The value of LOW can be bigger than UP. */
static tree
gcc_type_for_interval (mpz_t low, mpz_t up)
{
Hi Sebastian,
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)
{
- 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;
(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 that the calculation in precision_for_interval was incorrect
(or at least not understandable for me)?
The rest of the patch looks good.
Cheers
Tobi
