public inbox for java-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Remove build_int_cst_wide_type function.
@ 2010-05-07 18:59 Anatoly Sokolov
  2010-05-07 20:31 ` Richard Guenther
  0 siblings, 1 reply; 10+ messages in thread
From: Anatoly Sokolov @ 2010-05-07 18:59 UTC (permalink / raw)
  To: gcc-patches, java-patches; +Cc: Richard Guenther

Hello.


  The build_int_cst_wide_type and double_int_to_tree functions do the same
work but differ only in argument types. The build_int_cst_wide_type get
INT_CST as doule_int type, build_int_cst_wide_type as pair HOST_WIDE_INT.
This patch change all uses build_int_cst_wide_type with double_int_to_tree and
remove build_int_cst_wide_type function.

  Bootstrapped/regtested on x86_64-unknown-linux-gnu for c, c++ and java.

  OK for mainline?

        * double-int.h (double_int_ior): New function.
        * tree.h (build_int_cst_wide_type): Remove.
        * tree.c (build_int_cst_wide_type): Remove.
        (double_int_to_tree, double_int_fits_to_tree_p): Handle size types as
        sign extended.
        * fold-const.c (native_interpret_int): Use double_int_to_tree instead
        of build_int_cst_wide_type.
        * dojump.c (do_jump): (Ditto.).
        * stor-layout.c (set_sizetype): (Ditto.).
        
/java   
        * jcf-parse.c (get_constant): Use double_int_to_tree instead of
        build_int_cst_wide_type.


Index: gcc/java/jcf-parse.c
===================================================================
--- gcc/java/jcf-parse.c        (revision 159109)
+++ gcc/java/jcf-parse.c        (working copy)
@@ -1040,14 +1040,15 @@
       }
     case CONSTANT_Long:
       {
-       unsigned HOST_WIDE_INT num = JPOOL_UINT (jcf, index);
-       unsigned HOST_WIDE_INT lo;
-       HOST_WIDE_INT hi;
-       
-       lshift_double (num, 0, 32, 64, &lo, &hi, 0);
-       num = JPOOL_UINT (jcf, index+1);
-       add_double (lo, hi, num, 0, &lo, &hi);
-       value = build_int_cst_wide_type (long_type_node, lo, hi);
+       unsigned HOST_WIDE_INT num;
+       double_int val;
+
+       num = JPOOL_UINT (jcf, index);
+       val = double_int_lshift (uhwi_to_double_int (num), 32, 64, false);
+       num = JPOOL_UINT (jcf, index + 1);
+       val = double_int_ior (val, uhwi_to_double_int (num));
+
+       value = double_int_to_tree (long_type_node, val);
        break;
       }
 
Index: gcc/tree.c
===================================================================
--- gcc/tree.c  (revision 159109)
+++ gcc/tree.c  (working copy)
@@ -1071,25 +1071,19 @@
   return build_int_cst_wide (type, low1, hi);
 }
 
-/* Create an INT_CST node of TYPE and value HI:LOW.  The value is truncated
-   and sign extended according to the value range of TYPE.  */
-
-tree
-build_int_cst_wide_type (tree type,
-                        unsigned HOST_WIDE_INT low, HOST_WIDE_INT high)
-{
-  fit_double_type (low, high, &low, &high, type);
-  return build_int_cst_wide (type, low, high);
-}
-
 /* 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.  */
 
 tree
 double_int_to_tree (tree type, double_int cst)
 {
-  cst = double_int_ext (cst, TYPE_PRECISION (type), TYPE_UNSIGNED (type));
+  /* Size types *are* sign extended.  */
+  bool sign_extended_type = (!TYPE_UNSIGNED (type)
+                            || (TREE_CODE (type) == INTEGER_TYPE
+                                && TYPE_IS_SIZETYPE (type)));
 
+  cst = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
+
   return build_int_cst_wide (type, cst.low, cst.high);
 }
 
@@ -1099,10 +1093,14 @@
 bool
 double_int_fits_to_tree_p (const_tree type, double_int cst)
 {
-  double_int ext = double_int_ext (cst,
-                                  TYPE_PRECISION (type),
-                                  TYPE_UNSIGNED (type));
+  /* Size types *are* sign extended.  */
+  bool sign_extended_type = (!TYPE_UNSIGNED (type)
+                            || (TREE_CODE (type) == INTEGER_TYPE
+                                && TYPE_IS_SIZETYPE (type)));
 
+  double_int ext 
+    = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
+
   return double_int_equal_p (cst, ext);
 }
 
Index: gcc/tree.h
===================================================================
--- gcc/tree.h  (revision 159109)
+++ gcc/tree.h  (working copy)
@@ -4013,8 +4013,6 @@
 extern tree build_int_cst_type (tree, HOST_WIDE_INT);
 extern tree build_int_cstu (tree, unsigned HOST_WIDE_INT);
 extern tree build_int_cst_wide (tree, unsigned HOST_WIDE_INT, HOST_WIDE_INT);
-extern tree build_int_cst_wide_type (tree,
-                                    unsigned HOST_WIDE_INT, HOST_WIDE_INT);
 extern tree build_vector (tree, tree);
 extern tree build_vector_from_ctor (tree, VEC(constructor_elt,gc) *);
 extern tree build_constructor (tree, VEC(constructor_elt,gc) *);
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 159109)
+++ gcc/fold-const.c    (working copy)
@@ -7408,13 +7408,14 @@
   int total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
   int byte, offset, word, words;
   unsigned char value;
-  unsigned int HOST_WIDE_INT lo = 0;
-  HOST_WIDE_INT hi = 0;
+  double_int result;
 
   if (total_bytes > len)
     return NULL_TREE;
   if (total_bytes * BITS_PER_UNIT > 2 * HOST_BITS_PER_WIDE_INT)
     return NULL_TREE;
+
+  result = double_int_zero;
   words = total_bytes / UNITS_PER_WORD;
 
   for (byte = 0; byte < total_bytes; byte++)
@@ -7436,13 +7437,13 @@
       value = ptr[offset];
 
       if (bitpos < HOST_BITS_PER_WIDE_INT)
-       lo |= (unsigned HOST_WIDE_INT) value << bitpos;
+       result.low |= (unsigned HOST_WIDE_INT) value << bitpos;
       else
-       hi |= (unsigned HOST_WIDE_INT) value
-             << (bitpos - HOST_BITS_PER_WIDE_INT);
+       result.high |= (unsigned HOST_WIDE_INT) value
+                      << (bitpos - HOST_BITS_PER_WIDE_INT);
     }
 
-  return build_int_cst_wide_type (type, lo, hi);
+  return double_int_to_tree (type, result);
 }
 
 
Index: gcc/dojump.c
===================================================================
--- gcc/dojump.c        (revision 159109)
+++ gcc/dojump.c        (working copy)
@@ -539,10 +539,11 @@
                  && prefer_and_bit_test (TYPE_MODE (argtype),
                                          TREE_INT_CST_LOW (shift)))
                {
-                 unsigned HOST_WIDE_INT mask
-                   = (unsigned HOST_WIDE_INT) 1 << TREE_INT_CST_LOW (shift);
+                 double_int mask
+                   = double_int_setbit (double_int_zero, 
+                                        TREE_INT_CST_LOW (shift));
                  do_jump (build2 (BIT_AND_EXPR, argtype, arg,
-                                  build_int_cst_wide_type (argtype, mask, 0)),
+                                  double_int_to_tree (argtype, mask)),
                           clr_label, set_label, setclr_prob);
                  break;
                }
Index: gcc/double-int.h
===================================================================
--- gcc/double-int.h    (revision 159109)
+++ gcc/double-int.h    (working copy)
@@ -126,6 +126,7 @@
 double_int double_int_setbit (double_int, unsigned);
 
 /* Logical operations.  */
+
 static inline double_int
 double_int_not (double_int a)
 {
@@ -134,6 +135,14 @@
   return a;
 }
 
+static inline double_int
+double_int_ior (double_int a, double_int b)
+{
+  a.low |= b.low;
+  a.high |= b.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);
Index: gcc/stor-layout.c
===================================================================
--- gcc/stor-layout.c   (revision 159109)
+++ gcc/stor-layout.c   (working copy)
@@ -2275,9 +2275,7 @@
      sign-extended in a way consistent with force_fit_type.  */
   max = TYPE_MAX_VALUE (sizetype);
   TYPE_MAX_VALUE (sizetype)
-    = build_int_cst_wide_type (sizetype,
-                              TREE_INT_CST_LOW (max),
-                              TREE_INT_CST_HIGH (max));
+    = double_int_to_tree (sizetype, tree_to_double_int (max));
 
   t = make_node (INTEGER_TYPE);
   TYPE_NAME (t) = get_identifier ("bit_size_type");


Anatoly.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Remove build_int_cst_wide_type function.
  2010-05-07 18:59 Remove build_int_cst_wide_type function Anatoly Sokolov
@ 2010-05-07 20:31 ` Richard Guenther
  2010-05-10  9:24   ` Anatoly Sokolov
  2010-05-19 20:00   ` Re[2]: " Anatoly Sokolov
  0 siblings, 2 replies; 10+ messages in thread
From: Richard Guenther @ 2010-05-07 20:31 UTC (permalink / raw)
  To: Anatoly Sokolov; +Cc: gcc-patches, java-patches

2010/5/7 Anatoly Sokolov <aesok@post.ru>:
> Hello.
>
>
>  The build_int_cst_wide_type and double_int_to_tree functions do the same
> work but differ only in argument types. The build_int_cst_wide_type get
> INT_CST as doule_int type, build_int_cst_wide_type as pair HOST_WIDE_INT.
> This patch change all uses build_int_cst_wide_type with double_int_to_tree and
> remove build_int_cst_wide_type function.
>
>  Bootstrapped/regtested on x86_64-unknown-linux-gnu for c, c++ and java.
>
>  OK for mainline?

Comments inline

>        * double-int.h (double_int_ior): New function.
>        * tree.h (build_int_cst_wide_type): Remove.
>        * tree.c (build_int_cst_wide_type): Remove.
>        (double_int_to_tree, double_int_fits_to_tree_p): Handle size types as
>        sign extended.
>        * fold-const.c (native_interpret_int): Use double_int_to_tree instead
>        of build_int_cst_wide_type.
>        * dojump.c (do_jump): (Ditto.).
>        * stor-layout.c (set_sizetype): (Ditto.).
>
> /java
>        * jcf-parse.c (get_constant): Use double_int_to_tree instead of
>        build_int_cst_wide_type.
>
>
> Index: gcc/java/jcf-parse.c
> ===================================================================
> --- gcc/java/jcf-parse.c        (revision 159109)
> +++ gcc/java/jcf-parse.c        (working copy)
> @@ -1040,14 +1040,15 @@
>       }
>     case CONSTANT_Long:
>       {
> -       unsigned HOST_WIDE_INT num = JPOOL_UINT (jcf, index);
> -       unsigned HOST_WIDE_INT lo;
> -       HOST_WIDE_INT hi;
> -
> -       lshift_double (num, 0, 32, 64, &lo, &hi, 0);
> -       num = JPOOL_UINT (jcf, index+1);
> -       add_double (lo, hi, num, 0, &lo, &hi);
> -       value = build_int_cst_wide_type (long_type_node, lo, hi);
> +       unsigned HOST_WIDE_INT num;
> +       double_int val;
> +
> +       num = JPOOL_UINT (jcf, index);
> +       val = double_int_lshift (uhwi_to_double_int (num), 32, 64, false);
> +       num = JPOOL_UINT (jcf, index + 1);
> +       val = double_int_ior (val, uhwi_to_double_int (num));
> +
> +       value = double_int_to_tree (long_type_node, val);
>        break;
>       }
>
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c  (revision 159109)
> +++ gcc/tree.c  (working copy)
> @@ -1071,25 +1071,19 @@
>   return build_int_cst_wide (type, low1, hi);
>  }
>
> -/* Create an INT_CST node of TYPE and value HI:LOW.  The value is truncated
> -   and sign extended according to the value range of TYPE.  */
> -
> -tree
> -build_int_cst_wide_type (tree type,
> -                        unsigned HOST_WIDE_INT low, HOST_WIDE_INT high)
> -{
> -  fit_double_type (low, high, &low, &high, type);
> -  return build_int_cst_wide (type, low, high);
> -}
> -
>  /* 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.  */
>
>  tree
>  double_int_to_tree (tree type, double_int cst)
>  {
> -  cst = double_int_ext (cst, TYPE_PRECISION (type), TYPE_UNSIGNED (type));
> +  /* Size types *are* sign extended.  */
> +  bool sign_extended_type = (!TYPE_UNSIGNED (type)
> +                            || (TREE_CODE (type) == INTEGER_TYPE
> +                                && TYPE_IS_SIZETYPE (type)));

whoops - thanks for catching this.

> +  cst = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
> +
>   return build_int_cst_wide (type, cst.low, cst.high);
>  }
>
> @@ -1099,10 +1093,14 @@
>  bool
>  double_int_fits_to_tree_p (const_tree type, double_int cst)
>  {
> -  double_int ext = double_int_ext (cst,
> -                                  TYPE_PRECISION (type),
> -                                  TYPE_UNSIGNED (type));
> +  /* Size types *are* sign extended.  */
> +  bool sign_extended_type = (!TYPE_UNSIGNED (type)
> +                            || (TREE_CODE (type) == INTEGER_TYPE
> +                                && TYPE_IS_SIZETYPE (type)));
>
> +  double_int ext
> +    = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
> +
>   return double_int_equal_p (cst, ext);
>  }
>
> Index: gcc/tree.h
> ===================================================================
> --- gcc/tree.h  (revision 159109)
> +++ gcc/tree.h  (working copy)
> @@ -4013,8 +4013,6 @@
>  extern tree build_int_cst_type (tree, HOST_WIDE_INT);
>  extern tree build_int_cstu (tree, unsigned HOST_WIDE_INT);
>  extern tree build_int_cst_wide (tree, unsigned HOST_WIDE_INT, HOST_WIDE_INT);
> -extern tree build_int_cst_wide_type (tree,
> -                                    unsigned HOST_WIDE_INT, HOST_WIDE_INT);
>  extern tree build_vector (tree, tree);
>  extern tree build_vector_from_ctor (tree, VEC(constructor_elt,gc) *);
>  extern tree build_constructor (tree, VEC(constructor_elt,gc) *);
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c    (revision 159109)
> +++ gcc/fold-const.c    (working copy)
> @@ -7408,13 +7408,14 @@
>   int total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
>   int byte, offset, word, words;
>   unsigned char value;
> -  unsigned int HOST_WIDE_INT lo = 0;
> -  HOST_WIDE_INT hi = 0;
> +  double_int result;
>
>   if (total_bytes > len)
>     return NULL_TREE;
>   if (total_bytes * BITS_PER_UNIT > 2 * HOST_BITS_PER_WIDE_INT)
>     return NULL_TREE;
> +
> +  result = double_int_zero;
>   words = total_bytes / UNITS_PER_WORD;
>
>   for (byte = 0; byte < total_bytes; byte++)
> @@ -7436,13 +7437,13 @@
>       value = ptr[offset];
>
>       if (bitpos < HOST_BITS_PER_WIDE_INT)
> -       lo |= (unsigned HOST_WIDE_INT) value << bitpos;
> +       result.low |= (unsigned HOST_WIDE_INT) value << bitpos;
>       else
> -       hi |= (unsigned HOST_WIDE_INT) value
> -             << (bitpos - HOST_BITS_PER_WIDE_INT);
> +       result.high |= (unsigned HOST_WIDE_INT) value
> +                      << (bitpos - HOST_BITS_PER_WIDE_INT);
>     }
>
> -  return build_int_cst_wide_type (type, lo, hi);
> +  return double_int_to_tree (type, result);
>  }
>
>
> Index: gcc/dojump.c
> ===================================================================
> --- gcc/dojump.c        (revision 159109)
> +++ gcc/dojump.c        (working copy)
> @@ -539,10 +539,11 @@
>                  && prefer_and_bit_test (TYPE_MODE (argtype),
>                                          TREE_INT_CST_LOW (shift)))
>                {
> -                 unsigned HOST_WIDE_INT mask
> -                   = (unsigned HOST_WIDE_INT) 1 << TREE_INT_CST_LOW (shift);
> +                 double_int mask
> +                   = double_int_setbit (double_int_zero,
> +                                        TREE_INT_CST_LOW (shift));
>                  do_jump (build2 (BIT_AND_EXPR, argtype, arg,
> -                                  build_int_cst_wide_type (argtype, mask, 0)),
> +                                  double_int_to_tree (argtype, mask)),

Use build_int_cstu instead and retain using HOST_WIDE_INTs.  Btw,
both build_int_cst and build_int_cstu should do proper sign/zero
extension as the types precision may be lower than that of
HOST_WIDE_INT.

>                           clr_label, set_label, setclr_prob);
>                  break;
>                }
> Index: gcc/double-int.h
> ===================================================================
> --- gcc/double-int.h    (revision 159109)
> +++ gcc/double-int.h    (working copy)
> @@ -126,6 +126,7 @@
>  double_int double_int_setbit (double_int, unsigned);
>
>  /* Logical operations.  */
> +
>  static inline double_int
>  double_int_not (double_int a)
>  {
> @@ -134,6 +135,14 @@
>   return a;
>  }
>
> +static inline double_int
> +double_int_ior (double_int a, double_int b)
> +{
> +  a.low |= b.low;
> +  a.high |= b.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);
> Index: gcc/stor-layout.c
> ===================================================================
> --- gcc/stor-layout.c   (revision 159109)
> +++ gcc/stor-layout.c   (working copy)
> @@ -2275,9 +2275,7 @@
>      sign-extended in a way consistent with force_fit_type.  */
>   max = TYPE_MAX_VALUE (sizetype);
>   TYPE_MAX_VALUE (sizetype)
> -    = build_int_cst_wide_type (sizetype,
> -                              TREE_INT_CST_LOW (max),
> -                              TREE_INT_CST_HIGH (max));
> +    = double_int_to_tree (sizetype, tree_to_double_int (max));

Ok with these changes.

Thanks,
Richard.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Remove build_int_cst_wide_type function.
  2010-05-07 20:31 ` Richard Guenther
@ 2010-05-10  9:24   ` Anatoly Sokolov
  2010-05-10  9:34     ` Richard Guenther
  2010-05-19 20:00   ` Re[2]: " Anatoly Sokolov
  1 sibling, 1 reply; 10+ messages in thread
From: Anatoly Sokolov @ 2010-05-10  9:24 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, java-patches

Hi.

----- Original Message ----- 
From: "Richard Guenther" <richard.guenther@gmail.com>
To: "Anatoly Sokolov" <aesok@post.ru>
Cc: <gcc-patches@gcc.gnu.org>; <java-patches@gcc.gnu.org>
Sent: Saturday, May 08, 2010 12:31 AM
Subject: Re: Remove build_int_cst_wide_type function.


> Use build_int_cstu instead and retain using HOST_WIDE_INTs.  Btw,
> both build_int_cst and build_int_cstu should do proper sign/zero
> extension as the types precision may be lower than that of
> HOST_WIDE_INT.

  The build_int_cst and build_int_cstu function don't clear extra bits 
outside
of the type precision. The build_int_ct_type function truncate signed
HOST_WIDE_INT constant to the type precision. Comment before 
build_int_cst_type
say:
/* ...  Constants
   with these extra bits may confuse the fold so that it detects overflows
   even in cases when they do not occur, and in general should be avoided.
   We cannot however make this a default behavior of build_int_cst without
   more intrusive changes, since there are parts of gcc that rely on the 
extra
   precision of the integer constants.  */

  Consequently, need both build_int_cst and build_int_cstu functions which
don't truncate constant and new build_int_cstu_type function which truncate
unsigned HOST_WIDE_INT constants.

  How about rename build_int_cst_type/build_int_cstu_type function to
hwi_to_tree/uhwi_to_tree by analogy with the double_int_to_tree function?

Anatoly. 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Remove build_int_cst_wide_type function.
  2010-05-10  9:24   ` Anatoly Sokolov
@ 2010-05-10  9:34     ` Richard Guenther
  2010-05-10 10:24       ` Anatoly Sokolov
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Guenther @ 2010-05-10  9:34 UTC (permalink / raw)
  To: Anatoly Sokolov; +Cc: gcc-patches, java-patches

On Mon, May 10, 2010 at 11:24 AM, Anatoly Sokolov <aesok@post.ru> wrote:
> Hi.
>
> ----- Original Message ----- From: "Richard Guenther"
> <richard.guenther@gmail.com>
> To: "Anatoly Sokolov" <aesok@post.ru>
> Cc: <gcc-patches@gcc.gnu.org>; <java-patches@gcc.gnu.org>
> Sent: Saturday, May 08, 2010 12:31 AM
> Subject: Re: Remove build_int_cst_wide_type function.
>
>
>> Use build_int_cstu instead and retain using HOST_WIDE_INTs.  Btw,
>> both build_int_cst and build_int_cstu should do proper sign/zero
>> extension as the types precision may be lower than that of
>> HOST_WIDE_INT.
>
>  The build_int_cst and build_int_cstu function don't clear extra bits
> outside
> of the type precision. The build_int_ct_type function truncate signed
> HOST_WIDE_INT constant to the type precision. Comment before
> build_int_cst_type
> say:
> /* ...  Constants
>  with these extra bits may confuse the fold so that it detects overflows
>  even in cases when they do not occur, and in general should be avoided.
>  We cannot however make this a default behavior of build_int_cst without
>  more intrusive changes, since there are parts of gcc that rely on the extra
>  precision of the integer constants.  */
>
>  Consequently, need both build_int_cst and build_int_cstu functions which
> don't truncate constant and new build_int_cstu_type function which truncate
> unsigned HOST_WIDE_INT constants.
>
>  How about rename build_int_cst_type/build_int_cstu_type function to
> hwi_to_tree/uhwi_to_tree by analogy with the double_int_to_tree function?

I don't think the comment is valid.  The problem is that INTEGER_CSTs
are shared and non-canonical ones should never be entered into the
shared pool.  Which means that build_int_cst_wide should assert
that the constant is properly zero-/sign-extended according to its
type (at least with ENABLE_CHECKING).  Currently build_int_cst_wide
is used as an optimization (to avoid the zero-/sign-extension when its
already done), but really it is the low-level interface that shouldn't
be exposed ... (we have way too many ways to build INTEGER_CSTs...)

Richard.

> Anatoly.
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Remove build_int_cst_wide_type function.
  2010-05-10  9:34     ` Richard Guenther
@ 2010-05-10 10:24       ` Anatoly Sokolov
  2010-05-10 10:34         ` Richard Guenther
  0 siblings, 1 reply; 10+ messages in thread
From: Anatoly Sokolov @ 2010-05-10 10:24 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, java-patches

Hi.

----- Original Message ----- 
From: "Richard Guenther" <richard.guenther@gmail.com>
To: "Anatoly Sokolov" <aesok@post.ru>
Cc: <gcc-patches@gcc.gnu.org>; <java-patches@gcc.gnu.org>
Sent: Monday, May 10, 2010 1:33 PM
Subject: Re: Remove build_int_cst_wide_type function.


>> On Mon, May 10, 2010 at 11:24 AM, Anatoly Sokolov <aesok@post.ru> wrote:
>> Hi.
>>
>>
>>> Use build_int_cstu instead and retain using HOST_WIDE_INTs. Btw,
>>> both build_int_cst and build_int_cstu should do proper sign/zero
>>> extension as the types precision may be lower than that of
>>> HOST_WIDE_INT.
>>
>> The build_int_cst and build_int_cstu function don't clear extra bits
>> outside
>> of the type precision. The build_int_ct_type function truncate signed
>> HOST_WIDE_INT constant to the type precision. Comment before
>> build_int_cst_type
>> say:
>> /* ... Constants
>> with these extra bits may confuse the fold so that it detects overflows
>> even in cases when they do not occur, and in general should be avoided.
>> We cannot however make this a default behavior of build_int_cst without
>> more intrusive changes, since there are parts of gcc that rely on the 
>> extra
>> precision of the integer constants. */
>>
>> Consequently, need both build_int_cst and build_int_cstu functions which
>> don't truncate constant and new build_int_cstu_type function which 
>> truncate
>> unsigned HOST_WIDE_INT constants.
>>
>> How about rename build_int_cst_type/build_int_cstu_type function to
>> hwi_to_tree/uhwi_to_tree by analogy with the double_int_to_tree function?
>
> I don't think the comment is valid.  The problem is that INTEGER_CSTs
> are shared and non-canonical ones should never be entered into the
> shared pool.  Which means that build_int_cst_wide should assert
> that the constant is properly zero-/sign-extended according to its
> type (at least with ENABLE_CHECKING).  Currently build_int_cst_wide
> is used as an optimization (to avoid the zero-/sign-extension when its
> already done), but really it is the low-level interface that shouldn't
> be exposed ... (we have way too many ways to build INTEGER_CSTs...)

Hence, build_int_cst_type/build_int_cstu_type function should truncate
constants to the type precision and build_int_cst_type should be removed?

Can I remame build_int_cst/build_int_cstu functions to 
hwi_to_tree/uhwi_to_tree?

Anatoly.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Remove build_int_cst_wide_type function.
  2010-05-10 10:24       ` Anatoly Sokolov
@ 2010-05-10 10:34         ` Richard Guenther
  2010-05-10 11:17           ` Anatoly Sokolov
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Guenther @ 2010-05-10 10:34 UTC (permalink / raw)
  To: Anatoly Sokolov; +Cc: gcc-patches, java-patches

On Mon, May 10, 2010 at 12:24 PM, Anatoly Sokolov <aesok@post.ru> wrote:
> Hi.
>
> ----- Original Message ----- From: "Richard Guenther"
> <richard.guenther@gmail.com>
> To: "Anatoly Sokolov" <aesok@post.ru>
> Cc: <gcc-patches@gcc.gnu.org>; <java-patches@gcc.gnu.org>
> Sent: Monday, May 10, 2010 1:33 PM
> Subject: Re: Remove build_int_cst_wide_type function.
>
>
>>> On Mon, May 10, 2010 at 11:24 AM, Anatoly Sokolov <aesok@post.ru> wrote:
>>> Hi.
>>>
>>>
>>>> Use build_int_cstu instead and retain using HOST_WIDE_INTs. Btw,
>>>> both build_int_cst and build_int_cstu should do proper sign/zero
>>>> extension as the types precision may be lower than that of
>>>> HOST_WIDE_INT.
>>>
>>> The build_int_cst and build_int_cstu function don't clear extra bits
>>> outside
>>> of the type precision. The build_int_ct_type function truncate signed
>>> HOST_WIDE_INT constant to the type precision. Comment before
>>> build_int_cst_type
>>> say:
>>> /* ... Constants
>>> with these extra bits may confuse the fold so that it detects overflows
>>> even in cases when they do not occur, and in general should be avoided.
>>> We cannot however make this a default behavior of build_int_cst without
>>> more intrusive changes, since there are parts of gcc that rely on the
>>> extra
>>> precision of the integer constants. */
>>>
>>> Consequently, need both build_int_cst and build_int_cstu functions which
>>> don't truncate constant and new build_int_cstu_type function which
>>> truncate
>>> unsigned HOST_WIDE_INT constants.
>>>
>>> How about rename build_int_cst_type/build_int_cstu_type function to
>>> hwi_to_tree/uhwi_to_tree by analogy with the double_int_to_tree function?
>>
>> I don't think the comment is valid.  The problem is that INTEGER_CSTs
>> are shared and non-canonical ones should never be entered into the
>> shared pool.  Which means that build_int_cst_wide should assert
>> that the constant is properly zero-/sign-extended according to its
>> type (at least with ENABLE_CHECKING).  Currently build_int_cst_wide
>> is used as an optimization (to avoid the zero-/sign-extension when its
>> already done), but really it is the low-level interface that shouldn't
>> be exposed ... (we have way too many ways to build INTEGER_CSTs...)
>
> Hence, build_int_cst_type/build_int_cstu_type function should truncate
> constants to the type precision and build_int_cst_type should be removed?

Yes.  Assuming that it works of course - if the comment is
indeed correct and some code expects to build non-canonical
integer-csts that code should probably use double_ints
directly instead of creating trees.  Likely the excess precision
will be for intermediate results only.

> Can I remame build_int_cst/build_int_cstu functions to
> hwi_to_tree/uhwi_to_tree?

Hmm - there are a lot of calls to these, I'd prefer to leave them
as is for now.  If we can make build_int_cst_wide local to tree.c
then maybe - it would be a consistent iterface.  But as long
as that stays I see no reason to do it now.

Thanks,
Richard.

> Anatoly.
>
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Remove build_int_cst_wide_type function.
  2010-05-10 10:34         ` Richard Guenther
@ 2010-05-10 11:17           ` Anatoly Sokolov
  2010-05-10 11:35             ` Richard Guenther
  0 siblings, 1 reply; 10+ messages in thread
From: Anatoly Sokolov @ 2010-05-10 11:17 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, java-patches


----- Original Message ----- 
From: "Richard Guenther" <richard.guenther@gmail.com>
To: "Anatoly Sokolov" <aesok@post.ru>
Cc: <gcc-patches@gcc.gnu.org>; <java-patches@gcc.gnu.org>
Sent: Monday, May 10, 2010 2:34 PM
Subject: Re: Remove build_int_cst_wide_type function.


>> Can I remame build_int_cst/build_int_cstu functions to
>> hwi_to_tree/uhwi_to_tree?
>
> Hmm - there are a lot of calls to these, I'd prefer to leave them
> as is for now.  If we can make build_int_cst_wide local to tree.c
> then maybe - it would be a consistent iterface.  But as long
> as that stays I see no reason to do it now.
>

The build_int_cstu used 24 times only. The build_int_cst used 876 times, but
in most uses in get NULL_TREE as fitst argument that forces to do checks in
the build_int_cst  function:
...
  /* Support legacy code.  */
  if (!type)
    type = integer_type_node;
...

If change NULL_TREE to integer_type_node in all calls the build_int_cst
function then  build_int_cst may be simplified to inline function:

static inline tree
build_int_cst/shwi_to_tree (tree type, HOST_WIDE_INT low)
{
  return double_int_to_tree (type, shwi_to_double_int (low));
}

I.e., can do two changes for the cost of one.

Anatoly. 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Remove build_int_cst_wide_type function.
  2010-05-10 11:17           ` Anatoly Sokolov
@ 2010-05-10 11:35             ` Richard Guenther
  2010-05-10 11:47               ` Anatoly Sokolov
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Guenther @ 2010-05-10 11:35 UTC (permalink / raw)
  To: Anatoly Sokolov; +Cc: gcc-patches, java-patches

On Mon, May 10, 2010 at 1:17 PM, Anatoly Sokolov <aesok@post.ru> wrote:
>
> ----- Original Message ----- From: "Richard Guenther"
> <richard.guenther@gmail.com>
> To: "Anatoly Sokolov" <aesok@post.ru>
> Cc: <gcc-patches@gcc.gnu.org>; <java-patches@gcc.gnu.org>
> Sent: Monday, May 10, 2010 2:34 PM
> Subject: Re: Remove build_int_cst_wide_type function.
>
>
>>> Can I remame build_int_cst/build_int_cstu functions to
>>> hwi_to_tree/uhwi_to_tree?
>>
>> Hmm - there are a lot of calls to these, I'd prefer to leave them
>> as is for now.  If we can make build_int_cst_wide local to tree.c
>> then maybe - it would be a consistent iterface.  But as long
>> as that stays I see no reason to do it now.
>>
>
> The build_int_cstu used 24 times only. The build_int_cst used 876 times, but
> in most uses in get NULL_TREE as fitst argument that forces to do checks in
> the build_int_cst  function:
> ...
>  /* Support legacy code.  */
>  if (!type)
>   type = integer_type_node;
> ...
>
> If change NULL_TREE to integer_type_node in all calls the build_int_cst
> function then  build_int_cst may be simplified to inline function:
>
> static inline tree
> build_int_cst/shwi_to_tree (tree type, HOST_WIDE_INT low)
> {
>  return double_int_to_tree (type, shwi_to_double_int (low));
> }
>
> I.e., can do two changes for the cost of one.

The callers with NULL type should probably be reviewed separately.
Some of them might want to use the simpler size_int interface
(even if that changes the type from integer_type_node to
size_type_node).  Some obvious ones indeed explicitly want
integer_type_node.

Lumping both changes together will make it hard to track down
those persisting errors.  NULL_TREE at least tells us that this
is a suspicious case.

Richard.

> Anatoly.
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Remove build_int_cst_wide_type function.
  2010-05-10 11:35             ` Richard Guenther
@ 2010-05-10 11:47               ` Anatoly Sokolov
  0 siblings, 0 replies; 10+ messages in thread
From: Anatoly Sokolov @ 2010-05-10 11:47 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, java-patches


----- Original Message ----- 
From: "Richard Guenther" <richard.guenther@gmail.com>
To: "Anatoly Sokolov" <aesok@post.ru>
Cc: <gcc-patches@gcc.gnu.org>; <java-patches@gcc.gnu.org>
Sent: Monday, May 10, 2010 3:35 PM
Subject: Re: Remove build_int_cst_wide_type function.



> The callers with NULL type should probably be reviewed separately.
> Some of them might want to use the simpler size_int interface
> (even if that changes the type from integer_type_node to
> size_type_node).  Some obvious ones indeed explicitly want
> integer_type_node.
> 
> Lumping both changes together will make it hard to track down
> those persisting errors.  NULL_TREE at least tells us that this
> is a suspicious case.

Ok. I'm working on these patches.

Anatoly.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re[2]: Remove build_int_cst_wide_type function.
  2010-05-07 20:31 ` Richard Guenther
  2010-05-10  9:24   ` Anatoly Sokolov
@ 2010-05-19 20:00   ` Anatoly Sokolov
  1 sibling, 0 replies; 10+ messages in thread
From: Anatoly Sokolov @ 2010-05-19 20:00 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, java-patches

Hello.


>> Index: gcc/dojump.c
>> ===================================================================
>> --- gcc/dojump.c        (revision 159109)
>> +++ gcc/dojump.c        (working copy)
>> @@ -539,10 +539,11 @@
>>                  && prefer_and_bit_test (TYPE_MODE (argtype),
>>                                          TREE_INT_CST_LOW (shift)))
>>                {
>> -                 unsigned HOST_WIDE_INT mask
>> -                   = (unsigned HOST_WIDE_INT) 1 << TREE_INT_CST_LOW (shift);
>> +                 double_int mask
>> +                   = double_int_setbit (double_int_zero,
>> +                                        TREE_INT_CST_LOW (shift));
>>                  do_jump (build2 (BIT_AND_EXPR, argtype, arg,
>> -                                  build_int_cst_wide_type (argtype, mask, 0)),
>> +                                  double_int_to_tree (argtype, mask)),

> Use build_int_cstu instead and retain using HOST_WIDE_INTs.  Btw,
> both build_int_cst and build_int_cstu should do proper sign/zero
> extension as the types precision may be lower than that of
> HOST_WIDE_INT.



> Ok with these changes.

Committed as.

        * double-int.h (double_int_ior): New function.
        * tree.h (build_int_cst_wide_type): Remove.
        * tree.c (build_int_cst_wide_type): Remove.
        * fold-const.c (native_interpret_int): Use double_int_to_tree instead
        of build_int_cst_wide_type.
        * stor-layout.c (set_sizetype): (Ditto.).
        * dojump.c (do_jump): Use build_int_cstu instead of
        build_int_cst_wide_type.

/java
        * jcf-parse.c (get_constant): Use double_int_to_tree instead of
        build_int_cst_wide_type


Index: gcc/java/jcf-parse.c
===================================================================
--- gcc/java/jcf-parse.c        (revision 159566)
+++ gcc/java/jcf-parse.c        (working copy)
@@ -1040,14 +1040,15 @@
       }
     case CONSTANT_Long:
       {
-       unsigned HOST_WIDE_INT num = JPOOL_UINT (jcf, index);
-       unsigned HOST_WIDE_INT lo;
-       HOST_WIDE_INT hi;
-       
-       lshift_double (num, 0, 32, 64, &lo, &hi, 0);
-       num = JPOOL_UINT (jcf, index+1);
-       add_double (lo, hi, num, 0, &lo, &hi);
-       value = build_int_cst_wide_type (long_type_node, lo, hi);
+       unsigned HOST_WIDE_INT num;
+       double_int val;
+
+       num = JPOOL_UINT (jcf, index);
+       val = double_int_lshift (uhwi_to_double_int (num), 32, 64, false);
+       num = JPOOL_UINT (jcf, index + 1);
+       val = double_int_ior (val, uhwi_to_double_int (num));
+
+       value = double_int_to_tree (long_type_node, val);
        break;
       }
 
Index: gcc/tree.c
===================================================================
--- gcc/tree.c  (revision 159566)
+++ gcc/tree.c  (working copy)
@@ -1063,17 +1063,6 @@
   return build_int_cst_wide (type, low1, hi);
 }
 
-/* Create an INT_CST node of TYPE and value HI:LOW.  The value is truncated
-   and sign extended according to the value range of TYPE.  */
-
-tree
-build_int_cst_wide_type (tree type,
-                        unsigned HOST_WIDE_INT low, HOST_WIDE_INT high)
-{
-  fit_double_type (low, high, &low, &high, type);
-  return build_int_cst_wide (type, low, high);
-}
-
 /* 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.  */
 
Index: gcc/tree.h
===================================================================
--- gcc/tree.h  (revision 159566)
+++ gcc/tree.h  (working copy)
@@ -4012,8 +4012,6 @@
 extern tree build_int_cst (tree, HOST_WIDE_INT);
 extern tree build_int_cst_type (tree, HOST_WIDE_INT);
 extern tree build_int_cst_wide (tree, unsigned HOST_WIDE_INT, HOST_WIDE_INT);
-extern tree build_int_cst_wide_type (tree,
-                                    unsigned HOST_WIDE_INT, HOST_WIDE_INT);
 extern tree build_vector (tree, tree);
 extern tree build_vector_from_ctor (tree, VEC(constructor_elt,gc) *);
 extern tree build_constructor (tree, VEC(constructor_elt,gc) *);
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 159566)
+++ gcc/fold-const.c    (working copy)
@@ -7408,13 +7408,14 @@
   int total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
   int byte, offset, word, words;
   unsigned char value;
-  unsigned int HOST_WIDE_INT lo = 0;
-  HOST_WIDE_INT hi = 0;
+  double_int result;
 
   if (total_bytes > len)
     return NULL_TREE;
   if (total_bytes * BITS_PER_UNIT > 2 * HOST_BITS_PER_WIDE_INT)
     return NULL_TREE;
+
+  result = double_int_zero;
   words = total_bytes / UNITS_PER_WORD;
 
   for (byte = 0; byte < total_bytes; byte++)
@@ -7436,13 +7437,13 @@
       value = ptr[offset];
 
       if (bitpos < HOST_BITS_PER_WIDE_INT)
-       lo |= (unsigned HOST_WIDE_INT) value << bitpos;
+       result.low |= (unsigned HOST_WIDE_INT) value << bitpos;
       else
-       hi |= (unsigned HOST_WIDE_INT) value
-             << (bitpos - HOST_BITS_PER_WIDE_INT);
+       result.high |= (unsigned HOST_WIDE_INT) value
+                      << (bitpos - HOST_BITS_PER_WIDE_INT);
     }
 
-  return build_int_cst_wide_type (type, lo, hi);
+  return double_int_to_tree (type, result);
 }
 
 
Index: gcc/dojump.c
===================================================================
--- gcc/dojump.c        (revision 159566)
+++ gcc/dojump.c        (working copy)
@@ -542,7 +542,7 @@
                  unsigned HOST_WIDE_INT mask
                    = (unsigned HOST_WIDE_INT) 1 << TREE_INT_CST_LOW (shift);
                  do_jump (build2 (BIT_AND_EXPR, argtype, arg,
-                                  build_int_cst_wide_type (argtype, mask, 0)),
+                                  build_int_cstu (argtype, mask)),
                           clr_label, set_label, setclr_prob);
                  break;
                }
Index: gcc/double-int.h
===================================================================
--- gcc/double-int.h    (revision 159566)
+++ gcc/double-int.h    (working copy)
@@ -126,6 +126,9 @@
 double_int double_int_setbit (double_int, unsigned);
 
 /* Logical operations.  */
+
+/* Returns ~A.  */
+
 static inline double_int
 double_int_not (double_int a)
 {
@@ -134,6 +137,16 @@
   return a;
 }
 
+/* Returns A | B.  */
+
+static inline double_int
+double_int_ior (double_int a, double_int b)
+{
+  a.low |= b.low;
+  a.high |= b.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);
Index: gcc/stor-layout.c
===================================================================
--- gcc/stor-layout.c   (revision 159566)
+++ gcc/stor-layout.c   (working copy)
@@ -2275,9 +2275,7 @@
      sign-extended in a way consistent with force_fit_type.  */
   max = TYPE_MAX_VALUE (sizetype);
   TYPE_MAX_VALUE (sizetype)
-    = build_int_cst_wide_type (sizetype,
-                              TREE_INT_CST_LOW (max),
-                              TREE_INT_CST_HIGH (max));
+    = double_int_to_tree (sizetype, tree_to_double_int (max));
 
   t = make_node (INTEGER_TYPE);
   TYPE_NAME (t) = get_identifier ("bit_size_type");


Anatoly.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-05-19 20:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-07 18:59 Remove build_int_cst_wide_type function Anatoly Sokolov
2010-05-07 20:31 ` Richard Guenther
2010-05-10  9:24   ` Anatoly Sokolov
2010-05-10  9:34     ` Richard Guenther
2010-05-10 10:24       ` Anatoly Sokolov
2010-05-10 10:34         ` Richard Guenther
2010-05-10 11:17           ` Anatoly Sokolov
2010-05-10 11:35             ` Richard Guenther
2010-05-10 11:47               ` Anatoly Sokolov
2010-05-19 20:00   ` Re[2]: " Anatoly Sokolov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).