public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Convert gimple to a C++ class hierarchy
@ 2013-08-29 16:20 David Malcolm
  2013-08-29 16:20 ` [PATCH 5/6] Port various places from union access to subclass access David Malcolm
                   ` (7 more replies)
  0 siblings, 8 replies; 44+ 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] 44+ messages in thread

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

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-29 16:20 [PATCH 0/6] Convert gimple to a C++ class hierarchy David Malcolm
2013-08-29 16:20 ` [PATCH 5/6] Port various places from union access to subclass access David Malcolm
2013-08-29 16:20 ` [PATCH 4/6] Implement is_a_helper <>::test specializations for various gimple types David Malcolm
2013-08-29 16:20 ` [PATCH 1/6] Convert gimple types from a union to a C++ class hierarchy David Malcolm
2013-08-29 16:20 ` [PATCH 3/6] Autogenerated conversion of gimple to C++ David Malcolm
2013-08-31 13:40   ` [PATCH 3/6 v2] " David Malcolm
2013-08-29 16:20 ` [PATCH 2/6] Hand-written port of various accessors within gimple.h David Malcolm
2013-08-29 17:04 ` [PATCH 6/6] Add manual GTY hooks David Malcolm
2013-08-29 19:50   ` Steven Bosscher
2013-08-30  8:12     ` Richard Biener
2013-08-31 14:30       ` David Malcolm
2013-08-31 19:52         ` Richard Biener
2013-09-04 15:40           ` David Malcolm
2013-09-05  9:08             ` Richard Biener
2013-09-05 10:49             ` Diego Novillo
2013-08-30  8:09   ` Richard Biener
2013-08-31 13:48     ` [PATCH 6/6 v2] " David Malcolm
2013-08-30  8:43   ` [PATCH 6/6] " Richard Biener
2013-08-30 20:25     ` David Malcolm
2013-08-30 13:58 ` [PATCH 0/6] Convert gimple to a C++ class hierarchy Michael Matz
2013-08-30 14:02   ` Gabriel Dos Reis
2013-08-30 14:03     ` Jakub Jelinek
2013-08-30 14:26       ` Gabriel Dos Reis
2013-08-30 14:49         ` David Malcolm
2013-08-30 15:21         ` Michael Matz
2013-08-30 15:26           ` Gabriel Dos Reis
2013-08-30 15:31           ` Diego Novillo
2013-08-30 15:40             ` Jakub Jelinek
2013-08-30 15:50               ` Diego Novillo
2013-08-31 10:53                 ` Richard Biener
2013-08-31 13:34                   ` Basile Starynkevitch
2013-08-31 14:04                   ` Diego Novillo
2013-08-30 15:54               ` Gabriel Dos Reis
2013-08-31  9:56               ` Richard Biener
2013-08-30 19:38   ` David Malcolm
2013-09-02 11:45     ` Michael Matz
2013-09-04  1:08       ` David Malcolm
2013-09-04  1:12       ` David Malcolm
2013-09-04 13:22         ` Michael Matz
2013-09-02 12:35     ` Martin Jambor
2013-09-04  1:20       ` David Malcolm
2013-09-04 14:09         ` Jan Hubicka
2013-09-04 14:49           ` Mike Stump
2013-08-30 21:51 ` 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).