Re: Re[2]: Add double_int_not, double_int_lshift and double_int_rshift functions.

[Richard Guenther <richard dot guenther at gmail dot com>]

On Tue, Apr 13, 2010 at 11:01 PM, Anatoly Sokolov <aesok@post.ru> wrote:
> Hello.
>
>
>> 2010/4/10 Anatoly Sokolov <aesok@post.ru>:
>>>
>>> ?This patch add double_int_not, double_int_lshift and double_int_rshift
>>> functions in the GCC and do clean up code by using double_int_* functions.
>>>
>>> ?Bootstrapped/regtested on x86_64-unknown-linux-gnu, OK for mainline?
>
>> Ok.
>
>> Thanks for these cleanups. ?Can you move both tree_to_double_int
>> and double_int_negative_p to double-int.h from double-int.c as
>> a followup? ?A patch to do so is pre-approved.
>
> ?The double_int_negative_p function is converted to static inline.
>
> ?The tree_to_double_int function can not be defined as inline in the
> double-int.h file since it uses the TREE_INT_CST macro defined in the tree.h
> file but the double-int.h should be included before tree.h.
>
> ?Can move the tree_to_double_int function to the tree.h file, but there
> would be logical to move the double_int_to_tree and double_int_fits_to_tree_p
> functions in tree.h/tree.c also. What is your opinion?

I think tree_to_double_int could be a macro in double-int.h, like

#define tree_to_double_int (cst) (TREE_INT_CST (cst))

as it is just a fancy name for that.  The patch is ok, and the suggested
change is pre-approved.

Thanks,
Richard.

>> And there is a pasto in the description of double_int_rshift, rshift_double
>> explicitly requires COUNT to be positive. ?It would also have been better to
>> keep the same prototype ('int' for arith) or change everything to 'bool'.
>>
>> "Shift the A left"
>>
>> "Shift A left" would be better.
>>
>>
>> "Shift the A rigth"
>>
>> "Shift A right"
>
> Done.
>
> ?New patch. Bootstrapped/regtested on x86_64-unknown-linux-gnu, OK?
>
> ? ? ? ?* double-int.h (HOST_BITS_PER_DOUBLE_INT): Define.
> ? ? ? ?(double_int_not, double_int_lshift, double_int_rshift): Declare.
> ? ? ? ?(double_int_negative_p): Convert to static inline function.
> ? ? ? ?* double-int.c (double_int_lshift, double_int_lshift): Add new function.
> ? ? ? ?(double_int_negative_p): Remove.
> ? ? ? ?* tree.h (lshift_double, rshift_double):
> ? ? ? ?* tree.c (build_low_bits_mask): Clean up, use double_int_* functions.
> ? ? ? ?* fold-const.c (fold_convert_const_int_from_real,
> ? ? ? ?fold_convert_const_int_from_fixed, div_if_zero_remainder): (Ditto.).
> ? ? ? ?(lshift_double): Change type of arith argument to bool.
> ? ? ? ?(rshift_double): Change type of arith argument to bool. Correct
> ? ? ? ?comment.
> ? ? ? ?* expmed.c (mask_rtx, lshift_value): (Ditto.).
>
>
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c ?(revision 158274)
> +++ gcc/tree.c ?(working copy)
> @@ -1221,32 +1221,18 @@
> ?tree
> ?build_low_bits_mask (tree type, unsigned bits)
> ?{
> - ?unsigned HOST_WIDE_INT low;
> - ?HOST_WIDE_INT high;
> - ?unsigned HOST_WIDE_INT all_ones = ~(unsigned HOST_WIDE_INT) 0;
> + ?double_int mask;
>
> ? gcc_assert (bits <= TYPE_PRECISION (type));
>
> ? if (bits == TYPE_PRECISION (type)
> ? ? ? && !TYPE_UNSIGNED (type))
> - ? ?{
> - ? ? ?/* Sign extended all-ones mask. ?*/
> - ? ? ?low = all_ones;
> - ? ? ?high = -1;
> - ? ?}
> - ?else if (bits <= HOST_BITS_PER_WIDE_INT)
> - ? ?{
> - ? ? ?low = all_ones >> (HOST_BITS_PER_WIDE_INT - bits);
> - ? ? ?high = 0;
> - ? ?}
> + ? ?/* Sign extended all-ones mask. ?*/
> + ? ?mask = double_int_minus_one;
> ? else
> - ? ?{
> - ? ? ?bits -= HOST_BITS_PER_WIDE_INT;
> - ? ? ?low = all_ones;
> - ? ? ?high = all_ones >> (HOST_BITS_PER_WIDE_INT - bits);
> - ? ?}
> + ? ?mask = double_int_mask (bits);
>
> - ?return build_int_cst_wide (type, low, high);
> + ?return build_int_cst_wide (type, mask.low, mask.high);
> ?}
>
> ?/* Checks that X is integer constant that can be expressed in (unsigned)
> Index: gcc/tree.h
> ===================================================================
> --- gcc/tree.h ?(revision 158274)
> +++ gcc/tree.h ?(working copy)
> @@ -4841,10 +4841,10 @@
> ? mul_double_with_sign (l1, h1, l2, h2, lv, hv, false)
> ?extern void lshift_double (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
> ? ? ? ? ? ? ? ? ? ? ? ? ? HOST_WIDE_INT, unsigned int,
> - ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned HOST_WIDE_INT *, HOST_WIDE_INT *, int);
> + ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned HOST_WIDE_INT *, HOST_WIDE_INT *, bool);
> ?extern void rshift_double (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
> ? ? ? ? ? ? ? ? ? ? ? ? ? HOST_WIDE_INT, unsigned int,
> - ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned HOST_WIDE_INT *, HOST_WIDE_INT *, int);
> + ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned HOST_WIDE_INT *, HOST_WIDE_INT *, bool);
> ?extern void lrotate_double (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?HOST_WIDE_INT, unsigned int,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned HOST_WIDE_INT *, HOST_WIDE_INT *);
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c ? ?(revision 158274)
> +++ gcc/fold-const.c ? ?(working copy)
> @@ -436,7 +436,7 @@
> ?void
> ?lshift_double (unsigned HOST_WIDE_INT l1, HOST_WIDE_INT h1,
> ? ? ? ? ? ? ? HOST_WIDE_INT count, unsigned int prec,
> - ? ? ? ? ? ? ?unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv, int arith)
> + ? ? ? ? ? ? ?unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv, bool arith)
> ?{
> ? unsigned HOST_WIDE_INT signmask;
>
> @@ -491,7 +491,7 @@
> ?}
>
> ?/* Shift the doubleword integer in L1, H1 right by COUNT places
> - ? keeping only PREC bits of result. ?COUNT must be positive.
> + ? keeping only PREC bits of result. ?Shift left if COUNT is negative.
> ? ?ARITH nonzero specifies arithmetic shifting; otherwise use logical shift.
> ? ?Store the value as two `HOST_WIDE_INT' pieces in *LV and *HV. ?*/
>
> @@ -499,7 +499,7 @@
> ?rshift_double (unsigned HOST_WIDE_INT l1, HOST_WIDE_INT h1,
> ? ? ? ? ? ? ? HOST_WIDE_INT count, unsigned int prec,
> ? ? ? ? ? ? ? unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv,
> - ? ? ? ? ? ? ?int arith)
> + ? ? ? ? ? ? ?bool arith)
> ?{
> ? unsigned HOST_WIDE_INT signmask;
>
> @@ -881,10 +881,7 @@
> ?tree
> ?div_if_zero_remainder (enum tree_code code, const_tree arg1, const_tree arg2)
> ?{
> - ?unsigned HOST_WIDE_INT int1l, int2l;
> - ?HOST_WIDE_INT int1h, int2h;
> - ?unsigned HOST_WIDE_INT quol, reml;
> - ?HOST_WIDE_INT quoh, remh;
> + ?double_int quo, rem;
> ? int uns;
>
> ? /* The sign of the division is according to operand two, that
> @@ -895,17 +892,14 @@
> ? ? ? && TYPE_IS_SIZETYPE (TREE_TYPE (arg2)))
> ? ? uns = false;
>
> - ?int1l = TREE_INT_CST_LOW (arg1);
> - ?int1h = TREE_INT_CST_HIGH (arg1);
> - ?int2l = TREE_INT_CST_LOW (arg2);
> - ?int2h = TREE_INT_CST_HIGH (arg2);
> + ?quo = double_int_divmod (tree_to_double_int (arg1),
> + ? ? ? ? ? ? ? ? ? ? ? ? ?tree_to_double_int (arg2),
> + ? ? ? ? ? ? ? ? ? ? ? ? ?uns, code, &rem);
>
> - ?div_and_round_double (code, uns, int1l, int1h, int2l, int2h,
> - ? ? ? ? ? ? ? ? ? ? ? &quol, &quoh, &reml, &remh);
> - ?if (remh != 0 || reml != 0)
> - ? ?return NULL_TREE;
> + ?if (double_int_zero_p (rem))
> + ? ?return build_int_cst_wide (TREE_TYPE (arg1), quo.low, quo.high);
>
> - ?return build_int_cst_wide (TREE_TYPE (arg1), quol, quoh);
> + ?return NULL_TREE;
> ?}
>
> ?/* This is nonzero if we should defer warnings about undefined
> @@ -2279,7 +2273,7 @@
> ? ? ?C and C++ standards that simply state that the behavior of
> ? ? ?FP-to-integer conversion is unspecified upon overflow. ?*/
>
> - ?HOST_WIDE_INT high, low;
> + ?double_int val;
> ? REAL_VALUE_TYPE r;
> ? REAL_VALUE_TYPE x = TREE_REAL_CST (arg1);
>
> @@ -2297,8 +2291,7 @@
> ? if (REAL_VALUE_ISNAN (r))
> ? ? {
> ? ? ? overflow = 1;
> - ? ? ?high = 0;
> - ? ? ?low = 0;
> + ? ? ?val = double_int_zero;
> ? ? }
>
> ? /* See if R is less than the lower bound or greater than the
> @@ -2311,8 +2304,7 @@
> ? ? ? if (REAL_VALUES_LESS (r, l))
> ? ? ? ?{
> ? ? ? ? ?overflow = 1;
> - ? ? ? ? high = TREE_INT_CST_HIGH (lt);
> - ? ? ? ? low = TREE_INT_CST_LOW (lt);
> + ? ? ? ? val = tree_to_double_int (lt);
> ? ? ? ?}
> ? ? }
>
> @@ -2325,16 +2317,15 @@
> ? ? ? ? ?if (REAL_VALUES_LESS (u, r))
> ? ? ? ? ? ?{
> ? ? ? ? ? ? ?overflow = 1;
> - ? ? ? ? ? ? high = TREE_INT_CST_HIGH (ut);
> - ? ? ? ? ? ? low = TREE_INT_CST_LOW (ut);
> + ? ? ? ? ? ? val = tree_to_double_int (ut);
> ? ? ? ? ? ?}
> ? ? ? ?}
> ? ? }
>
> ? if (! overflow)
> - ? ?REAL_VALUE_TO_INT (&low, &high, r);
> + ? ?real_to_integer2 ((HOST_WIDE_INT *) &val.low, &val.high, &r);
>
> - ?t = force_fit_type_double (type, low, high, -1,
> + ?t = force_fit_type_double (type, val.low, val.high, -1,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? overflow | TREE_OVERFLOW (arg1));
> ? return t;
> ?}
> @@ -2354,39 +2345,32 @@
> ? mode = TREE_FIXED_CST (arg1).mode;
> ? if (GET_MODE_FBIT (mode) < 2 * HOST_BITS_PER_WIDE_INT)
> ? ? {
> - ? ? ?lshift_double (temp.low, temp.high,
> - ? ? ? ? ? ? ? ? ? ?- GET_MODE_FBIT (mode), 2 * HOST_BITS_PER_WIDE_INT,
> - ? ? ? ? ? ? ? ? ? ?&temp.low, &temp.high, SIGNED_FIXED_POINT_MODE_P (mode));
> + ? ? ?temp = double_int_rshift (temp, GET_MODE_FBIT (mode),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? HOST_BITS_PER_DOUBLE_INT,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SIGNED_FIXED_POINT_MODE_P (mode));
>
> ? ? ? /* Left shift temp to temp_trunc by fbit. ?*/
> - ? ? ?lshift_double (temp.low, temp.high,
> - ? ? ? ? ? ? ? ? ? ?GET_MODE_FBIT (mode), 2 * HOST_BITS_PER_WIDE_INT,
> - ? ? ? ? ? ? ? ? ? ?&temp_trunc.low, &temp_trunc.high,
> - ? ? ? ? ? ? ? ? ? ?SIGNED_FIXED_POINT_MODE_P (mode));
> + ? ? ?temp_trunc = double_int_lshift (temp, GET_MODE_FBIT (mode),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? HOST_BITS_PER_DOUBLE_INT,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SIGNED_FIXED_POINT_MODE_P (mode));
> ? ? }
> ? else
> ? ? {
> - ? ? ?temp.low = 0;
> - ? ? ?temp.high = 0;
> - ? ? ?temp_trunc.low = 0;
> - ? ? ?temp_trunc.high = 0;
> + ? ? ?temp = double_int_zero;
> + ? ? ?temp_trunc = double_int_zero;
> ? ? }
>
> ? /* If FIXED_CST is negative, we need to round the value toward 0.
> ? ? ?By checking if the fractional bits are not zero to add 1 to temp. ?*/
> - ?if (SIGNED_FIXED_POINT_MODE_P (mode) && temp_trunc.high < 0
> + ?if (SIGNED_FIXED_POINT_MODE_P (mode)
> + ? ? ?&& double_int_negative_p (temp_trunc)
> ? ? ? && !double_int_equal_p (TREE_FIXED_CST (arg1).data, temp_trunc))
> - ? ?{
> - ? ? ?double_int one;
> - ? ? ?one.low = 1;
> - ? ? ?one.high = 0;
> - ? ? ?temp = double_int_add (temp, one);
> - ? ?}
> + ? ?temp = double_int_add (temp, double_int_one);
>
> ? /* Given a fixed-point constant, make new constant with new type,
> ? ? ?appropriately sign-extended or truncated. ?*/
> ? t = force_fit_type_double (type, temp.low, temp.high, -1,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ?(temp.high < 0
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?(double_int_negative_p (temp)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&& (TYPE_UNSIGNED (type)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?< TYPE_UNSIGNED (TREE_TYPE (arg1))))
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? | TREE_OVERFLOW (arg1));
> Index: gcc/double-int.c
> ===================================================================
> --- gcc/double-int.c ? ?(revision 158274)
> +++ gcc/double-int.c ? ?(working copy)
> @@ -290,6 +290,30 @@
> ? return double_int_mod (a, b, true, code);
> ?}
>
> +/* Shift A left by COUNT places keeping only PREC bits of result. ?Shift
> + ? right if COUNT is negative. ?ARITH true specifies arithmetic shifting;
> + ? otherwise use logical shift. ?*/
> +
> +double_int
> +double_int_lshift (double_int a, HOST_WIDE_INT count, unsigned int prec, bool arith)
> +{
> + ?double_int ret;
> + ?lshift_double (a.low, a.high, count, prec, &ret.low, &ret.high, arith);
> + ?return ret;
> +}
> +
> +/* Shift A rigth by COUNT places keeping only PREC bits of result. ?Shift
> + ? left if COUNT is negative. ?ARITH true specifies arithmetic shifting;
> + ? otherwise use logical shift. ?*/
> +
> +double_int
> +double_int_rshift (double_int a, HOST_WIDE_INT count, unsigned int prec, bool arith)
> +{
> + ?double_int ret;
> + ?rshift_double (a.low, a.high, count, prec, &ret.low, &ret.high, arith);
> + ?return ret;
> +}
> +
> ?/* Constructs tree in type TYPE from with value given by CST. ?Signedness of CST
> ? ?is assumed to be the same as the signedness of TYPE. ?*/
>
> @@ -314,15 +338,6 @@
> ? return double_int_equal_p (cst, ext);
> ?}
>
> -/* Returns true if CST is negative. ?Of course, CST is considered to
> - ? be signed. ?*/
> -
> -bool
> -double_int_negative_p (double_int cst)
> -{
> - ?return cst.high < 0;
> -}
> -
> ?/* Returns -1 if A < B, 0 if A == B and 1 if A > B. ?Signedness of the
> ? ?comparison is given by UNS. ?*/
>
> Index: gcc/double-int.h
> ===================================================================
> --- gcc/double-int.h ? ?(revision 158274)
> +++ gcc/double-int.h ? ?(working copy)
> @@ -57,6 +57,8 @@
> ? HOST_WIDE_INT high;
> ?} double_int;
>
> +#define HOST_BITS_PER_DOUBLE_INT (2 * HOST_BITS_PER_WIDE_INT)
> +
> ?union tree_node;
>
> ?/* Constructors and conversions. ?*/
> @@ -127,7 +129,29 @@
> ?double_int double_int_divmod (double_int, double_int, bool, unsigned, double_int *);
> ?double_int double_int_sdivmod (double_int, double_int, unsigned, double_int *);
> ?double_int double_int_udivmod (double_int, double_int, unsigned, double_int *);
> -bool double_int_negative_p (double_int);
> +
> +/* Logical operations. ?*/
> +static inline double_int
> +double_int_not (double_int a)
> +{
> + ?a.low = ~a.low;
> + ?a.high = ~ a.high;
> + ?return a;
> +}
> +
> +/* Shift operations. ?*/
> +double_int double_int_lshift (double_int, HOST_WIDE_INT, unsigned int, bool);
> +double_int double_int_rshift (double_int, HOST_WIDE_INT, unsigned int, bool);
> +
> +/* Returns true if CST is negative. ?Of course, CST is considered to
> + ? be signed. ?*/
> +
> +static inline bool
> +double_int_negative_p (double_int cst)
> +{
> + ?return cst.high < 0;
> +}
> +
> ?int double_int_cmp (double_int, double_int, bool);
> ?int double_int_scmp (double_int, double_int);
> ?int double_int_ucmp (double_int, double_int);
> Index: gcc/expmed.c
> ===================================================================
> --- gcc/expmed.c ? ? ? ?(revision 158274)
> +++ gcc/expmed.c ? ? ? ?(working copy)
> @@ -1839,39 +1839,15 @@
> ?static rtx
> ?mask_rtx (enum machine_mode mode, int bitpos, int bitsize, int complement)
> ?{
> - ?HOST_WIDE_INT masklow, maskhigh;
> + ?double_int mask;
>
> - ?if (bitsize == 0)
> - ? ?masklow = 0;
> - ?else if (bitpos < HOST_BITS_PER_WIDE_INT)
> - ? ?masklow = (HOST_WIDE_INT) -1 << bitpos;
> - ?else
> - ? ?masklow = 0;
> + ?mask = double_int_mask (bitsize);
> + ?mask = double_int_lshift (mask, bitpos, HOST_BITS_PER_DOUBLE_INT, false);
>
> - ?if (bitpos + bitsize < HOST_BITS_PER_WIDE_INT)
> - ? ?masklow &= ((unsigned HOST_WIDE_INT) -1
> - ? ? ? ? ? ? ? >> (HOST_BITS_PER_WIDE_INT - bitpos - bitsize));
> -
> - ?if (bitpos <= HOST_BITS_PER_WIDE_INT)
> - ? ?maskhigh = -1;
> - ?else
> - ? ?maskhigh = (HOST_WIDE_INT) -1 << (bitpos - HOST_BITS_PER_WIDE_INT);
> -
> - ?if (bitsize == 0)
> - ? ?maskhigh = 0;
> - ?else if (bitpos + bitsize > HOST_BITS_PER_WIDE_INT)
> - ? ?maskhigh &= ((unsigned HOST_WIDE_INT) -1
> - ? ? ? ? ? ? ? ?>> (2 * HOST_BITS_PER_WIDE_INT - bitpos - bitsize));
> - ?else
> - ? ?maskhigh = 0;
> -
> ? if (complement)
> - ? ?{
> - ? ? ?maskhigh = ~maskhigh;
> - ? ? ?masklow = ~masklow;
> - ? ?}
> + ? ?mask = double_int_not (mask);
>
> - ?return immed_double_const (masklow, maskhigh, mode);
> + ?return immed_double_const (mask.low, mask.high, mode);
> ?}
>
> ?/* Return a constant integer (CONST_INT or CONST_DOUBLE) rtx with the value
> @@ -1880,24 +1856,12 @@
> ?static rtx
> ?lshift_value (enum machine_mode mode, rtx value, int bitpos, int bitsize)
> ?{
> - ?unsigned HOST_WIDE_INT v = INTVAL (value);
> - ?HOST_WIDE_INT low, high;
> + ?double_int val;
> +
> + ?val = double_int_zext (uhwi_to_double_int (INTVAL (value)), bitsize);
> + ?val = double_int_lshift (val, bitpos, HOST_BITS_PER_DOUBLE_INT, false);
>
> - ?if (bitsize < HOST_BITS_PER_WIDE_INT)
> - ? ?v &= ~((HOST_WIDE_INT) -1 << bitsize);
> -
> - ?if (bitpos < HOST_BITS_PER_WIDE_INT)
> - ? ?{
> - ? ? ?low = v << bitpos;
> - ? ? ?high = (bitpos > 0 ? (v >> (HOST_BITS_PER_WIDE_INT - bitpos)) : 0);
> - ? ?}
> - ?else
> - ? ?{
> - ? ? ?low = 0;
> - ? ? ?high = v << (bitpos - HOST_BITS_PER_WIDE_INT);
> - ? ?}
> -
> - ?return immed_double_const (low, high, mode);
> + ?return immed_double_const (val.low, val.high, mode);
> ?}
>
> ?/* Extract a bit field that is split across two words
>
>
> ?Anatoly.
>
>