On Fri, May 2, 2014 at 11:56 PM, David Malcolm <dmalc...@redhat.com> wrote: > This patch series demonstrates a way of reimplementing the 89-patch series: > "[PATCH 00/89] Compile-time gimple-checking" > http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01148.html > > whilst avoiding introducing a pair of "gimple_foo/const_gimple_foo" typedefs > for each subclass. > > It eliminates the "gimple" and "const_gimple" typedefs, > renaming "gimple_statement_base" to "gimple_stmt", giving types: > "gimple_stmt *" and "const gimple_stmt *" > thoughout the middle-end. The rest of the gimple statement classes are > renamed, converting the various > gimple_statement_with_FOO > to: > gimple_stmt_with_FOO > and the remainder: > gimple_statement_SOME_SUBCLASS > to just: > gimple_SOME_SUBCLASS > > The idea is then to reimplement the earlier patch series, porting many of > these: > gimple_stmt *something > to point to some more concrete subclass; I've done this for GIMPLE_SWITCH.
Thanks for doing this. > It requires two patches that I've already posted separately: > > (A): "[PATCH] gengtype: Support explicit pointers in template arguments": > http://gcc.gnu.org/ml/gcc-patches/2014-05/msg00003.html > (which apparently will need reworking after wide-int is merged; > oh well). I'll look at this after the wide-int merge (which hopefully happens soon...) > (B): "[PATCH 19/89] Const-correctness of gimple_call_builtin_p": > http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01194.html > (I have a separate bootstrap®rtest in progress for just this one, > which appears to be pre-approved per Jeff's earlier comments). > > Of the 3 patches in the series itself: > > Patch 1: > This one is handwritten: it renames the gimple statement classes in > their declarations, in the docs, in the selftests and renames explicit > uses of *subclasses*. > > Patch 2: > This one is autogenerated: it does the mass conversion of "gimple" to > "gimple_stmt *" and "const_gimple" to "const gimple_stmt *" throughout > the code. > > The conversion script is at: > > https://github.com/davidmalcolm/gcc-refactoring-scripts/blob/master/rename_gimple.py > > It's not a real C++ parser, but it has some smarts for handling > awkward cases like: > gimple def0, def2; > which must become: > gimple_stmt *def0, *def2; > ^ note the second '*' character. > > You can see the selftests for the conversion script at: > > https://github.com/davidmalcolm/gcc-refactoring-scripts/blob/master/test_rename_gimple.py > > Patch 3: > This one is a port of the previously-posted: > "[PATCH 02/89] Introduce gimple_switch and use it in various places" > http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01154.html > to the new approach. As per an earlier revision: > http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01549.html > it *eliminates* the > stmt->as_a_gimple_switch () > and > stmt->dyn_cast_gimple_switch () > casting methods from the base class in favor of direct usage of is-a.h: > as_a <gimple_switch *> (stmt) > and > dyn_cast <gimple_switch *> (stmt) > > This version of the patch also eliminates the > gimple_switch / const_gimple_switch > typedefs from the initial patch series in favor of "gimple_switch" > being the name of the subclass. I wonder if, when we intoduce a gimple namespace, we can write dyn_cast <switch *> (stmt) by means of ADL? Thus, imagine just doing namespace gimple { ... gimple.h contents ... } using gimple::gimple; ... a few more? ... typedef gimple::gimple gimple; And then gimple *stmt; gimple::switch *s = dyn_cast <switch *> (stmt); (yeah, awkward 'switch' reserved keyword as you say below ...) Thus, for all the new explicit sub-types you introduce use the namespace syntax. Can ADL work in our favors here with the way dyn_cast and friends work? Thus, do we really need to write dyn_cast <gimple::switch *> (stmt)? No, I'm not asking you to do that now (but a limited form of a gimple namespace would be nice to have - using the "old-style" 'gimple' type avoids changing too much code I suppose). Just sth that you don't run out of work ;) > Hopefully porting this first patch in the series gives a sense of what > the end-result would look like. > > There are a few possible variations on the names we could pick in this > approach. > > I deliberately renamed "gimple" to "gimple_stmt" since IIRC Andrew MacLeod > had suggested something like this, for the potential of making "gimple" > be a namespace. That said, using namespaces may be controversial, and > would need further gengtype support, which may be tricky (i.e. fully teach > gengtype about C++ namespaces, which may be a gengtype hack too far). > [I'd much prefer to avoid C++ namespaces in the core code, mostly because > of gengtype]. > > Also, AIUI, Andrew is looking at introducing concepts of gimple types and > gimple expressions, so "gimple" may no longer imply a *statement*. > > Alternatively, we could make the base class be just "gimple" (which would > be more consistent with the names of the accessor functions). I strongly prefer to name it 'gimple', not 'gimple_stmt'. Because it's less to type, and because it will make all other types shorter as well. And because 'gimple' _is_ a stmt right now, so gimple_stmt is redundant. Same applies to gimple_stmt_with_FOO, just make it gimple_with_FOO. I understand the namespace issue, but we don't have a namespace right now. Also gimple::gimple works just fine, no? > There's also the "bargain basement" namespaces approach, where we don't > have an implicit "gimple" namespace, but just *pretend* we do, and rename > the base type to "stmt", with e.g. "gimple_statement_phi" becoming just > "phi". ["gimple_switch" would need to become "switch_", say, to avoid the > reserved word]. Ick (for the 'switch' case ... CamelCase anyone? :)). > Successfully bootstrapped and regrtested on x86_64-unknown-linux-gnu > (Fedora 20) (on top of the 2 dependent patches mentioned above; equal > results compared to a control build of r209953. > > How does this look? (for trunk; after 4.9.1 is released). Not looking at the exact patches right now. Thanks, Richard. > > David Malcolm (3): > Handwritten part of conversion of "gimple" to "gimple_stmt *" > Autogenerated part of conversion of "gimple" to "gimple_stmt *" > Introduce gimple_switch and use it in various places > > gcc/asan.c | 36 +- > gcc/builtins.c | 10 +- > gcc/builtins.h | 2 +- > gcc/c-family/c-gimplify.c | 4 +- > gcc/calls.c | 2 +- > gcc/calls.h | 2 +- > gcc/cfgexpand.c | 48 +- > gcc/cfgexpand.h | 2 +- > gcc/cfgloop.c | 2 +- > gcc/cfgloop.h | 2 +- > gcc/cfgloopmanip.c | 4 +- > gcc/cgraph.c | 32 +- > gcc/cgraph.h | 24 +- > gcc/cgraphbuild.c | 12 +- > gcc/cgraphclones.c | 8 +- > gcc/cgraphunit.c | 12 +- > gcc/config/aarch64/aarch64-builtins.c | 4 +- > gcc/config/alpha/alpha.c | 14 +- > gcc/config/i386/i386.c | 14 +- > gcc/config/rs6000/rs6000.c | 4 +- > gcc/coretypes.h | 10 +- > gcc/cp/cp-gimplify.c | 2 +- > gcc/doc/gimple.texi | 824 ++++++++------ > gcc/dumpfile.c | 4 +- > gcc/dumpfile.h | 4 +- > gcc/except.h | 2 +- > gcc/expr.c | 28 +- > gcc/expr.h | 2 +- > gcc/fold-const.c | 2 +- > gcc/fold-const.h | 2 +- > gcc/gdbhooks.py | 4 +- > gcc/ggc.h | 6 +- > gcc/gimple-builder.c | 26 +- > gcc/gimple-builder.h | 16 +- > gcc/gimple-fold.c | 58 +- > gcc/gimple-fold.h | 8 +- > gcc/gimple-iterator.c | 36 +- > gcc/gimple-iterator.h | 22 +- > gcc/gimple-low.c | 30 +- > gcc/gimple-low.h | 2 +- > gcc/gimple-pretty-print.c | 110 +- > gcc/gimple-pretty-print.h | 12 +- > gcc/gimple-ssa-isolate-paths.c | 14 +- > gcc/gimple-ssa-strength-reduction.c | 92 +- > gcc/gimple-ssa.h | 22 +- > gcc/gimple-streamer-in.c | 12 +- > gcc/gimple-streamer-out.c | 8 +- > gcc/gimple-walk.c | 18 +- > gcc/gimple-walk.h | 12 +- > gcc/gimple.c | 315 +++--- > gcc/gimple.h | 1782 > +++++++++++++++--------------- > gcc/gimplify-me.c | 4 +- > gcc/gimplify-me.h | 2 +- > gcc/gimplify.c | 102 +- > gcc/gimplify.h | 12 +- > gcc/graphite-poly.c | 8 +- > gcc/graphite-scop-detection.c | 20 +- > gcc/graphite-sese-to-poly.c | 168 +-- > gcc/gsstruct.def | 52 +- > gcc/internal-fn.c | 40 +- > gcc/internal-fn.h | 2 +- > gcc/ipa-inline-analysis.c | 45 +- > gcc/ipa-inline.c | 4 +- > gcc/ipa-profile.c | 2 +- > gcc/ipa-prop.c | 80 +- > gcc/ipa-prop.h | 6 +- > gcc/ipa-pure-const.c | 12 +- > gcc/ipa-ref.c | 10 +- > gcc/ipa-ref.h | 12 +- > gcc/ipa-split.c | 40 +- > gcc/java/java-gimplify.c | 2 +- > gcc/lto-streamer-in.c | 16 +- > gcc/lto-streamer-out.c | 6 +- > gcc/omp-low.c | 253 ++--- > gcc/passes.c | 4 +- > gcc/predict.c | 32 +- > gcc/profile.c | 8 +- > gcc/sese.c | 18 +- > gcc/sese.h | 8 +- > gcc/ssa-iterators.h | 42 +- > gcc/stmt.c | 4 +- > gcc/system.h | 2 +- > gcc/target.def | 2 +- > gcc/testsuite/g++.dg/plugin/selfassign.c | 8 +- > gcc/testsuite/gcc.dg/plugin/selfassign.c | 8 +- > gcc/tracer.c | 4 +- > gcc/trans-mem.c | 114 +- > gcc/trans-mem.h | 2 +- > gcc/tree-affine.c | 2 +- > gcc/tree-call-cdce.c | 50 +- > gcc/tree-cfg.c | 277 ++--- > gcc/tree-cfg.h | 22 +- > gcc/tree-cfgcleanup.c | 28 +- > gcc/tree-cfgcleanup.h | 2 +- > gcc/tree-chrec.c | 10 +- > gcc/tree-chrec.h | 6 +- > gcc/tree-complex.c | 34 +- > gcc/tree-core.h | 4 +- > gcc/tree-data-ref.c | 14 +- > gcc/tree-data-ref.h | 8 +- > gcc/tree-dfa.c | 16 +- > gcc/tree-dfa.h | 2 +- > gcc/tree-eh.c | 199 ++-- > gcc/tree-eh.h | 38 +- > gcc/tree-emutls.c | 8 +- > gcc/tree-if-conv.c | 34 +- > gcc/tree-inline.c | 105 +- > gcc/tree-inline.h | 6 +- > gcc/tree-into-ssa.c | 62 +- > gcc/tree-into-ssa.h | 4 +- > gcc/tree-loop-distribution.c | 67 +- > gcc/tree-nested.c | 32 +- > gcc/tree-nrv.c | 8 +- > gcc/tree-object-size.c | 28 +- > gcc/tree-outof-ssa.c | 26 +- > gcc/tree-outof-ssa.h | 4 +- > gcc/tree-parloops.c | 52 +- > gcc/tree-pass.h | 6 +- > gcc/tree-phinodes.c | 52 +- > gcc/tree-phinodes.h | 14 +- > gcc/tree-predcom.c | 62 +- > gcc/tree-profile.c | 32 +- > gcc/tree-scalar-evolution.c | 72 +- > gcc/tree-scalar-evolution.h | 2 +- > gcc/tree-sra.c | 62 +- > gcc/tree-ssa-alias.c | 38 +- > gcc/tree-ssa-alias.h | 14 +- > gcc/tree-ssa-ccp.c | 48 +- > gcc/tree-ssa-coalesce.c | 10 +- > gcc/tree-ssa-copy.c | 14 +- > gcc/tree-ssa-copyrename.c | 2 +- > gcc/tree-ssa-dce.c | 40 +- > gcc/tree-ssa-dom.c | 90 +- > gcc/tree-ssa-dom.h | 2 +- > gcc/tree-ssa-dse.c | 10 +- > gcc/tree-ssa-forwprop.c | 154 +-- > gcc/tree-ssa-ifcombine.c | 20 +- > gcc/tree-ssa-live.c | 18 +- > gcc/tree-ssa-loop-ch.c | 6 +- > gcc/tree-ssa-loop-im.c | 70 +- > gcc/tree-ssa-loop-ivcanon.c | 20 +- > gcc/tree-ssa-loop-ivopts.c | 68 +- > gcc/tree-ssa-loop-manip.c | 32 +- > gcc/tree-ssa-loop-niter.c | 56 +- > gcc/tree-ssa-loop-niter.h | 4 +- > gcc/tree-ssa-loop-prefetch.c | 16 +- > gcc/tree-ssa-loop-unswitch.c | 8 +- > gcc/tree-ssa-loop.h | 2 +- > gcc/tree-ssa-math-opts.c | 90 +- > gcc/tree-ssa-operands.c | 42 +- > gcc/tree-ssa-operands.h | 10 +- > gcc/tree-ssa-phiopt.c | 86 +- > gcc/tree-ssa-phiprop.c | 18 +- > gcc/tree-ssa-pre.c | 52 +- > gcc/tree-ssa-propagate.c | 46 +- > gcc/tree-ssa-propagate.h | 14 +- > gcc/tree-ssa-reassoc.c | 176 +-- > gcc/tree-ssa-sccvn.c | 58 +- > gcc/tree-ssa-sccvn.h | 8 +- > gcc/tree-ssa-sink.c | 22 +- > gcc/tree-ssa-strlen.c | 42 +- > gcc/tree-ssa-structalias.c | 44 +- > gcc/tree-ssa-tail-merge.c | 40 +- > gcc/tree-ssa-ter.c | 12 +- > gcc/tree-ssa-threadedge.c | 46 +- > gcc/tree-ssa-threadedge.h | 4 +- > gcc/tree-ssa-threadupdate.c | 10 +- > gcc/tree-ssa-uncprop.c | 12 +- > gcc/tree-ssa-uninit.c | 78 +- > gcc/tree-ssa.c | 40 +- > gcc/tree-ssa.h | 4 +- > gcc/tree-ssanames.c | 8 +- > gcc/tree-ssanames.h | 16 +- > gcc/tree-stdarg.c | 12 +- > gcc/tree-switch-conversion.c | 50 +- > gcc/tree-tailcall.c | 30 +- > gcc/tree-vect-data-refs.c | 100 +- > gcc/tree-vect-generic.c | 22 +- > gcc/tree-vect-loop-manip.c | 62 +- > gcc/tree-vect-loop.c | 174 +-- > gcc/tree-vect-patterns.c | 194 ++-- > gcc/tree-vect-slp.c | 124 +-- > gcc/tree-vect-stmts.c | 244 ++-- > gcc/tree-vectorizer.c | 22 +- > gcc/tree-vectorizer.h | 116 +- > gcc/tree-vrp.c | 152 +-- > gcc/tree.c | 4 +- > gcc/tree.h | 4 +- > gcc/tsan.c | 12 +- > gcc/ubsan.c | 20 +- > gcc/value-prof.c | 96 +- > gcc/value-prof.h | 26 +- > gcc/vtable-verify.c | 10 +- > 193 files changed, 4851 insertions(+), 4670 deletions(-) > > -- > 1.8.5.3 >
