public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RFC: gimple.[ch] break apart
@ 2013-10-31  5:45 Andrew MacLeod
  2013-10-31  6:15 ` Jeff Law
                   ` (2 more replies)
  0 siblings, 3 replies; 117+ messages in thread
From: Andrew MacLeod @ 2013-10-31  5:45 UTC (permalink / raw)
  To: gcc-patches

Hopefully the other attempts to send this aren't queued up...  in any 
case, maybe I can't even attach a .dot file?... So no attachments this 
time...

instead, the diagram is here:
http://gcc.gnu.org/wiki/rearch?action=AttachFile&do=view&target=gimple.png

-------------------------------------------------------------

I've made 4 attempts now to split gimple.[ch] into reasonable component
parts, and I've finally found something that I can make work and fits my
plans.

I've attached a diagram to (hopefully :-) clarify things.

The original purpose of gimple.[ch] was to provide gimple statements.
This replaces the tcc_statement tree kind during the gimplification
process. No other tree kinds have been converted to gimple
structs/classes.  That what the next stage of my project will do.

As a result, any gimple queries regarding types, decls, or expressions
are actually tree queries. They are sprinkled throughout gimple.[ch] and
gimplify.[ch], not to mention tree.[ch] as well as other parts of the
compiler where they happened to be needed.  This has caused various
ordering issues among the inline functions when I tried to split out the
stmt, iterator, and gimplification bits from gimple.[ch].  Not to
mention a lack of an obvious home for some of these functions.

I'd like to move these as I encounter them into a new file,
gimple-decl.[ch].  When I'm working on the other gimple classes, this
will be further split into gimple-decl, gimple-type and gimple-expr as
appropriate but it seems reasonable to create just the one file now to
clump them since there is no other formal organization. So any function
which is actually querying/setting/building a decl, type, or expression
for gimple would go here.

I also want to split out the structure and accessing bits of the gimple
statement structure into gimple-stmt.[ch].  This would be just the
struct decls as well as all the accessing/setting/building functions...

The gimple_stmt_iterators (gsi) themselves also break out into their own
file quite naturally.

I find that gimple_seq does not seem to be a very clearly defined
thing.  Although a gimple_seq is just a typedef for a gimple stmt (I
thought it use to be a container?), it provides some additional
statement queuing functionality.   Ie, you don't have to worry about
next and prev pointers in the stmt's you build, you simply queue them up
and attach them where you want.  In that sense, its a kind of overlay on
top of a gimple-stmt as it provides additional functionality.
gimple_seq also utilizes gimple_smt_iterators under the covers. They do
not expose the iterators to a function using a gimple_seq but they do
need that knowledge to mange the lists under the covers (thus a dashed
line in the diagram).

Its unclear to me whether gimple_seq's are intended to have a future, or
whether their functionality should be rolled right into statements
themselves.  I believe it may be possible to include gimple-iterator.h
in gimple-stmt.c to provide the implementation without affecting the
inheritance layout, although I haven't actually tried that path.

Or we can treat them as a different layer with their own gimple-seq.[ch]
files.

Or we could combine gimple_seq and gsi routines in the same file, but
that would have the downside of exposing the gsi routines to the
gimplifier, which should have no need of.   Either of the latter 2
options seem reasonable to me for now.

The remnants of gimple.[ch] would contain various general helper
routines (walkers, etc), much like tree-ssa.[ch] does for ssa.

And finally gimplify.[ch] would contain all the stuff required for the
front ends to generate gimple code.  This is actually a front end
interface.  At the moment it isn't obvious since all the current gimple
code also uses trees and calls the gimplifier frequently. As I push
gimple types and decls into the back end and remove trees, the backend
should simply generate gimple directly.  Gimplify should slowly become
usable only via tree based front ends...  (Thus the dotted line from BE
to gimplify.. it should be removed eventually)

Which means that all the front end files should be including *only*
gimplify.h, and getting everything they need from there. Currently a
number of them include gimple.h which should not be required.

How reasonable or unreasonable does this sound? :-)  I've been tearing
the file apart in different ways and orders, and this seems to be the
most workable solution I have come up with that doesn't involve hacks.

Andrew


^ permalink raw reply	[flat|nested] 117+ messages in thread
* [PATCH 0/6] Convert gimple to a C++ class hierarchy
@ 2013-08-29 16:20 David Malcolm
  2013-08-29 16:20 ` [PATCH 4/6] Implement is_a_helper <>::test specializations for various gimple types David Malcolm
  0 siblings, 1 reply; 117+ messages in thread
From: David Malcolm @ 2013-08-29 16:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

The various gimple types are currently implemented using a hand-coded
C inheritance scheme, with a "union gimple_statement_d" holding the
various possible structs for a statement.

The following series of patches convert it to a C++ class
hierarchy, using the existing structs, eliminating the union. The
"gimple" typedef changes from being a
  (union gimple_statement_d *)
to being a:
  (struct gimple_statement_base *)

There are no virtual functions in the new code: the sizes of the various
structs are unchanged.

It makes use of "is-a.h", using the as_a <T> template function to
perform downcasts, which are checked (via gcc_checking_assert) in an
ENABLE_CHECKING build, and are simple casts in an unchecked build,
albeit it in an inlined function rather than a macro.

For example, one can write:

  gimple_statement_phi *phi =
    as_a <gimple_statement_phi> (gsi_stmt (gsi));

and then directly access the fields of the phi, as a phi.  The existing
accessor functions in gimple.h become somewhat redundant in this
scheme, but are preserved.

I believe this is a superior scheme to the C implementation:

  * We can get closer to compile-time type-safety, checking the gimple
    code once and downcasting with an as_a, then directly accessing
    fields, rather than going through accessor functions that check
    each time.  In some places we may want to replace a "gimple" with
    a subclass e.g. phis are always of the phi subclass, to get full
    compile-time type-safety.

  * This scheme is likely to be easier for newbies to understand.
  
  * Currently in gdb, dereferencing a gimple leads to screenfuls of text,
    showing all the various union values.  With this, you get just the base
    class, and can cast it to the appropriate subclass.

  * We're working directly with the language constructs, rather than
    rolling our own, and thus other tools can better understand the
    code.  For example, you can see Doxygen's rendering of the gimple
    class hierarchy (in the modified code) here:
    http://fedorapeople.org/~dmalcolm/gcc/2013-08-28/doxygen/html/structgimple__statement__base.html

The names of the structs are rather verbose.  I would prefer to also
rename them all to eliminate the "_statement" component:
  "gimple_statement_base" -> "gimple_base"
  "gimple_statement_phi"  -> "gimple_phi"
  "gimple_statement_omp"  -> "gimple_omp"
etc, but I didn't do this to mimimize the patch size.  But if the core
maintainers are up for that, I can redo the patch series with that
change also.

The patch is in 6 parts; all of them are needed together.

  * Patch 1 of 6: This patch adds inheritance to the various gimple
    types, eliminating the initial baseclass fields, and eliminating the
    union gimple_statement_d.   All the types remain structs.  They
    become marked with GTY((user)).

  * Patch 2 of 6: This patch ports various accessor functions within
    gimple.h to the new scheme.

  * Patch 3 of 6: This patch is autogenerated by "refactor_gimple.py"
    from https://github.com/davidmalcolm/gcc-refactoring-scripts
    There is a test suite "test_refactor_gimple.py" which may give a
    clearer idea of the changes that the script makes (and add
    confidence that it's doing the right thing).
    The patch converts code of the form:
      {
        GIMPLE_CHECK (gs, SOME_CODE);
        gimple_subclass_get/set_some_field (gs, value);
      }
    to code of this form:
      {
        some_subclass *stmt = as_a <some_subclass> (gs);
        stmt->some_field = value;
      }
    It also autogenerates specializations of 
        is_a_helper <T>::test
    equivalent to a GIMPLE_CHECK() for use by is_a and as_a.

  * Patch 4 of 6: This patch implement further specializations of
    is_a_helper <T>::test, for gimple_has_ops and gimple_has_mem_ops.

  * Patch 5 of 6: This patch does the rest of porting from union access
    to subclass access (all the fiddly places that the script in patch 3
    couldn't handle).

  * Patch 6 of 6: This patch adds manual implementation of the GTY
    hooks, written by heavily-editing the autogenerated gtype-desc.c
    code from an earlier incarnation.   It implements the equivalent
    of chain_next for gimple statements.

Successfully bootstrapped and tested on x86_64-unknown-linux-gnu: all
testcases show the same results as an unpatched build (relative to
r202029).

OK for trunk?

 gcc/Makefile.in           |    2 +-
 gcc/coretypes.h           |    5 +-
 gcc/ggc.h                 |    6 +-
 gcc/gimple-iterator.c     |   72 +--
 gcc/gimple-pretty-print.c |    6 +-
 gcc/gimple-pretty-print.h |    4 +-
 gcc/gimple-streamer-in.c  |   19 +-
 gcc/gimple-streamer-out.c |    2 +-
 gcc/gimple.c              |  819 +++++++++++++++++++++++-
 gcc/gimple.h              | 1545 +++++++++++++++++++++++++++++----------------
 gcc/gimplify.c            |    4 +-
 gcc/system.h              |    2 +-
 gcc/tree-cfg.c            |    6 +-
 gcc/tree-inline.c         |    2 +-
 gcc/tree-phinodes.c       |   39 +-
 gcc/tree-ssa-ccp.c        |    2 +-
 16 files changed, 1860 insertions(+), 675 deletions(-)

-- 
1.7.11.7

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

end of thread, other threads:[~2014-07-29  8:07 UTC | newest]

Thread overview: 117+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-31  5:45 RFC: gimple.[ch] break apart Andrew MacLeod
2013-10-31  6:15 ` Jeff Law
2013-10-31 16:41 ` [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3) David Malcolm
2013-10-31 16:27   ` [PATCH 2/6] Hand-written port of various accessors within gimple.h David Malcolm
2013-11-14  9:53     ` Jeff Law
2013-10-31 16:27   ` [PATCH 1/6] Convert gimple types from a union to C++ inheritance David Malcolm
2013-11-14 23:00     ` Jeff Law
2013-11-19  0:22       ` David Malcolm
2013-11-19  8:49         ` Jeff Law
2013-10-31 16:27   ` [PATCH 4/6] Implement is_a_helper <>::test specializations for various gimple types David Malcolm
2013-11-14  9:41     ` Jeff Law
2013-11-18 19:51       ` David Malcolm
2013-11-18 20:10         ` Andrew MacLeod
2013-11-18 20:15         ` Jeff Law
2013-10-31 16:31   ` [PATCH 3/6] Automated part of conversion of gimple types to use C++ inheritance David Malcolm
2013-11-14  9:43     ` Jeff Law
2013-11-18 22:17       ` [PATCH] Updated automated patch (was Re: [PATCH 3/6] Automated part of conversion of gimple types to use C++ inheritance) David Malcolm
2013-11-19  8:49         ` Jeff Law
2013-11-19 16:36           ` David Malcolm
2013-11-22  0:27         ` Jakub Jelinek
2013-11-22  0:35           ` Jeff Law
2013-11-22  1:51             ` Jakub Jelinek
2013-11-22  2:52               ` Andrew MacLeod
2013-11-22  3:48                 ` David Malcolm
2013-11-25 18:09                 ` [PATCH] Fix checking of gimple types David Malcolm
2013-11-25 18:42                   ` Michael Matz
2013-11-25 22:18                   ` Jeff Law
2013-11-27  8:54                     ` David Malcolm
2014-07-23 13:16                   ` Thomas Schwinge
2014-07-24  1:50                     ` David Malcolm
2014-07-29  8:11                       ` Thomas Schwinge
2013-10-31 16:46   ` [PATCH 6/6] Update gdb hooks to reflect changes to " David Malcolm
2013-11-14  9:10     ` Jeff Law
2013-10-31 16:49   ` [PATCH 5/6] Port various places from union access to subclass access David Malcolm
2013-11-14  9:23     ` Jeff Law
2013-11-19  0:52       ` David Malcolm
2013-10-31 18:43   ` [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3) Basile Starynkevitch
2013-11-01 21:36   ` Andrew MacLeod
2013-11-01 21:41     ` Jakub Jelinek
2013-11-01 21:47       ` Andrew MacLeod
2013-11-01 21:57         ` Jakub Jelinek
2013-11-01 22:58           ` David Malcolm
2013-11-04 13:23             ` Andrew MacLeod
2013-11-04 21:52               ` David Malcolm
2013-11-04 22:09                 ` David Malcolm
2013-11-04 22:31                   ` Andrew MacLeod
2013-11-05 21:27                     ` Jeff Law
2013-11-04 22:43                   ` Andrew MacLeod
2013-11-04 22:28                 ` Jakub Jelinek
2013-11-04 22:49                   ` Andrew MacLeod
2013-11-05 21:09                   ` Jeff Law
2013-11-05 11:53                 ` Richard Biener
2013-11-05 12:33                   ` David Malcolm
2013-11-05 12:52                     ` Richard Biener
2013-11-04 14:00           ` Andrew MacLeod
2013-11-04 14:01             ` Jakub Jelinek
2013-11-04 14:15               ` Andrew MacLeod
2013-11-04 18:23       ` Jeff Law
2013-11-01 22:43     ` David Malcolm
2013-11-01 23:43       ` Trevor Saunders
2013-11-04 13:15       ` Andrew MacLeod
2013-11-05 17:23     ` [PATCH] Add gimple subclasses for every gimple code (was Re: [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3)) David Malcolm
2013-11-06 16:53       ` Michael Matz
2013-11-07  6:19         ` David Malcolm
2013-11-07  7:08           ` Jeff Law
2013-11-08 19:23             ` David Malcolm
2013-11-14  8:38               ` Jeff Law
2013-11-14 15:06                 ` Michael Matz
2013-11-14 18:32                 ` David Malcolm
2013-11-15  2:49                   ` Jeff Law
2013-11-07 14:57           ` Michael Matz
2013-11-08  0:07         ` Alec Teal
2013-11-08 14:31           ` Michael Matz
2013-11-05 18:22     ` [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3) Andrew MacLeod
2013-11-05 21:33   ` Jeff Law
2013-11-05 22:01     ` David Malcolm
2013-11-05 22:17       ` Jeff Law
2013-11-06  1:14         ` Ian Lance Taylor
2013-11-06 20:49           ` Jeff Law
2013-11-06 20:57             ` Trevor Saunders
2013-11-05 22:24       ` Andrew MacLeod
2013-11-05 22:12     ` Andrew MacLeod
2013-11-06  9:37     ` Richard Biener
2013-11-06 11:20       ` Bernd Schmidt
2013-11-06 11:43         ` Richard Biener
2013-11-06 11:53           ` Jakub Jelinek
2013-11-06 13:14             ` Richard Biener
2013-11-06 13:23               ` Jakub Jelinek
2013-11-06 16:42                 ` David Malcolm
2013-11-06 16:55                   ` Jakub Jelinek
2013-11-06 18:34                     ` Tom Tromey
2013-11-06 19:15                       ` Jeff Law
2013-11-06 20:05                         ` Tom Tromey
2013-11-06 20:45                           ` Jeff Law
2013-11-06 13:31             ` Joseph S. Myers
2013-11-06 21:25               ` Jeff Law
2013-11-06 21:09             ` Jeff Law
2013-11-06 12:42           ` Bernd Schmidt
2013-11-06 21:04           ` Jeff Law
2013-11-06 21:06           ` Andrew MacLeod
2013-11-06 21:52             ` Jeff Law
2013-11-07 10:29             ` Richard Biener
2013-11-07 14:01               ` Joseph S. Myers
2013-11-07 14:42                 ` Richard Biener
2013-11-07 14:53               ` Andrew MacLeod
2013-11-10 12:35             ` Richard Sandiford
2013-11-10 15:27               ` Richard Biener
2013-11-06 11:56         ` Eric Botcazou
2013-11-06 20:51         ` Jeff Law
2013-11-06 21:26       ` Jeff Law
2013-11-14  8:40   ` Jeff Law
2013-11-05 16:58 ` [patch] Create gimple-expr..[ch] ... was Re: RFC: gimple.[ch] break apart Andrew MacLeod
2013-11-05 17:52   ` Jeff Law
2013-11-07 10:58   ` Basile Starynkevitch
2013-11-07 13:47     ` Andrew MacLeod
2013-11-07 18:13       ` Diego Novillo
  -- strict thread matches above, loose matches on Subject: below --
2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
2013-08-29 16:20 ` [PATCH 4/6] Implement is_a_helper <>::test specializations for various gimple types David Malcolm

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