[wide-int] Undo some differences with trunk

I've committed a patch upstream to convert TREE_INT_CST_LOW to tree_to_[su]hwi
if there is an obvious tree_fits_[su]hwi_p guard.  There were some other
changes from TREE_INT_CST_LOW to tree_to_[su]hwi that weren't as obvious
and I think we should deal with them separately.  As before, these changes
aren't really related to the wide-int conversion, since the code that uses
them is operating on HWIs rather than double_ints.  The patch is still here,
so it's not like the work is lost.

There were also some changes from TREE_INT_CST_LOW (X) to
TREE_INT_CST_ELT (X, 0), which are no longer needed now that we define
TREE_INT_CST_LOW too.

There was one case where the branch switched a tree_fits_uhwi_p to
a tree_fits_shwi_p, but with a !!! about the original choice.
The two other instances of the !!! sequence kept the original tests,
so I think this might have been unintentional.  If not, then again
it should go on trunk first.

The patch also undoes a couple of other ordering and whitespace differences.

Tested on x86_64-linux-gnu.  OK for wide-int?

Thanks,
Richard

I would leave out the last frag of tree.c.  It actually gets rid of what 
i consider, a latent bug. it does not show up with the current 
implementation of wide-int, but if that implementation changed, it 
would.   The problem is that you really should not expect that you can 
set the min or max value for a type without first setting the precision 
of that type.

Aside from that, this looks fine.

kenny

On 11/20/2013 08:58 AM, Richard Sandiford wrote:
> I've committed a patch upstream to convert TREE_INT_CST_LOW to tree_to_[su]hwi
> if there is an obvious tree_fits_[su]hwi_p guard.  There were some other
> changes from TREE_INT_CST_LOW to tree_to_[su]hwi that weren't as obvious
> and I think we should deal with them separately.  As before, these changes
> aren't really related to the wide-int conversion, since the code that uses
> them is operating on HWIs rather than double_ints.  The patch is still here,
> so it's not like the work is lost.
>
> There were also some changes from TREE_INT_CST_LOW (X) to
> TREE_INT_CST_ELT (X, 0), which are no longer needed now that we define
> TREE_INT_CST_LOW too.
>
> There was one case where the branch switched a tree_fits_uhwi_p to
> a tree_fits_shwi_p, but with a !!! about the original choice.
> The two other instances of the !!! sequence kept the original tests,
> so I think this might have been unintentional.  If not, then again
> it should go on trunk first.
>
> The patch also undoes a couple of other ordering and whitespace differences.
>
> Tested on x86_64-linux-gnu.  OK for wide-int?
>
> Thanks,
> Richard
>
>
> Index: gcc/c-family/cilk.c
> ===================================================================
> --- gcc/c-family/cilk.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/c-family/cilk.c	2013-11-20 13:48:38.491410510 +0000
> @@ -1208,7 +1208,7 @@ extract_free_variables (tree t, struct w
>   	int ii = 0;
>   	if (TREE_CODE (TREE_OPERAND (t, 0)) == INTEGER_CST)
>   	  {
> -	    len = tree_to_uhwi (TREE_OPERAND (t, 0));
> +	    len = TREE_INT_CST_LOW (TREE_OPERAND (t, 0));
>   
>   	    for (ii = 0; ii < len; ii++)
>   	      extract_free_variables (TREE_OPERAND (t, ii), wd, ADD_READ);
> Index: gcc/c/Make-lang.in
> ===================================================================
> --- gcc/c/Make-lang.in	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/c/Make-lang.in	2013-11-20 13:48:38.474410604 +0000
> @@ -137,3 +137,4 @@ c.stageprofile: stageprofile-start
>   	-mv c/*$(objext) stageprofile/c
>   c.stagefeedback: stagefeedback-start
>   	-mv c/*$(objext) stagefeedback/c
> +
> Index: gcc/config/avr/avr.c
> ===================================================================
> --- gcc/config/avr/avr.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/config/avr/avr.c	2013-11-20 13:48:38.523410332 +0000
> @@ -12233,7 +12233,7 @@ avr_fold_builtin (tree fndecl, int n_arg
>             }
>   
>           tmap = wide_int_to_tree (map_type, arg[0]);
> -        map = tree_to_uhwi (tmap);
> +        map = TREE_INT_CST_LOW (tmap);
>   
>           if (TREE_CODE (tval) != INTEGER_CST
>               && 0 == avr_map_metric (map, MAP_MASK_PREIMAGE_F))
> Index: gcc/config/nds32/nds32.c
> ===================================================================
> --- gcc/config/nds32/nds32.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/config/nds32/nds32.c	2013-11-20 13:48:38.492410505 +0000
> @@ -1276,7 +1276,7 @@ nds32_construct_isr_vectors_information
>   	  /* Pick up each vector id value.  */
>   	  id = TREE_VALUE (id_list);
>   	  /* Add vector_number_offset to get actual vector number.  */
> -	  vector_id = tree_to_uhwi (id) + vector_number_offset;
> +	  vector_id = TREE_INT_CST_LOW (id) + vector_number_offset;
>   
>   	  /* Enable corresponding vector and set function name.  */
>   	  nds32_isr_vectors[vector_id].category = (intr)
> @@ -1318,7 +1318,7 @@ nds32_construct_isr_vectors_information
>   
>         /* The total vectors = interrupt + exception numbers + reset.
>            There are 8 exception and 1 reset in nds32 architecture.  */
> -      nds32_isr_vectors[0].total_n_vectors = tree_to_uhwi (id) + 8 + 1;
> +      nds32_isr_vectors[0].total_n_vectors = TREE_INT_CST_LOW (id) + 8 + 1;
>         strcpy (nds32_isr_vectors[0].func_name, func_name);
>   
>         /* Retrieve nmi and warm function.  */
> Index: gcc/cp/parser.c
> ===================================================================
> --- gcc/cp/parser.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/cp/parser.c	2013-11-20 13:48:38.503410443 +0000
> @@ -3804,7 +3804,7 @@ make_string_pack (tree value)
>     tree charvec;
>     tree argpack = make_node (NONTYPE_ARGUMENT_PACK);
>     const char *str = TREE_STRING_POINTER (value);
> -  int sz = tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (value))));
> +  int sz = TREE_INT_CST_LOW (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (value))));
>     int len = TREE_STRING_LENGTH (value) / sz - 1;
>     tree argvec = make_tree_vec (2);
>   
> Index: gcc/cp/vtable-class-hierarchy.c
> ===================================================================
> --- gcc/cp/vtable-class-hierarchy.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/cp/vtable-class-hierarchy.c	2013-11-20 13:48:38.504410438 +0000
> @@ -453,7 +453,7 @@ check_and_record_registered_pairs (tree
>       vptr_address = TREE_OPERAND (vptr_address, 0);
>   
>     if (TREE_OPERAND_LENGTH (vptr_address) > 1)
> -    offset = tree_to_uhwi (TREE_OPERAND (vptr_address, 1));
> +    offset = TREE_INT_CST_LOW (TREE_OPERAND (vptr_address, 1));
>     else
>       offset = 0;
>   
> @@ -876,7 +876,7 @@ output_set_info (tree record_type, vec<t
>               vptr_name = IDENTIFIER_POINTER (DECL_NAME (arg0));
>   
>             if (TREE_CODE (arg1) == INTEGER_CST)
> -            vptr_offset = tree_to_uhwi (arg1);
> +            vptr_offset = TREE_INT_CST_LOW (arg1);
>           }
>   
>         snprintf (buffer, sizeof (buffer), "%s %s %s + %d\n",
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/dwarf2out.c	2013-11-20 13:48:38.509410410 +0000
> @@ -15112,7 +15112,7 @@ insert_wide_int (const wide_int &val, un
>         return;
>       }
>   
> -  // We'd have to extend this code to support odd sizes.
> +  /* We'd have to extend this code to support odd sizes.  */
>     gcc_assert (elt_size % (HOST_BITS_PER_WIDE_INT/BITS_PER_UNIT) == 0);
>   
>     int n = elt_size / (HOST_BITS_PER_WIDE_INT/BITS_PER_UNIT);
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/fold-const.c	2013-11-20 13:48:38.512410393 +0000
> @@ -13044,7 +13044,7 @@ fold_binary_loc (location_t loc,
>   	      unsigned HOST_WIDE_INT log2 = tree_log2 (arg01);
>   	      /* If (C2 << C1) doesn't overflow, then ((X >> C1) & C2) != 0
>   		 can be rewritten as (X & (C2 << C1)) != 0.  */
> -	      if ((log2 + tree_to_uhwi (arg001)) < prec)
> +	      if ((log2 + TREE_INT_CST_LOW (arg001)) < prec)
>   		{
>   		  tem = fold_build2_loc (loc, LSHIFT_EXPR, itype, arg01, arg001);
>   		  tem = fold_build2_loc (loc, BIT_AND_EXPR, itype, arg000, tem);
> @@ -16155,7 +16155,7 @@ fold_read_from_constant_string (tree exp
>   	  && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (string)))) == 1))
>   	return build_int_cst_type (TREE_TYPE (exp),
>   				   (TREE_STRING_POINTER (string)
> -				    [tree_to_uhwi (index)]));
> +				    [TREE_INT_CST_LOW (index)]));
>       }
>     return NULL;
>   }
> Index: gcc/fortran/trans-common.c
> ===================================================================
> --- gcc/fortran/trans-common.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/fortran/trans-common.c	2013-11-20 13:48:38.512410393 +0000
> @@ -400,8 +400,8 @@ build_common_decl (gfc_common_head *com,
>   	gfc_warning ("Named COMMON block '%s' at %L shall be of the "
>   		     "same size as elsewhere (%lu vs %lu bytes)", com->name,
>   		     &com->where,
> -		     (unsigned long) tree_to_uhwi (size),
> -		     (unsigned long) tree_to_uhwi (DECL_SIZE_UNIT (decl)));
> +		     (unsigned long) TREE_INT_CST_LOW (size),
> +		     (unsigned long) TREE_INT_CST_LOW (DECL_SIZE_UNIT (decl)));
>   
>         if (tree_int_cst_lt (DECL_SIZE_UNIT (decl), size))
>   	{
> Index: gcc/fortran/trans-expr.c
> ===================================================================
> --- gcc/fortran/trans-expr.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/fortran/trans-expr.c	2013-11-20 13:48:38.525410321 +0000
> @@ -2645,8 +2645,8 @@ gfc_string_to_single_character (tree len
>         && TREE_CODE (TREE_OPERAND (TREE_OPERAND (str, 0), 0)) == STRING_CST
>         && array_ref_low_bound (TREE_OPERAND (str, 0))
>   	 == TREE_OPERAND (TREE_OPERAND (str, 0), 1)
> -      && tree_to_uhwi (len) > 1
> -      && tree_to_uhwi (len)
> +      && TREE_INT_CST_LOW (len) > 1
> +      && TREE_INT_CST_LOW (len)
>   	 == (unsigned HOST_WIDE_INT)
>   	    TREE_STRING_LENGTH (TREE_OPERAND (TREE_OPERAND (str, 0), 0)))
>       {
> Index: gcc/genpreds.c
> ===================================================================
> --- gcc/genpreds.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/genpreds.c	2013-11-20 13:48:38.513410388 +0000
> @@ -810,6 +810,7 @@ add_constraint (const char *name, const
>       {
>         enum rtx_code appropriate_code
>   	= is_const_int ? CONST_INT : CONST_DOUBLE;
> +
>         /* Consider relaxing this requirement in the future.  */
>         if (regclass
>   	  || GET_CODE (exp) != AND
> Index: gcc/gimplify.c
> ===================================================================
> --- gcc/gimplify.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/gimplify.c	2013-11-20 13:48:38.515410377 +0000
> @@ -2642,7 +2642,7 @@ gimple_boolify (tree expr)
>         return expr;
>   
>       case ANNOTATE_EXPR:
> -      if ((enum annot_expr_kind) tree_to_uhwi (TREE_OPERAND (expr, 1))
> +      if ((enum annot_expr_kind) TREE_INT_CST_LOW (TREE_OPERAND (expr, 1))
>   	  == annot_expr_ivdep_kind)
>   	{
>   	  TREE_OPERAND (expr, 0) = gimple_boolify (TREE_OPERAND (expr, 0));
> Index: gcc/ipa-devirt.c
> ===================================================================
> --- gcc/ipa-devirt.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/ipa-devirt.c	2013-11-20 13:48:38.515410377 +0000
> @@ -209,7 +209,7 @@ hash_type_name (tree t)
>   
>         if (TREE_CODE (v) == POINTER_PLUS_EXPR)
>   	{
> -	  hash = TREE_INT_CST_ELT (TREE_OPERAND (v, 1), 0);
> +	  hash = TREE_INT_CST_LOW (TREE_OPERAND (v, 1));
>   	  v = TREE_OPERAND (TREE_OPERAND (v, 0), 0);
>   	}
>   
> Index: gcc/java/typeck.c
> ===================================================================
> --- gcc/java/typeck.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/java/typeck.c	2013-11-20 13:48:38.525410321 +0000
> @@ -219,7 +219,7 @@ java_array_type_length (tree array_type)
>   	{
>   	  tree high = TYPE_MAX_VALUE (index_type);
>   	  if (TREE_CODE (high) == INTEGER_CST)
> -	    return tree_to_uhwi (high) + 1;
> +	    return TREE_INT_CST_LOW (high) + 1;
>   	}
>       }
>     return -1;
> Index: gcc/stor-layout.c
> ===================================================================
> --- gcc/stor-layout.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/stor-layout.c	2013-11-20 13:48:38.516410371 +0000
> @@ -1244,7 +1244,7 @@ place_field (record_layout_info rli, tre
>         && ! integer_zerop (DECL_SIZE (field))
>         && tree_fits_uhwi_p (DECL_SIZE (field))
>         /* BUG!!! rli->offset is checked as unsigned but used as signed.   */
> -      && tree_fits_shwi_p (rli->offset)
> +      && tree_fits_uhwi_p (rli->offset)
>         && tree_fits_uhwi_p (TYPE_SIZE (type)))
>       {
>         unsigned int type_align = TYPE_ALIGN (type);
> Index: gcc/targhooks.c
> ===================================================================
> --- gcc/targhooks.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/targhooks.c	2013-11-20 13:48:38.517410365 +0000
> @@ -276,6 +276,7 @@ default_cxx_guard_type (void)
>     return long_long_integer_type_node;
>   }
>   
> +
>   /* Returns the size of the cookie to use when allocating an array
>      whose elements have the indicated TYPE.  Assumes that it is already
>      known that a cookie is needed.  */
> Index: gcc/tree-pretty-print.c
> ===================================================================
> --- gcc/tree-pretty-print.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/tree-pretty-print.c	2013-11-20 13:48:38.517410365 +0000
> @@ -2118,7 +2118,7 @@ dump_generic_node (pretty_printer *buffe
>   
>       case ANNOTATE_EXPR:
>         pp_string (buffer, "ANNOTATE_EXPR <");
> -      switch ((enum annot_expr_kind) tree_to_shwi (TREE_OPERAND (node, 1)))
> +      switch ((enum annot_expr_kind) TREE_INT_CST_LOW (TREE_OPERAND (node, 1)))
>   	{
>   	case annot_expr_ivdep_kind:
>   	  pp_string (buffer, "ivdep, ");
> Index: gcc/tree.c
> ===================================================================
> --- gcc/tree.c	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/tree.c	2013-11-20 13:48:38.520410349 +0000
> @@ -6993,7 +6993,7 @@ tree_fits_uhwi_p (const_tree t)
>   tree_to_shwi (const_tree t)
>   {
>     gcc_assert (tree_fits_shwi_p (t));
> -  return TREE_INT_CST_ELT (t, 0);
> +  return TREE_INT_CST_LOW (t);
>   }
>   
>   /* T is an INTEGER_CST whose numerical value (extended according to
> @@ -7004,7 +7004,7 @@ tree_to_shwi (const_tree t)
>   tree_to_uhwi (const_tree t)
>   {
>     gcc_assert (tree_fits_uhwi_p (t));
> -  return TREE_INT_CST_ELT (t, 0);
> +  return TREE_INT_CST_LOW (t);
>   }
>   
>   /* Return the most significant (sign) bit of T.  */
> @@ -9575,8 +9575,8 @@ build_common_tree_nodes (bool signed_cha
>        may be larger than char depending on the value of BOOL_TYPE_SIZE.  */
>     boolean_type_node = make_unsigned_type (BOOL_TYPE_SIZE);
>     TREE_SET_CODE (boolean_type_node, BOOLEAN_TYPE);
> -  TYPE_PRECISION (boolean_type_node) = 1;
>     TYPE_MAX_VALUE (boolean_type_node) = build_int_cst (boolean_type_node, 1);
> +  TYPE_PRECISION (boolean_type_node) = 1;
>   
>     /* Define what type to use for size_t.  */
>     if (strcmp (SIZE_TYPE, "unsigned int") == 0)
> Index: gcc/tree.h
> ===================================================================
> --- gcc/tree.h	2013-11-20 13:43:30.896150594 +0000
> +++ gcc/tree.h	2013-11-20 13:48:38.521410343 +0000
> @@ -3697,14 +3697,14 @@ extern inline __attribute__ ((__gnu_inli
>   tree_to_shwi (const_tree t)
>   {
>     gcc_assert (tree_fits_shwi_p (t));
> -  return TREE_INT_CST_ELT (t, 0);
> +  return TREE_INT_CST_LOW (t);
>   }
>   
>   extern inline __attribute__ ((__gnu_inline__)) unsigned HOST_WIDE_INT
>   tree_to_uhwi (const_tree t)
>   {
>     gcc_assert (tree_fits_uhwi_p (t));
> -  return TREE_INT_CST_ELT (t, 0);
> +  return TREE_INT_CST_LOW (t);
>   }
>   #endif
>   extern int tree_int_cst_sgn (const_tree);
>

Kenneth Zadeck <zadeck@naturalbridge.com> writes:
> I would leave out the last frag of tree.c.  It actually gets rid of what 
> i consider, a latent bug. it does not show up with the current 
> implementation of wide-int, but if that implementation changed, it 
> would.   The problem is that you really should not expect that you can 
> set the min or max value for a type without first setting the precision 
> of that type.

Ah, OK.  I should have realised there was more to it.

> Aside from that, this looks fine.

Thanks, applied without that hunk.

Richard

On Nov 20, 2013, at 5:58 AM, Richard Sandiford <rsandifo@linux.vnet.ibm.com> wrote:
> I've committed a patch upstream to convert TREE_INT_CST_LOW to tree_to_[su]hwi
> if there is an obvious tree_fits_[su]hwi_p guard.

So, in general, I like putting them on trunk, and then merging them into the branch.  When that is done, the difference disappears without the removal of the difference.  If one misses one, then it does't magically disappear.  If one changes it in a manner that is different than the branch, the change then shows up as a conflict to resolve.  Also, when that is done, we can then just see the actual changes you are making, instead of those intermixed into random changes that made it to trunk and random ones that did not.

> The patch also undoes a couple of other ordering and whitespace differences.

So, for example, these changes are just dropped on the floor.  :-(  I picked them up, and put them into trunk.  I can't tell which other patches are also so dropped that I might think are important.

Mike Stump <mikestump@comcast.net> writes:
> On Nov 20, 2013, at 5:58 AM, Richard Sandiford
> <rsandifo@linux.vnet.ibm.com> wrote:
>> I've committed a patch upstream to convert TREE_INT_CST_LOW to tree_to_[su]hwi
>> if there is an obvious tree_fits_[su]hwi_p guard.
>
> So, in general, I like putting them on trunk, and then merging them into
> the branch.  When that is done, the difference disappears without the
> removal of the difference.  If one misses one, then it does't magically
> disappear.  If one changes it in a manner that is different than the
> branch, the change then shows up as a conflict to resolve.  Also, when
> that is done, we can then just see the actual changes you are making,
> instead of those intermixed into random changes that made it to trunk
> and random ones that did not.

I don't get what you're trying to say.  I _did_ put the changes I want
to keep on trunk, and merged them back to the branch so that the remaining
TREE_INT_CST_LOW->tree_to_* changes stood out.

The remaining branch changes weren't obvious without some justification
and are independent of what we're doing in wide-int.  Which is why I asked
permission to revert them on the branch and leave them to be handled 
separately.  As I said, the patch is still here, so it's not like the
work is lost.

>> The patch also undoes a couple of other ordering and whitespace differences.
>
> So, for example, these changes are just dropped on the floor.  :-( I
> picked them up, and put them into trunk.  I can't tell which other
> patches are also so dropped that I might think are important.

That's why I'm posting the patches for review, so you can see and comment.

Thanks,
Richard