On Wed, Oct 3, 2012 at 7:15 PM, Kenneth Zadeck <zad...@naturalbridge.com> wrote:
> The enclosed patch is the third of at least four patches that fix the
> problems associated with supporting integers on the target that are
> wider than two HOST_WIDE_INTs.
>
> While GCC claims to support OI mode, and we have two public ports that
> make minor use of this mode, in practice, compilation that uses OImode
> mode commonly gives the wrong result or ices. We have a private port
> of GCC for an architecture that is further down the road to needing
> comprehensive OImode and we have discovered that this is unusable. We
> have decided to fix it in a general way that so that it is most
> beneficial to the GCC community. It is our belief that we are just a
> little ahead of the X86 and the NEON and these patches will shortly be
> essential.
>
> The first two of these patches were primarily lexigraphical and have
> already been committed. They transformed the uses of CONST_DOUBLE
> so that it is easy to tell what the intended usage is.
>
> The underlying structures in the next two patches are very general:
> once they are added to the compiler, the compiler will be able to
> support targets with any size of integer from hosts of any size
> integer.
>
> The patch enclosed deals with the portable RTL parts of the compiler.
> The next patch, which is currently under construction deals with the
> tree level. However, this patch can be put on the trunk as is, and it
> will eleviate many, but not all of the current limitations in the rtl
> parts of the compiler.
>
> Some of the patch is conditional, depending on a port defining the
> symbol 'TARGET_SUPPORTS_WIDE_INT' to be non zero. Defining this
> symbol to be non zero is declaring that the port has been converted to
> use the new form or integer constants. However, the patch is
> completely backwards compatible to allow ports that do not need this
> immediately to convert at their leasure. The conversion process is
> not difficult, but it does require some knowledge of the port, so we
> are not volinteering to do this for all ports.
>
> OVERVIEW OF THE PATCH:
>
> The patch defines a new datatype, a 'wide_int' (defined in
> wide-int.[ch], and this datatype will be used to perform all of the
> integer constant math in the compiler. Externally, wide-int is very
> similar to double-int except that it does not have the limitation that
> math must be done on exactly two HOST_WIDE_INTs.
>
> Internally, a wide_int is a structure that contains a fixed sized
> array of HOST_WIDE_INTs, a length field and a mode. The size of the
That it has a mode sounds odd to me and makes it subtly different
from HOST_WIDE_INT and double-int. Maybe the patch will tell
why this is so.
> array is determined at generation time by dividing the number of bits
> of the largest integer supported on the target by the number of bits
> in a HOST_WIDE_INT of the host. Thus, with this format, any length of
> integer can be supported on any host.
>
> A new rtx type is created, the CONST_WIDE_INT, which contains a
> garbage collected array of HOST_WIDE_INTS that is large enough to hold
> the constant. For the targets that define TARGET_SUPPORTS_WIDE_INT to
> be non zero, CONST_DOUBLES are only used to hold floating point
> values. If the target leaves TARGET_SUPPORTS_WIDE_INT defined as 0,
> CONST_WIDE_INTs are not used and CONST_DOUBLEs are as they were
> before.
>
> CONST_INT does not change except that it is defined to hold all
> constants that fit in exactly one HOST_WIDE_INT. Note that is slightly
> different than the current trunk. Before this patch, the TImode
> constant '5' could either be in a CONST_INT or CONST_DOUBLE depending
> on which code path was used to create it. This patch changes this so
> that if the constant fits in a CONST_INT then it is represented in a
> CONST_INT no matter how it is created.
>
> For the array inside a CONST_WIDE_INT, and internally in wide-int, we
> use a compressed form for integers that need more than one
> HOST_WIDE_INT. Higher elements of the array are not needed if they
> are just a sign extension of the elements below them. This does not
> imply that constants are signed or are sign extended, this is only a
> compression technique.
>
> While it might seem to be more esthetically pleasing to have not
> introduced the CONST_WIDE_INT and to have changed the representation
> of the CONST_INT to accomodate larger numbers, this would have both
> used more space and would be a time consuming change for the port
> maintainers. We believe that most ports can be quickly converted with
> the current scheme because there is just not a lot of code in the back
> ends that cares about large constants. Furthermore, the CONST_INT is
> very space efficient and even in a program that was heavy in large
> values, most constants would still fit in a CONST_INT.
>
> All of the parts of the rtl level that deal with CONST_DOUBLE as an
> now conditionally work with CONST_WIDE_INTs depending on the value
> of TARGET_SUPPORTS_WIDE_INT. We believe that this patch removes all
> of the ices and wrong code places at the portable rtl level. However,
> there are still places in the portable rtl code that refuse to
> transform the code unless it is a CONST_INT. Since these do not cause
> failures, they can be handled later. The patch is already very large.
>
> It should be noted that much of the constant overflow checking in the
> constant math dissappears with these patches. The overflow checking
> code in the current compiler is really divided into two cases:
> overflow on the host and overflow on the target. The overflow
> checking on the host was to make sure that the math did overflow when
> done on two HOST_WIDE_INTs. All of this code goes away. These
> patches allow the constant math to be done exactly the way it is done
> on the target.
>
> This patch also aids other cleanups that are being considered at the
> rtl level:
>
> 1) These patches remove most of the host dependencies on the
> optimizations. Currently a 32 bit GCC host will produce different
> code for a specific target than a 64 bit host will. This is because
> many of the transformations only work on constants that can be a
> represented with a single HWI or two HWIs. If the target has larger
> integers than the host, the compilation suffers.
>
> 2) Bernd's need to make GCC correctly support partial its is made
> easier by the wide-int library. This library carefully does all
> arithmetic in the precision of the mode included in it. While there
> are still places at the rtl level that still do arithmetic inline,
> we plan to convert those to use the library over time. This patch
> converts a substantial number of those places.
>
> 3) This patch is one step along the path to add modes to rtl integer
> constants. There is no longer any checking to see if a CONST_DOUBLE
> has VOIDmode as its mode. Furthermore, all constructors for various
> wide ints do take a mode and require that it not be VOIDmode. There
> is still a lot of work to do to make this conversion possible.
>
> Richard Sandiford has been over the rtl portions of this patch a few
> times. He has not looked at the wide-int files in any detail. This
> patch has been heavily tested on my private ports and also on x86-64.
>
>
> CONVERSION PROCESS
>
> Converting a port mostly requires looking for the places where
> CONST_DOUBLES are used with VOIDmode and replacing that code with code
> that accesses CONST_WIDE_INTs. "grep -i const_double" at the port
> level gets you to 95% of the changes that need to be made. There are
> a few places that require a deeper look.
>
> 1) There is no equivalent to hval and lval for CONST_WIDE_INTs.
> This would be difficult to express in the md language since there
> are a variable number of elements.
>
> Most ports only check that hval is either 0 or -1 to see if the int
> is small. As mentioned above, this will no longer be necessary
> since small constants are always CONST_INT. Of course there are
> still a few exceptions, the alpha's constraint used by the zap
> instruction certainly requires careful examination by C code.
> However, all the current code does is pass the hval and lval to C
> code, so evolving the c code to look at the CONST_WIDE_INT is not
> really a large change.
>
> 2) Because there is no standard template that ports use to
> materialize constants, there is likely to be some futzing that is
> unique to each port in this code.
>
> 3) The rtx costs may have to be adjusted to properly account for
> larger constants that are represented as CONST_WIDE_INT.
>
> All and all it has not taken us long to convert ports that we are
> familiar with.
>
> OTHER COMMENTS
>
> I did find what i believe is one interesting bug in the double-int
> code. I believe that the code that performs divide and mod with round
> to nearest is seriously wrong for unsigned integers. I believe that
> it will get the wrong answer for any numbers that are large enough to
> look negative if they consider signed integers. Asside from that,
> wide-int should perform in a very similar manner to double-int.
>
> I am sorry for the size of this patch. However, there does not appear
> to change the underlying data structure to support wider integers
> without doing something like this.
Some pieces can be easily split out, like the introduction and use
of CONST_SCALAR_INT_P.
As my general comment I would like to see double-int and wide-int
unified from an interface perspective. Which means that double-int
should be a specialization of wide-int which should be a template
(which also means its size is constant). Thus,
typedef wide_int<2> double_int;
should be the way to expose the double_int type.
The main question remains - why does wide_int have a mode?
That looks redundant, both with information stored in types
and the RTL constant, and with the len field (that would be
always GET_MODE_SIZE () / ...?). Also when you associate
a mode it's weird you don't associate a signedness.
Thus I'd ask you to rework this to be a template on 'len'
(number of HOST_WIDE_INT words), drop the mode member
and unify double-int and wide-int. Co-incidentially incrementally
doing this by converting double-int to a typedef of a
wide_int<2> specialization (thus moving double-int implementation
stuff to be wide_int<2> specialized) would be prefered.
Btw,
+/* 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
+wide_int_to_tree (tree type, const wide_int &cst)
+{
+ wide_int v;
+ if (TYPE_UNSIGNED (type))
+ v = cst.zext (TYPE_PRECISION (type));
+ else
+ v = cst.sext (TYPE_PRECISION (type));
+
+ return build_int_cst_wide (type, v.elt (0), v.elt (1));
+}
is surely broken. A wide-int does not fit a double-int. How are you
going to "fix" this?
Thanks,
Richard.
> kenny
