This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC for next stage1 00/16] Compile-time gimple-checking


As of r205034 (de6bd75e3c9bc1efe8a6387d48eedaa4dafe622d) and
r205428 (a90353203da18288cdac1b0b78fe7b22c69fe63f) the various gimple
statements form a C++ inheritance hierarchy, but we're not yet making much
use of that in the code: everything refers to just "gimple" (or
"const_gimple"), and type-checking is performed at run-time within the
various gimple_foo_* accessors in gimple.h, and almost nowhere else.

The following patch series introduces compile-time checking of much of
the handling of gimple statements.

Various new typedefs are introduced for pointers to statements where the
specific code is known, matching the corresponding names from gimple.def.

For example, it introduces a "gimple_bind" typedef, which is a
  (gimple_statement_bind *)
which has the invariant that stmt->code == GIMPLE_BIND.

All of the gimple_bind_* accessors in gimple.h are converted from taking
just a "gimple" to a "gimple_bind".

Various other expressions have their types strengthened from "gimple" to
"gimple_bind", and from plain vec<gimple> to vec<gimple_bind> (e.g. within
gimplify.c for handling the bind stack).

Other such typedefs that are introduced are:
  * gimple_switch
  * gimple_cond
  * gimple_assign
  * gimple_label
  * gimple_debug
  * gimple_phi
with various analogous strengthening of types, such as for parameters of
functions, for variables, and for fields of some structs - though it's
only for gimple_bind that I managed to make all the gimple.h accessors
type-safe.

Where necessary a new gimple_with_ops subclass is added [1], with the
same layout as before, merely adding an invariant on the code. [2]

In each case there are some checked downcasts from "gimple" down to the more
concrete statement-type, so that the runtime-checking in the checked build
happens there, at the boundary between types, rather than the current
checking, which is every time an accessor is called and almost nowhere else.
Once we're in a more concrete type than "gimple", the compiler can enforce
the type-checking for us at compile-time.

For example, various variables in tree-into-ssa.c change from just
vec<gimple> to being vec<gimple_phi>, capturing the "phi-ness" of the
contents as a compile-time check (and then not needing to check them any
more).  Similarly, within tree-inline.h's struct copy_body_data, the field
"debug_stmts" can be "concretized" from a vec<gimple> to a
vec<gimple_debug>.

In doing the checked downcasts I ran into the verbosity of the as_a <>
API.  I first tried simplifying them with custom functions e.g.:

   static inline gimple_bind as_a_gimple_bind (gimple gs)
   {
     return as_a <gimple_statement_bind> (gs);
   }

but the approach I've gone with makes these checked casts be methods of
the gimple_statement_base class, so that e.g. in a switch statement you
can write:

    case GIMPLE_SWITCH:
      dump_gimple_switch (buffer, gs->as_a_gimple_switch (), spc, flags);
      break;

where the ->as_a_gimple_switch is a no-op cast from "gimple" to the more
concrete "gimple_switch" in a release build, with runtime checking for
code == GIMPLE_SWITCH added in a checked build (it uses as_a <>
internally).

This is much less verbose than trying to do it with as_a <> directly, and
I think doing it as a method reads better aloud (to my English-speaking
mind, at-least):
  "gs as a gimple switch",
as opposed to:
  "as a gimple switch... gs".

It makes the base class a little cluttered, but IMHO it hits a sweet-spot
of readability and type-safety with relatively little verbosity (only 8
more characters than doing it with a raw C-style cast).   Another
advantage of having the checked cast as a *method* is that it implies that
the input must be non-NULL (which is the case).

For phis, I made gsi_start_phis return a gimple_phi_iterator, rather than
a gimple_stmt_iterator, where the phi iterator type is new, a subclass of
gimple_stmt_iterator.  It has identical layout, but adds a "phi ()" method,
which is like gsi_stmt(), but does a checked cast to a gimple_phi.  This
allows great swathes of phi-manipulation code to be concretized into
working on type "gimple_phi" (rather than just "gimple"), with minimal
patching.

Having these typedefs around may also be useful for debugging, so that you
can now type e.g.

  (gdb) p *(gimple_cond)stmt

to quickly get all fields of a stmt (without having to think
"is this code a gimple_statement_with_ops?")

Successfully bootstrapped&regtested the cumulative effect of the patches on
x86_64-unknown-linux-gnu (albeit without cloog and thus graphite I now
realize), on top of r205716.

It's a work-in-progress: I'm posting it now to confirm that I'm on the
right track.

I'm excited about this approach: but given how much code it changes, I'm
guessing this is more appropriate for the next stage1 than 4.9's stage3,
(in which case I can start a branch).   Though if there's a sudden rush of
enthusiasm for more C++ I'm happy to apply it to trunk now :)

(that said patch 1 in the series probably *is* appropriate for trunk as-is).

Thoughts?

Dave

[1] the exception is gimple_assign, which subclasses
gimple_with_memory_ops instead.

[2] the GTY tag is specified as GSS_WITH_OPS for such subclasses (other
than gimple assign): gengtype's inheritance-handling requires every type
in an inheritance hierarchy to have a tag specified, or it can silently
fail to mark instances of such types; as of r205428 it can handle
duplicate values for the type tag within one hierarchy, so it's OK for
them all to have "GSS_WITH_OPS"; they'll all use that case within the
GC-marking switch-statement in gtype-desc.c


David Malcolm (16):
  Const-correctness fixes for some gimple accessors
  Introduce gimple_switch and use it in various places
  Introduce gimple_bind and use it for accessors.
  Introduce gimple_cond and use it in various places
  Introduce gimple_assign and use it in various places
  Introduce gimple_label and use it in a few places
  Introduce gimple_debug and use it in a few places
  Introduce gimple_phi and use it in various places
  Introduce gimple_phi_iterator
  Update ssa_prop_visit_phi_fn to take a gimple_phi
  tree-parloops.c: use gimple_phi in various places
  tree-predcom.c: use gimple_phi in various places
  tree-ssa-phiprop.c: use gimple_phi
  tree-ssa-loop-niter.c: use gimple_phi in a few places
  tree-ssa-loop-manip.c: use gimple_phi in three places
  tree-ssa-loop-ivopts.c: use gimple_phi in a few places

 gcc/asan.c                     |   5 +-
 gcc/c-family/c-gimplify.c      |   4 +-
 gcc/cfgexpand.c                |   6 +-
 gcc/cfgloop.c                  |   6 +-
 gcc/cfgloopmanip.c             |   2 +-
 gcc/coretypes.h                |  32 ++++
 gcc/expr.h                     |   2 +-
 gcc/gdbhooks.py                |  19 ++-
 gcc/gimple-builder.c           |  16 +-
 gcc/gimple-builder.h           |  16 +-
 gcc/gimple-fold.c              |   4 +-
 gcc/gimple-iterator.c          |  12 +-
 gcc/gimple-iterator.h          |  11 +-
 gcc/gimple-low.c               |   5 +-
 gcc/gimple-pretty-print.c      |  27 ++--
 gcc/gimple-ssa-isolate-paths.c |   4 +-
 gcc/gimple-walk.c              |   5 +-
 gcc/gimple.c                   |  76 +++++-----
 gcc/gimple.h                   | 324 +++++++++++++++++++++++++++++++++--------
 gcc/gimplify.c                 |  43 +++---
 gcc/gimplify.h                 |   6 +-
 gcc/ipa-inline-analysis.c      |   7 +-
 gcc/java/java-gimplify.c       |   2 +-
 gcc/omp-low.c                  |  47 +++---
 gcc/sese.c                     |   2 +-
 gcc/stmt.c                     |   4 +-
 gcc/trans-mem.c                |   2 +-
 gcc/tree-cfg.c                 | 104 ++++++-------
 gcc/tree-cfg.h                 |   2 +-
 gcc/tree-cfgcleanup.c          |  12 +-
 gcc/tree-complex.c             |   6 +-
 gcc/tree-eh.c                  |   5 +-
 gcc/tree-if-conv.c             |  13 +-
 gcc/tree-inline.c              |  63 ++++----
 gcc/tree-inline.h              |   2 +-
 gcc/tree-into-ssa.c            |  22 +--
 gcc/tree-loop-distribution.c   |   3 +-
 gcc/tree-nested.c              |  21 +--
 gcc/tree-outof-ssa.c           |  16 +-
 gcc/tree-parloops.c            |  16 +-
 gcc/tree-phinodes.c            |  37 +++--
 gcc/tree-phinodes.h            |   4 +-
 gcc/tree-predcom.c             |  22 +--
 gcc/tree-scalar-evolution.c    |  17 ++-
 gcc/tree-ssa-ccp.c             |   2 +-
 gcc/tree-ssa-coalesce.c        |   4 +-
 gcc/tree-ssa-copy.c            |   2 +-
 gcc/tree-ssa-dce.c             |  10 +-
 gcc/tree-ssa-dom.c             |  28 ++--
 gcc/tree-ssa-forwprop.c        |  11 +-
 gcc/tree-ssa-ifcombine.c       |  23 +--
 gcc/tree-ssa-live.c            |   8 +-
 gcc/tree-ssa-loop-im.c         |   4 +-
 gcc/tree-ssa-loop-ivopts.c     |  26 ++--
 gcc/tree-ssa-loop-manip.c      |  28 ++--
 gcc/tree-ssa-loop-niter.c      |  15 +-
 gcc/tree-ssa-loop-unswitch.c   |  19 ++-
 gcc/tree-ssa-phiopt.c          |   4 +-
 gcc/tree-ssa-phiprop.c         |  10 +-
 gcc/tree-ssa-propagate.c       |   2 +-
 gcc/tree-ssa-propagate.h       |   2 +-
 gcc/tree-ssa-reassoc.c         |   4 +-
 gcc/tree-ssa-sccvn.c           |   6 +-
 gcc/tree-ssa-tail-merge.c      |   8 +-
 gcc/tree-ssa-threadedge.c      |  12 +-
 gcc/tree-ssa-threadedge.h      |   2 +-
 gcc/tree-ssa-threadupdate.c    |  16 +-
 gcc/tree-ssa-uncprop.c         |   7 +-
 gcc/tree-ssa-uninit.c          |  22 +--
 gcc/tree-ssa.c                 |  12 +-
 gcc/tree-switch-conversion.c   |  36 ++---
 gcc/tree-tailcall.c            |   8 +-
 gcc/tree-vect-loop-manip.c     |  34 ++---
 gcc/tree-vect-loop.c           |   4 +-
 gcc/tree-vrp.c                 |  44 +++---
 gcc/value-prof.c               |   4 +-
 76 files changed, 900 insertions(+), 571 deletions(-)

-- 
1.7.11.7


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]