public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* wide-int branch now up for public comment and review
@ 2013-08-13 20:57 Kenneth Zadeck
  2013-08-22  8:25 ` Richard Sandiford
                   ` (2 more replies)
  0 siblings, 3 replies; 50+ messages in thread
From: Kenneth Zadeck @ 2013-08-13 20:57 UTC (permalink / raw)
  To: rguenther, gcc-patches, Mike Stump, r.sandiford

Richi and everyone else who may be interested,

Congrats on your first child.  They are a lot of fun, but are very
high maintenence.

Today we put up the wide-int branch for all to see and play with. See

svn+ssh://gcc.gnu.org/svn/gcc/branches/wide-int

At this point, we have completed testing it on x86-64.  Not only is it
regression free, but for code that uses only 64 bit or smaller data
types, it produces identical machine language (if a couple of changes
are made to the truck - see the patch below).  We are currently
working on the PPC and expect to get this platform to the same
position very soon.

 From a high level view, the branch looks somewhat closer to what you
asked for than I would have expected.  There are now three
implementations of wide-int as a template.  The default is the one you
saw before and takes its precision from the input mode or type. There
are two other template instances which have fixed precisions that are
defined to be large enough to be assumed to be infinite (just like
your favorite use of double-int).  Both are used in places where there
is not the notion of precision correctness of the operands. One is
used for all addressing arithmetic and the other is used mostly in the
vectorizer and loop optimizations.  The bottom line is that both a
finite and infinite precision model are really necessary in the
current structure of GCC.

The two infinite precision classes are not exactly the storage classes
that you proposed because they are implemented using the same storage
model as the default template but they do provide a different view of
the math which I assume was your primary concern.  You may also decide
that there is not reason to have a separate class for the addressing
arithmetic since they work substantially the same way.  We did it so
that we have the option in the future to allow the two reps to
diverge.

The one place where I can see changing which template is used is in
tree-ssa-ccp.  This is the only one of the many GCC constant
propagator that does not use the default template.  I did not convert
this pass to use the default template because, for testing purposes
(at your suggestion), we did tried to minimize the improvements so
that we get the same code out with wide-int.  When I convert it to use
the default template, the pass will run slightly faster and will find
slightly more constants: both very desirable features, but not in the
context of getting this large patch into GCC.

As I said earlier, we get the same code as long as the program uses
only 64 bit or smaller types.  For code that uses larger types, we do
not.  The problem actually stems from one of the assertions that you
made when we were arguing about fixed vs infinite precision.  You had
said that a lot of the code depended on double ints behaving like
infinite precision.  You were right!!!  However, what this really
meant is that when that code was subjected to at 128 bit type, it just
produced bogus results!!!!  All of this has been fixed now on the
branch.  The code that uses the default template works within it's
precision.  The code that uses one of the infinite precision templates
can be guaranteed that there is always enough head room because we
sniff out the largest mode on the target and multiply that by 4.
However, the net result is that programs that use 128 bit types get
better code out that is more likely to be correct.

The vast majority of the patch falls into two types of code:

1) The 4 files that hold the wide-int code itself.  You have seen a
    lot of this code before except for the infinite precision
    templates.  Also the classes are more C++ than C in their flavor.
    In particular, the integration with trees is very tight in that an
    int-cst or regular integers can be the operands of any wide-int
    operation.

2) The code that encapsulates the representation of a TREE_INT_CST.
    For the latter, I introduced a series of abstractions to hide the
    access so that I could change the representation of TREE_INT_CST
    away from having exactly two HWIs.  I do not really like these
    abstractions, but the good news is that most of them can/will go
    away after this branch is integrated into the trunk.  These
    abstractions allow the code to do the same function, without
    exposing the change in the data structures.  However, they preserve
    the fact that for the most part, the middle end of the compiler
    tries to do no optimization on anything larger than a single HWI.
    But this preserves the basic behavior of the compiler which is what
    you asked us to do.

    The abstractions that I have put in to hide the rep of TREE_INT_CST 
are:

    host_integerp (x, 1) -> tree_fits_uhwi_p (x)
    host_integerp (x, 0) -> tree_fits_shwi_p (x)
    host_integerp (x, TYPE_UNSIGNED (y)) -> tree_fits_hwi_p (x, 
TYPE_SIGN (y))
    host_integerp (x, TYPE_UNSIGNED (x)) -> tree_fits_hwi_p (x)


    TREE_INT_CST_HIGH (x) == 0 || TREE_INT_CST_HIGH (value) == -1 -> 
cst_fits_shwi_p (x)
    TREE_INT_CST_HIGH (x) + (tree_int_cst_sgn (x) < 0) -> 
cst_fits_shwi_p (x)
    cst_and_fits_in_hwi (x) -> cst_fits_shwi_p (x)

    TREE_INT_CST_HIGH (x) == 0) -> cst_fits_uhwi_p (x)

    tree_low_cst (x, 1) ->  tree_to_uhwi (x)
    tree_low_cst (x, 0) ->  tree_to_shwi (x)
    TREE_INT_CST_LOW (x) -> to either tree_to_uhwi (x), tree_to_shwi (x) 
or tree_to_hwi (x)

    Code that used the TREE_INT_CST_HIGH in ways beyond checking to see
    if contained 0 or -1 was converted directly to wide-int.


You had proposed that one of the ways that we should/could test the
non single HWI paths in wide-int was to change the size of the element
of the array used to represent value in wide-int.   I believe that
there are better ways to do this testing.   For one, the infinite
precision templates do not use the fast pathway anyway because
currently those pathways are only triggered for precisions that fit in
a single HWI.   (There is the possibility that some of the infinite
precision functions could use this fast path, but they currently do
not.)   However, what we are planning to do when the ppc gets stable
is to build a 64 bit compiler for the x86 that uses a 32 bit HWI.
This is no longer a supported path, but fixing the bugs on it would
shake out the remaining places where the compiler (as well as the
wide-int code) gets the wrong answer for larger types.

The code still has our tracing in it.   We will remove it before the
branch is committed, but for large scale debugging, we find this
very useful.

I am not going to close with the typical "ok to commit?" closing
because I know you will have a lot to say.   But I do think that you
will find that this is a lot closer to what you envisioned than what
you saw before.

kenny

=====================================

The two patches for the truck below are necessary to get identical
code between the wide-int branch and the truck.   The first patch has
been submitted for review and fixes a bug.   The second patch will not
be submitted as it is just for compatibility.   The second patch
slightly changes the hash function that the rtl gcse passes use. Code
is modified based on the traversal of a hash function, so if the hash
functions are not identical, the code is slightly different between
the two branches.


=====================================
diff --git a/gcc/expr.c b/gcc/expr.c
index 923f59b..f5744b0 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -4815,7 +4815,8 @@ expand_assignment (tree to, tree from, bool 
nontemporal)
                    bitregion_start, bitregion_end,
                    mode1, from,
                    get_alias_set (to), nontemporal);
-      else if (bitpos >= mode_bitsize / 2)
+      else if (bitpos >= mode_bitsize / 2
+           && bitpos+bitsize <= mode_bitsize)
          result = store_field (XEXP (to_rtx, 1), bitsize,
                    bitpos - mode_bitsize / 2,
                    bitregion_start, bitregion_end,
@@ -4834,8 +4835,12 @@ expand_assignment (tree to, tree from, bool 
nontemporal)
          }
        else
          {
+          HOST_WIDE_INT extra = 0;
+          if (bitpos+bitsize > mode_bitsize)
+        extra = bitpos+bitsize - mode_bitsize;
            rtx temp = assign_stack_temp (GET_MODE (to_rtx),
-                        GET_MODE_SIZE (GET_MODE (to_rtx)));
+                        GET_MODE_SIZE (GET_MODE (to_rtx))
+                        + extra);
            write_complex_part (temp, XEXP (to_rtx, 0), false);
            write_complex_part (temp, XEXP (to_rtx, 1), true);
            result = store_field (temp, bitsize, bitpos,
diff --git a/gcc/rtl.def b/gcc/rtl.def
index b4ce1b9..5ed015c 100644
--- a/gcc/rtl.def
+++ b/gcc/rtl.def
@@ -342,6 +342,8 @@ DEF_RTL_EXPR(TRAP_IF, "trap_if", "ee", RTX_EXTRA)
  /* numeric integer constant */
  DEF_RTL_EXPR(CONST_INT, "const_int", "w", RTX_CONST_OBJ)

+DEF_RTL_EXPR(CONST_WIDE_INT, "const_wide_int", "", RTX_CONST_OBJ)
+
  /* fixed-point constant */
  DEF_RTL_EXPR(CONST_FIXED, "const_fixed", "www", RTX_CONST_OBJ)


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

end of thread, other threads:[~2013-09-06  0:10 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-13 20:57 wide-int branch now up for public comment and review Kenneth Zadeck
2013-08-22  8:25 ` Richard Sandiford
2013-08-23 15:03 ` Richard Sandiford
2013-08-23 21:01   ` Kenneth Zadeck
2013-08-24 10:44     ` Richard Sandiford
2013-08-24 13:10       ` Richard Sandiford
2013-08-24 18:16         ` Kenneth Zadeck
2013-08-25  7:27           ` Richard Sandiford
2013-08-25 13:21             ` Kenneth Zadeck
2013-08-24 21:22       ` Kenneth Zadeck
2013-08-24  0:03   ` Mike Stump
2013-08-24  1:59   ` Mike Stump
2013-08-24  3:34   ` Mike Stump
2013-08-24  9:04     ` Richard Sandiford
2013-08-24 20:46   ` Kenneth Zadeck
2013-08-25 10:52   ` Richard Sandiford
2013-08-25 15:14     ` Kenneth Zadeck
2013-08-26  2:22     ` Mike Stump
2013-08-26  5:40       ` Kenneth Zadeck
2013-08-28  9:11       ` Richard Biener
2013-08-29 13:34         ` Kenneth Zadeck
2013-08-25 18:12   ` Mike Stump
2013-08-25 18:57     ` Richard Sandiford
2013-08-25 19:59       ` Mike Stump
2013-08-25 20:11       ` Mike Stump
2013-08-25 21:38     ` Joseph S. Myers
2013-08-25 21:53       ` Mike Stump
2013-08-28  9:06   ` Richard Biener
2013-08-28  9:51     ` Richard Sandiford
2013-08-28 10:40       ` Richard Biener
2013-08-28 11:52         ` Richard Sandiford
2013-08-28 12:04           ` Richard Biener
2013-08-28 12:32             ` Richard Sandiford
2013-08-28 12:49               ` Richard Biener
2013-08-28 16:58                 ` Mike Stump
2013-08-28 21:15                   ` Kenneth Zadeck
2013-08-29  3:18                     ` Mike Stump
2013-08-28 16:08         ` Mike Stump
2013-08-29  7:42           ` Richard Biener
2013-08-29 19:34             ` Mike Stump
2013-08-30  8:51               ` Richard Biener
2013-09-01 19:26               ` Richard Sandiford
2013-09-05 21:00                 ` Richard Sandiford
2013-09-06  0:10                   ` Mike Stump
2013-08-28 13:11     ` Kenneth Zadeck
2013-08-29  0:15     ` Kenneth Zadeck
2013-08-29  9:13       ` Richard Biener
2013-08-29 12:38         ` Kenneth Zadeck
2013-08-24 18:42 ` Florian Weimer
2013-08-24 19:48   ` Kenneth Zadeck

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).