Patchwork wide-int, rtl

login
register
mail settings
Submitter Kenneth Zadeck
Date Dec. 6, 2013, 9:06 p.m.
Message ID <52A23C51.8040107@naturalbridge.com>
Download mbox | patch
Permalink /patch/298275/
State New
Headers show

Comments

Kenneth Zadeck - Dec. 6, 2013, 9:06 p.m.
On 11/27/2013 11:24 AM, Eric Botcazou wrote:
>> Richi has asked the we break the wide-int patch so that the individual port
>> and front end maintainers can review their parts without have to go through
>> the entire patch.    This patch covers the first half of the rtl code.
> --- a/gcc/cse.c
> +++ b/gcc/cse.c
> @@ -2336,15 +2336,23 @@ hash_rtx_cb (const_rtx x, enum machine_mode mode,
>                  + (unsigned int) INTVAL (x));
>         return hash;
>   
> +    case CONST_WIDE_INT:
> +      {
> +	int i;
> +	for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++)
> +	  hash += CONST_WIDE_INT_ELT (x, i);
> +      }
> +      return hash;
>
> You can write "for (int i = 0; ..." now and remove the parentheses.
>
done
> --- a/gcc/cselib.c
> +++ b/gcc/cselib.c
> @@ -1121,15 +1120,23 @@ cselib_hash_rtx (rtx x, int create, enum machine_mode
> memmode)
>         hash += ((unsigned) CONST_INT << 7) + INTVAL (x);
>         return hash ? hash : (unsigned int) CONST_INT;
>   
> +    case CONST_WIDE_INT:
> +      {
> +	int i;
> +	for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++)
> +	  hash += CONST_WIDE_INT_ELT (x, i);
> +      }
> +      return hash;
> +
>
> Likewise.
done
> --- a/gcc/expmed.c
> +++ b/gcc/expmed.c
> @@ -3298,23 +3268,13 @@ choose_multiplier (unsigned HOST_WIDE_INT d, int n,
> int precision,
>     pow = n + lgup;
>     pow2 = n + lgup - precision;
>   
> -  /* We could handle this with some effort, but this case is much
> -     better handled directly with a scc insn, so rely on caller using
> -     that.  */
> -  gcc_assert (pow != HOST_BITS_PER_DOUBLE_INT);
>
> Why removing it?
the code no longer has restrictions on the size/mode of the variables.
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -722,64 +722,33 @@ convert_modes (enum machine_mode mode, enum machine_mode
> oldmode, rtx x, int uns
>     if (mode == oldmode)
>       return x;
>   
> -  /* There is one case that we must handle specially: If we are converting
> -     a CONST_INT into a mode whose size is twice HOST_BITS_PER_WIDE_INT and
> -     we are to interpret the constant as unsigned, gen_lowpart will do
> -     the wrong if the constant appears negative.  What we want to do is
> -     make the high-order word of the constant zero, not all ones.  */
> -
> -  if (unsignedp && GET_MODE_CLASS (mode) == MODE_INT
> -      && GET_MODE_BITSIZE (mode) == HOST_BITS_PER_DOUBLE_INT
> -      && CONST_INT_P (x) && INTVAL (x) < 0)
> +  if (CONST_SCALAR_INT_P (x)
> +      && GET_MODE_CLASS (mode) == MODE_INT)
>
> On a single line.
done
>
>       {
> -      double_int val = double_int::from_uhwi (INTVAL (x));
> -
> -      /* We need to zero extend VAL.  */
> -      if (oldmode != VOIDmode)
> -	val = val.zext (GET_MODE_BITSIZE (oldmode));
> -
> -      return immed_double_int_const (val, mode);
> +      /* If the caller did not tell us the old mode, then there is
> +	 not much to do with respect to canonization.  We have to assume
> +	 that all the bits are significant.  */
> +      if (GET_MODE_CLASS (oldmode) != MODE_INT)
> +	oldmode = MAX_MODE_INT;
> +      wide_int w = wide_int::from (std::make_pair (x, oldmode),
> +				   GET_MODE_PRECISION (mode),
> +				   unsignedp ? UNSIGNED : SIGNED);
> +      return immed_wide_int_const (w, mode);
>
> canonicalization?
done
>
> @@ -5301,10 +5271,10 @@ store_expr (tree exp, rtx target, int call_param_p,
> bool nontemporal)
>   			       &alt_rtl);
>       }
>   
> -  /* If TEMP is a VOIDmode constant and the mode of the type of EXP is not
> -     the same as that of TARGET, adjust the constant.  This is needed, for
> -     example, in case it is a CONST_DOUBLE and we want only a word-sized
> -     value.  */
> +  /* If TEMP is a VOIDmode constant and the mode of the type of EXP is
> +     not the same as that of TARGET, adjust the constant.  This is
> +     needed, for example, in case it is a CONST_DOUBLE or
> +     CONST_WIDE_INT and we want only a word-sized value.  */
>     if (CONSTANT_P (temp) && GET_MODE (temp) == VOIDmode
>         && TREE_CODE (exp) != ERROR_MARK
>         && GET_MODE (target) != TYPE_MODE (TREE_TYPE (exp)))
>
> Why reformatting the whole comment?
it is what emacs did.  I have formatted it back.
> @@ -9481,11 +9459,19 @@ expand_expr_real_1 (tree exp, rtx target, enum
> machine_mode tmode,
>         return decl_rtl;
>   
>       case INTEGER_CST:
> -      temp = immed_double_const (TREE_INT_CST_LOW (exp),
> -				 TREE_INT_CST_HIGH (exp), mode);
> -
> -      return temp;
> -
> +      {
> +	tree type = TREE_TYPE (exp);
>
> Redundant, see the beginning of the function.
fixed
> +	/* One could argue that GET_MODE_PRECISION (TYPE_MODE (type))
> +	   should always be the same as TYPE_PRECISION (type).
> +	   However, it is not.  Since we are converting from tree to
> +	   rtl, we have to expose this ugly truth here.  */
> +	temp = immed_wide_int_const (wide_int::from
> +				       (exp,
> +					GET_MODE_PRECISION (TYPE_MODE (type)),
> +					TYPE_SIGN (type)),
> +				     TYPE_MODE (type));
> +	return temp;
> +      }
>
> I don't really see how one could argue that, given that there are much fewer
> modes than possible type precisions, so please rewrite the comment, e.g.:
>
> "Given that TYPE_PRECISION (type) is not always equal to
> GET_MODE_PRECISION (TYPE_MODE (type)), we need to extend from the former
> to the latter according to the signedness of the type".
>
> What about a fast track where the precisions are indeed equal?
>

There is not really a faster track here.    you still are starting with 
a tree and converting to an rtx.   All that the default one would do 
would be to access the types precision and sign and use that.



> @@ -11145,8 +11131,8 @@ const_vector_from_tree (tree exp)
>   	RTVEC_ELT (v, i) = CONST_FIXED_FROM_FIXED_VALUE (TREE_FIXED_CST (elt),
>   							 inner);
>         else
> -	RTVEC_ELT (v, i) = immed_double_int_const (tree_to_double_int (elt),
> -						   inner);
> +	RTVEC_ELT (v, i)
> +	  = immed_wide_int_const (elt, TYPE_MODE (TREE_TYPE (elt)));
>       }
>   
>     return gen_rtx_CONST_VECTOR (mode, v);
>
> Why replacing inner?
fixed.   this was just a mistake
>
> --- a/gcc/machmode.def
> +++ b/gcc/machmode.def
> @@ -229,6 +229,9 @@ UACCUM_MODE (USA, 4, 16, 16); /* 16.16 */
>   UACCUM_MODE (UDA, 8, 32, 32); /* 32.32 */
>   UACCUM_MODE (UTA, 16, 64, 64); /* 64.64 */
>   
> +/* Should be overridden by EXTRA_MODES_FILE if wrong.  */
> +#define MAX_BITS_PER_UNIT 8
> +
>
> What is it for?  It's not documented at all.
>
This requires some discussion as to the direction we want to go. This is 
put in so that in gen_modes we can compute MAX_BITSIZE_MODE_ANY_INT and 
MAX_BITSIZE_MODE_ANY_MODE.    The problem is that during genmodes we do 
have access to BITS_PER_UNIT.    These two computed symbols are then 
used as compile time constants in other parts of the compiler to 
allocate data structures that are guaranteed to be large enough.

Richard Sandiford put this in so we would preserve the ability to build 
a multi-targetted compiler where the targets had different values for 
BITS_PER_UNIT.   So one possibility is that we add some documentation to 
this effect.

A different solution is to notice that there is already code in the 
compiler that would not support BITS_PER_UNIT being anything but a 
compile time constant, and so then change the genmodes code so that it 
produced different symbols that were in terms of units rather than bits 
and then convert the places in the compiler that use these new symbols 
by multiplying by BITS_PER_UNIT there.   However, that is really only 
going to work if BITS_PER_UNIT is a compile time constant.

do you like one of these or do you have another suggestion?



> --- a/gcc/print-rtl.c
> +++ b/gcc/print-rtl.c
> @@ -617,6 +617,12 @@ print_rtx (const_rtx in_rtx)
>   	  fprintf (outfile, " [%s]", s);
>   	}
>         break;
> +
> +    case CONST_WIDE_INT:
> +      if (! flag_simple)
> +	fprintf (outfile, " ");
> +      cwi_output_hex (outfile, in_rtx);
> +      break;
>   #endif
>   
> Remove the if (! flag_simple) test.
>
done
> --- a/gcc/recog.c
> +++ b/gcc/recog.c
>
>   /* Returns 1 if OP is an operand that is a constant integer or constant
> -   floating-point number.  */
> +   floating-point number of MODE.  */
> +
> +int
> +const_double_operand (rtx op, enum machine_mode mode)
> +{
> +  return (GET_CODE (op) == CONST_DOUBLE)
> +	  && (GET_MODE (op) == mode || mode == VOIDmode);
> +}
>
> Can CONST_DOUBLEs still represent constant integers?
yes.   if unless the target has been converted to expect that integers 
that do not fit into a single hwi are put in CONST_WIDE_INT, they remain 
in double_ints.
> diff --git a/gcc/rtl.def b/gcc/rtl.def
> index 15a997b..a76b28b 100644
> --- a/gcc/rtl.def
> +++ b/gcc/rtl.def
> @@ -345,6 +345,9 @@ DEF_RTL_EXPR(TRAP_IF, "trap_if", "ee", RTX_EXTRA)
>   /* numeric integer constant */
>   DEF_RTL_EXPR(CONST_INT, "const_int", "w", RTX_CONST_OBJ)
>   
> +/* numeric integer constant */
> +DEF_RTL_EXPR(CONST_WIDE_INT, "const_wide_int", "", RTX_CONST_OBJ)
> +
>
> I think that we should consider using a new letter here, for example W.
> "" is reserved to very specific cases.
>
The data structure to hold a CONST_WIDE_INT is actually different than 
the data structure that holds the rest of the rtx types.  a 
CONST_WIDE_INTs has a single variable length array in it.    The length 
of that array is determined by the size of the constant that it needs to 
hold.    At the suggestion of richi, we made the optimization that their 
should only be a single one of these arrays allowed.    This saved 
several pointer indirections that would have been required had we made 
it general enough to hop from variable sized element to variable sized 
element as you do with the normal rtl scanning.


> --- a/gcc/rtl.h
> +++ b/gcc/rtl.h
> @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3.  If not see
>   #ifndef GCC_RTL_H
>   #define GCC_RTL_H
>   
> +#include <utility>
>
> Ouch.  Can't this really be avoided?
>
we use the c++ std::make_pair to convert rtls to wide-ints.    if we 
ever add modes to rtl int constants, this would all go away.   But there 
was no reason to reinvent this given that it is part of every c++ 
implementation.
> @@ -336,6 +348,14 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"),
>        1 in a VALUE or DEBUG_EXPR is NO_LOC_P in var-tracking.c.  */
>     unsigned return_val : 1;
>   
> +  union {
> +    /* RTXs are free to use up to 32 bit from here.  */
> +
> +    /* In a CONST_WIDE_INT (aka hwivec_def), this is the number of
> +       HOST_WIDE_INTs in the hwivec_def.  */
> +    unsigned GTY ((tag ("CONST_WIDE_INT"))) num_elem:32;
> +  } GTY ((desc ("GET_CODE (&%0)"))) u2;
> +
>     /* The first element of the operands of this rtx.
>        The number of operands and their types are controlled
>        by the `code' field, according to rtl.def.  */
>
> Why can't this be put into hwivec_def?
the comment now describes the reason behind this properly.
>
> --- a/gcc/simplify-rtx.c
> +++ b/gcc/simplify-rtx.c
>
> +      if (op_mode == VOIDmode)
> +	{
> +	  /* CONST_INT have VOIDmode as the mode.  We assume that all
> +	     the bits of the constant are significant, though, this is
> +	     a dangerous assumption as many times CONST_INTs are
> +	     created and used with garbage in the bits outside of the
> +	     precision of the implied mode of the const_int.  */
> +	  op_mode = MAX_MODE_INT;
> +	}
>
> Well, if that's really dangerous, we should find another way or at least
> stop the compiler before generating wrong code.
I think that this may be more about me being honest than anything 
else.   In preparation for this patch we converted a lot of suspect 
GEN_INTs to gen_int_mode, but there are a lot left.  (these were pushed 
directly to the trunk)  The truth is that until we bite the bullet and 
put modes in rtl integer constants this is always going to be a source 
of bugs.   The good news is that as targets move to supporting wide 
ints, they will move away from one of the problematic parts of this 
where the canonical way to test if a const_double has an int inside is 
to test if the mode is VOIDmode.
>
> +#if TARGET_SUPPORTS_WIDE_INT == 0
> +      /* This assert keeps the simplification from producing a result
> +	 that cannot be represented in a CONST_DOUBLE but a lot of
> +	 upstream callers expect that this function never fails to
> +	 simplify something and so you if you added this to the test
> +	 above the code would die later anyway.  If this assert
> +	 happens, you just need to make the port support wide int.  */
> +      gcc_assert (width <= HOST_BITS_PER_DOUBLE_INT);
> +#endif
>
> Can't we be more forgiving here and in the other similar places?
I answered this earlier.   but to reiterate, if you need big integers, 
convert your port to TARGET_SUPPORTS_WIDE_INT, you cannot put a 180 bit 
constant into 128 bits of data.
>
> +  /* We support any size mode.  */
> +  max_bitsize = MAX (GET_MODE_BITSIZE (outermode),
> +		     GET_MODE_BITSIZE (innermode));
>
> max_bitsize
>    = MAX (GET_MODE_BITSIZE (outermode), GET_MODE_BITSIZE (innermode));
>
>
Eric,

Since there are a couple of open issues, i am not going ask for ok.   
But the patch enclosed does do everything that is settled.

Kenny
Richard Sandiford - Dec. 7, 2013, 10:28 a.m.
Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>> +	/* One could argue that GET_MODE_PRECISION (TYPE_MODE (type))
>> +	   should always be the same as TYPE_PRECISION (type).
>> +	   However, it is not.  Since we are converting from tree to
>> +	   rtl, we have to expose this ugly truth here.  */
>> +	temp = immed_wide_int_const (wide_int::from
>> +				       (exp,
>> +					GET_MODE_PRECISION (TYPE_MODE (type)),
>> +					TYPE_SIGN (type)),
>> +				     TYPE_MODE (type));
>> +	return temp;
>> +      }
>>
>> I don't really see how one could argue that, given that there are much fewer
>> modes than possible type precisions, so please rewrite the comment, e.g.:
>>
>> "Given that TYPE_PRECISION (type) is not always equal to
>> GET_MODE_PRECISION (TYPE_MODE (type)), we need to extend from the former
>> to the latter according to the signedness of the type".
>>
>> What about a fast track where the precisions are indeed equal?
>>
>
> There is not really a faster track here.    you still are starting with 
> a tree and converting to an rtx.   All that the default one would do 
> would be to access the types precision and sign and use that.

FWIW it would be:

	temp = immed_wide_int_const (exp, TYPE_MODE (type));

But it's hard to tell whether it would buy much.  It didn't show up as
a hot spot when I was doing performance measurements before.

>> --- a/gcc/machmode.def
>> +++ b/gcc/machmode.def
>> @@ -229,6 +229,9 @@ UACCUM_MODE (USA, 4, 16, 16); /* 16.16 */
>>   UACCUM_MODE (UDA, 8, 32, 32); /* 32.32 */
>>   UACCUM_MODE (UTA, 16, 64, 64); /* 64.64 */
>>   
>> +/* Should be overridden by EXTRA_MODES_FILE if wrong.  */
>> +#define MAX_BITS_PER_UNIT 8
>> +
>>
>> What is it for?  It's not documented at all.
>>
> This requires some discussion as to the direction we want to go. This is 
> put in so that in gen_modes we can compute MAX_BITSIZE_MODE_ANY_INT and 
> MAX_BITSIZE_MODE_ANY_MODE.    The problem is that during genmodes we do 
> have access to BITS_PER_UNIT.    These two computed symbols are then 
> used as compile time constants in other parts of the compiler to 
> allocate data structures that are guaranteed to be large enough.
>
> Richard Sandiford put this in so we would preserve the ability to build 
> a multi-targetted compiler where the targets had different values for 
> BITS_PER_UNIT.   So one possibility is that we add some documentation to 
> this effect.

Sorry, I forgot yesterday an important detail behind why this seemed
like a good thing.  I think there was a strong feeling (from me and others)
that wide-int.h shouldn't depend on tm.h.  If we make wide-int.h depend
on tm.h then basically all the compiler does.

So as it stands we can't use BITS_PER_UNIT directly.  Having a
MAX_BITS_PER_UNIT for "all compiled-in targets" (which obviously
as things stand is exactly one) seemed like a reasonable abstraction.

Alternatively we could say that BITS_PER_UNIT is really part of the
definition of QImode and move it to the modes.def file.

Thanks,
Richard
Richard Guenther - Dec. 9, 2013, 9:40 a.m.
On Sat, Dec 7, 2013 at 11:28 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>> +    /* One could argue that GET_MODE_PRECISION (TYPE_MODE (type))
>>> +       should always be the same as TYPE_PRECISION (type).
>>> +       However, it is not.  Since we are converting from tree to
>>> +       rtl, we have to expose this ugly truth here.  */
>>> +    temp = immed_wide_int_const (wide_int::from
>>> +                                   (exp,
>>> +                                    GET_MODE_PRECISION (TYPE_MODE (type)),
>>> +                                    TYPE_SIGN (type)),
>>> +                                 TYPE_MODE (type));
>>> +    return temp;
>>> +      }
>>>
>>> I don't really see how one could argue that, given that there are much fewer
>>> modes than possible type precisions, so please rewrite the comment, e.g.:
>>>
>>> "Given that TYPE_PRECISION (type) is not always equal to
>>> GET_MODE_PRECISION (TYPE_MODE (type)), we need to extend from the former
>>> to the latter according to the signedness of the type".
>>>
>>> What about a fast track where the precisions are indeed equal?
>>>
>>
>> There is not really a faster track here.    you still are starting with
>> a tree and converting to an rtx.   All that the default one would do
>> would be to access the types precision and sign and use that.
>
> FWIW it would be:
>
>         temp = immed_wide_int_const (exp, TYPE_MODE (type));
>
> But it's hard to tell whether it would buy much.  It didn't show up as
> a hot spot when I was doing performance measurements before.
>
>>> --- a/gcc/machmode.def
>>> +++ b/gcc/machmode.def
>>> @@ -229,6 +229,9 @@ UACCUM_MODE (USA, 4, 16, 16); /* 16.16 */
>>>   UACCUM_MODE (UDA, 8, 32, 32); /* 32.32 */
>>>   UACCUM_MODE (UTA, 16, 64, 64); /* 64.64 */
>>>
>>> +/* Should be overridden by EXTRA_MODES_FILE if wrong.  */
>>> +#define MAX_BITS_PER_UNIT 8
>>> +
>>>
>>> What is it for?  It's not documented at all.
>>>
>> This requires some discussion as to the direction we want to go. This is
>> put in so that in gen_modes we can compute MAX_BITSIZE_MODE_ANY_INT and
>> MAX_BITSIZE_MODE_ANY_MODE.    The problem is that during genmodes we do
>> have access to BITS_PER_UNIT.    These two computed symbols are then
>> used as compile time constants in other parts of the compiler to
>> allocate data structures that are guaranteed to be large enough.
>>
>> Richard Sandiford put this in so we would preserve the ability to build
>> a multi-targetted compiler where the targets had different values for
>> BITS_PER_UNIT.   So one possibility is that we add some documentation to
>> this effect.
>
> Sorry, I forgot yesterday an important detail behind why this seemed
> like a good thing.  I think there was a strong feeling (from me and others)
> that wide-int.h shouldn't depend on tm.h.  If we make wide-int.h depend
> on tm.h then basically all the compiler does.
>
> So as it stands we can't use BITS_PER_UNIT directly.  Having a
> MAX_BITS_PER_UNIT for "all compiled-in targets" (which obviously
> as things stand is exactly one) seemed like a reasonable abstraction.
>
> Alternatively we could say that BITS_PER_UNIT is really part of the
> definition of QImode and move it to the modes.def file.

That makes sense to me - thus it will end up in insn-modes.h?  Note
that this file already uses BITS_PER_UNIT ...

Richard.

> Thanks,
> Richard
Richard Sandiford - Dec. 9, 2013, 10:55 a.m.
Richard Biener <richard.guenther@gmail.com> writes:
> On Sat, Dec 7, 2013 at 11:28 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>>> +    /* One could argue that GET_MODE_PRECISION (TYPE_MODE (type))
>>>> +       should always be the same as TYPE_PRECISION (type).
>>>> +       However, it is not.  Since we are converting from tree to
>>>> +       rtl, we have to expose this ugly truth here.  */
>>>> +    temp = immed_wide_int_const (wide_int::from
>>>> +                                   (exp,
>>>> +                                    GET_MODE_PRECISION (TYPE_MODE (type)),
>>>> +                                    TYPE_SIGN (type)),
>>>> +                                 TYPE_MODE (type));
>>>> +    return temp;
>>>> +      }
>>>>
>>>> I don't really see how one could argue that, given that there are much fewer
>>>> modes than possible type precisions, so please rewrite the comment, e.g.:
>>>>
>>>> "Given that TYPE_PRECISION (type) is not always equal to
>>>> GET_MODE_PRECISION (TYPE_MODE (type)), we need to extend from the former
>>>> to the latter according to the signedness of the type".
>>>>
>>>> What about a fast track where the precisions are indeed equal?
>>>>
>>>
>>> There is not really a faster track here.    you still are starting with
>>> a tree and converting to an rtx.   All that the default one would do
>>> would be to access the types precision and sign and use that.
>>
>> FWIW it would be:
>>
>>         temp = immed_wide_int_const (exp, TYPE_MODE (type));
>>
>> But it's hard to tell whether it would buy much.  It didn't show up as
>> a hot spot when I was doing performance measurements before.
>>
>>>> --- a/gcc/machmode.def
>>>> +++ b/gcc/machmode.def
>>>> @@ -229,6 +229,9 @@ UACCUM_MODE (USA, 4, 16, 16); /* 16.16 */
>>>>   UACCUM_MODE (UDA, 8, 32, 32); /* 32.32 */
>>>>   UACCUM_MODE (UTA, 16, 64, 64); /* 64.64 */
>>>>
>>>> +/* Should be overridden by EXTRA_MODES_FILE if wrong.  */
>>>> +#define MAX_BITS_PER_UNIT 8
>>>> +
>>>>
>>>> What is it for?  It's not documented at all.
>>>>
>>> This requires some discussion as to the direction we want to go. This is
>>> put in so that in gen_modes we can compute MAX_BITSIZE_MODE_ANY_INT and
>>> MAX_BITSIZE_MODE_ANY_MODE.    The problem is that during genmodes we do
>>> have access to BITS_PER_UNIT.    These two computed symbols are then
>>> used as compile time constants in other parts of the compiler to
>>> allocate data structures that are guaranteed to be large enough.
>>>
>>> Richard Sandiford put this in so we would preserve the ability to build
>>> a multi-targetted compiler where the targets had different values for
>>> BITS_PER_UNIT.   So one possibility is that we add some documentation to
>>> this effect.
>>
>> Sorry, I forgot yesterday an important detail behind why this seemed
>> like a good thing.  I think there was a strong feeling (from me and others)
>> that wide-int.h shouldn't depend on tm.h.  If we make wide-int.h depend
>> on tm.h then basically all the compiler does.
>>
>> So as it stands we can't use BITS_PER_UNIT directly.  Having a
>> MAX_BITS_PER_UNIT for "all compiled-in targets" (which obviously
>> as things stand is exactly one) seemed like a reasonable abstraction.
>>
>> Alternatively we could say that BITS_PER_UNIT is really part of the
>> definition of QImode and move it to the modes.def file.
>
> That makes sense to me - thus it will end up in insn-modes.h?

Yeah.

> Note that this file already uses BITS_PER_UNIT ...

Right, but only in the definitions that Kenny added in preparation for
wide-int:

2013-03-28  Kenneth Zadeck  <zadeck@naturalbridge.com>

	* genmodes.c (emit_max_int): New function.
	(emit_insn_modes_h): Added call to emit_max_function.
	* doc/rtl.texi (MAX_BITSIZE_MODE_ANY_INT, MAX_BITSIZE_MODE_ANY_MODE):
	Added doc.
	* machmode.def: Fixed comment.

And it's those that we were changing to use MAX_BITS_PER_UNITS instead,
so that there were no direct uses of BITS_PER_UNIT in insn-modes.h.

In the original version wide-int.h had to include tm.h in order for
these BITS_PER_UNIT-based macros to be usable.

Thanks,
Richard

Patch

Index: gcc/cselib.c
===================================================================
--- gcc/cselib.c	(revision 205749)
+++ gcc/cselib.c	(working copy)
@@ -1121,11 +1121,8 @@  cselib_hash_rtx (rtx x, int create, enum
       return hash ? hash : (unsigned int) CONST_INT;
 
     case CONST_WIDE_INT:
-      {
-	int i;
-	for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++)
-	  hash += CONST_WIDE_INT_ELT (x, i);
-      }
+      for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++)
+	hash += CONST_WIDE_INT_ELT (x, i);
       return hash;
 
     case CONST_DOUBLE:
Index: gcc/cse.c
===================================================================
--- gcc/cse.c	(revision 205749)
+++ gcc/cse.c	(working copy)
@@ -2337,11 +2337,8 @@  hash_rtx_cb (const_rtx x, enum machine_m
       return hash;
 
     case CONST_WIDE_INT:
-      {
-	int i;
-	for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++)
-	  hash += CONST_WIDE_INT_ELT (x, i);
-      }
+      for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++)
+	hash += CONST_WIDE_INT_ELT (x, i);
       return hash;
 
     case CONST_DOUBLE:
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	(revision 205749)
+++ gcc/rtl.h	(working copy)
@@ -348,7 +348,10 @@  struct GTY((chain_next ("RTX_NEXT (&%h)"
   unsigned return_val : 1;
 
   union {
-    /* RTXs are free to use up to 32 bit from here.  */
+    /* The final union field is aligned to 64 bits on LP64 hosts,
+       giving a 32-bit gap after the fields above. We optimize the
+       layout for that case and use the gap for extra code-specific
+       information.  */
 
     /* In a CONST_WIDE_INT (aka hwivec_def), this is the number of
        HOST_WIDE_INTs in the hwivec_def.  */
Index: gcc/print-rtl.c
===================================================================
--- gcc/print-rtl.c	(revision 205749)
+++ gcc/print-rtl.c	(working copy)
@@ -619,8 +619,7 @@  print_rtx (const_rtx in_rtx)
       break;
 
     case CONST_WIDE_INT:
-      if (! flag_simple)
-	fprintf (outfile, " ");
+      fprintf (outfile, " ");
       cwi_output_hex (outfile, in_rtx);
       break;
 #endif
Index: gcc/expr.c
===================================================================
--- gcc/expr.c	(revision 205749)
+++ gcc/expr.c	(working copy)
@@ -727,12 +727,11 @@  convert_modes (enum machine_mode mode, e
   if (mode == oldmode)
     return x;
 
-  if (CONST_SCALAR_INT_P (x)
-      && GET_MODE_CLASS (mode) == MODE_INT)
+  if (CONST_SCALAR_INT_P (x) && GET_MODE_CLASS (mode) == MODE_INT)
     {
-      /* If the caller did not tell us the old mode, then there is
-	 not much to do with respect to canonization.  We have to assume
-	 that all the bits are significant.  */
+      /* If the caller did not tell us the old mode, then there is not
+	 much to do with respect to canonicalization.  We have to
+	 assume that all the bits are significant.  */
       if (GET_MODE_CLASS (oldmode) != MODE_INT)
 	oldmode = MAX_MODE_INT;
       wide_int w = wide_int::from (std::make_pair (x, oldmode),
@@ -5295,10 +5294,10 @@  store_expr (tree exp, rtx target, int ca
 			       &alt_rtl);
     }
 
-  /* If TEMP is a VOIDmode constant and the mode of the type of EXP is
-     not the same as that of TARGET, adjust the constant.  This is
-     needed, for example, in case it is a CONST_DOUBLE or
-     CONST_WIDE_INT and we want only a word-sized value.  */
+  /* If TEMP is a VOIDmode constant and the mode of the type of EXP is not
+     the same as that of TARGET, adjust the constant.  This is needed, for
+     example, in case it is a CONST_DOUBLE or CONST_WIDE_INT and we want 
+     only a word-sized value.  */
   if (CONSTANT_P (temp) && GET_MODE (temp) == VOIDmode
       && TREE_CODE (exp) != ERROR_MARK
       && GET_MODE (target) != TYPE_MODE (TREE_TYPE (exp)))
@@ -9477,19 +9476,18 @@  expand_expr_real_1 (tree exp, rtx target
       return decl_rtl;
 
     case INTEGER_CST:
-      {
-	tree type = TREE_TYPE (exp);
-	/* One could argue that GET_MODE_PRECISION (TYPE_MODE (type))
-	   should always be the same as TYPE_PRECISION (type).
-	   However, it is not.  Since we are converting from tree to
-	   rtl, we have to expose this ugly truth here.  */
-	temp = immed_wide_int_const (wide_int::from
-				       (exp,
-					GET_MODE_PRECISION (TYPE_MODE (type)),
-					TYPE_SIGN (type)),
-				     TYPE_MODE (type));
-	return temp;
-      }
+      /* "Given that TYPE_PRECISION (type) is not always equal to
+         GET_MODE_PRECISION (TYPE_MODE (type)), we need to extend from
+         the former to the latter according to the signedness of the
+         type". */
+
+      temp = immed_wide_int_const (wide_int::from
+				   (exp,
+				    GET_MODE_PRECISION (TYPE_MODE (type)),
+				    TYPE_SIGN (type)),
+				   TYPE_MODE (type));
+      return temp;
+
     case VECTOR_CST:
       {
 	tree tmp = NULL_TREE;
@@ -11149,8 +11147,7 @@  const_vector_from_tree (tree exp)
 	RTVEC_ELT (v, i) = CONST_FIXED_FROM_FIXED_VALUE (TREE_FIXED_CST (elt),
 							 inner);
       else
-	RTVEC_ELT (v, i)
-	  = immed_wide_int_const (elt, TYPE_MODE (TREE_TYPE (elt)));
+	RTVEC_ELT (v, i) = immed_wide_int_const (elt, inner);
     }
 
   return gen_rtx_CONST_VECTOR (mode, v);