On Mon, Apr 21, 2014 at 6:56 PM, David Malcolm <dmalc...@redhat.com> wrote: > This is a greatly-expanded version of: > http://gcc.gnu.org/ml/gcc-patches/2013-12/msg01262.html > > 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.
Even though I like these changes in principle I also wear a release managers hat. Being one of the persons doing frequent backports of trunk fixes to branches this will cause a _lot_ of headache. So ... can we delay this until, say, 4.9.1 is out? Thanks, Richard. > For example, it introduces a "gimple_bind" typedef, which is a > (gimple_statement_bind *) > which has the invariant that stmt->code == GIMPLE_BIND. > > The idea is that all of the gimple_foo_* accessors in gimple.h are > converted from taking just a "gimple" to a "gimple_foo". I've managed > this so far for 15 of the gimple statement subclasses; for example, all > of the "gimple_bind_*" accessors now require a "gimple_bind" rather than > a plain "gimple". > > Similarly, variabless throughout the middle-end have their types > strengthened from plain "gimple" to a typedef expressing a pointer to > some concrete statement subclass, and similarly for vectors. For example > various variables 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). > > Numerous other such typedefs are introduced: essentially two for each of > the gimple code values: a gimple_foo and a const_gimple_foo variant e.g. > gimple_switch and const_gimple_switch (some of the rarer codes don't have > such typedefs yet). > > Some of these typedefs are aliases for existing subclasses within the > "gimple" class hierarchy, but others are new with this patch series. As > with the existing subclasses, they don't add any extra fields, they merely > express invariants on the gimple's code. > > 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. > > An additional benefit is that human readers of the code should (I hope) > have an easier time following what's going on: assumptions about the > underlying gimple_code of a stmt that previously were hidden are now > obvious, expressed directly in the type system. > > 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); indeed great swathes of phi-manipulation code are changed from > acting on vanilla "gimple" to acting on "gimple_phi". > > 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>. > > Another notable such concretization is that the "call_stmt" field of a > cgraph_edge becomes a gimple_call, rather than a plain gimple. > > In doing the checked downcasts I ran into the verbosity of the as_a <> > API (in our "is-a.h"). 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", > which I find clunky. > > 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 implicitly > documents the requirement that the input must be non-NULL. > > There are an analogous family of "dyn_cast_gimple_foo" methods in the > base class, again as an abbreviation of the rather verbose is-a.h API. > > 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 lots 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?") > > *********** > Correctness > *********** > Successfully bootstrapped®tested the cumulative effect of the patches > on x86_64-unknown-linux-gnu with cloog and thus graphite, with all > frontends (via --enable-languages=all,ada,go), on top of r209027: the > regrtest shows identical results to that of a control build, in both cases > with: > > gcc/testsuite/ada/acats/acats.sum : total: 2320 PASS: 2320 > gcc/testsuite/g++/g++.sum : total: 90205 FAIL: 3 PASS: 86738 XFAIL: 445 > UNSUPPORTED: 3019 > gcc/testsuite/gcc/gcc.sum : total: 110165 FAIL: 31 PASS: 108028 XFAIL: 263 > XPASS: 40 UNSUPPORTED: 1803 > gcc/testsuite/gfortran/gfortran.sum : total: 45650 PASS: 45533 XFAIL: 52 > UNSUPPORTED: 65 > gcc/testsuite/gnat/gnat.sum : total: 1245 PASS: 1224 XFAIL: 18 UNSUPPORTED: 3 > gcc/testsuite/go/go.sum : total: 7266 PASS: 7258 XFAIL: 1 UNTESTED: 6 > UNSUPPORTED: 1 > gcc/testsuite/objc/objc.sum : total: 2973 PASS: 2893 XFAIL: 6 UNSUPPORTED: 74 > x86_64-unknown-linux-gnu/boehm-gc/testsuite/boehm-gc.sum : total: 13 PASS: > 12 UNSUPPORTED: 1 > x86_64-unknown-linux-gnu/libatomic/testsuite/libatomic.sum : total: 54 PASS: > 54 > x86_64-unknown-linux-gnu/libffi/testsuite/libffi.sum : total: 1856 PASS: > 1801 UNSUPPORTED: 55 > x86_64-unknown-linux-gnu/libgo/libgo.sum : total: 122 PASS: 122 > x86_64-unknown-linux-gnu/libgomp/testsuite/libgomp.sum : total: 2412 PASS: > 2412 > x86_64-unknown-linux-gnu/libitm/testsuite/libitm.sum : total: 30 PASS: 26 > XFAIL: 3 UNSUPPORTED: 1 > x86_64-unknown-linux-gnu/libjava/testsuite/libjava.sum : total: 2586 PASS: > 2582 XFAIL: 4 > x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.sum : total: 10260 > PASS: 9995 XFAIL: 41 UNSUPPORTED: 224 > > apart from a test removed/test added false positive in: > go.test/test/dwarf/dwarf.dir/main.go > due to the test summary containing embedded absolute paths, which vary > between my control vs experiment builds. > > [FWIW, for the above I used a new DejaVu result-handling tool I've > written, "jamais-vu": > https://github.com/davidmalcolm/jamais-vu > which others on this list may find helpful] > > *********** > Performance > *********** > I benchmarked the compiler with and without this patch series (experiment > and control, respectively), by repeatedly building the two complicated C++ > files supplied by Michael Matz in: > http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00062.html > with stripped cc1plus, using taskset(1) to pin the cc1plus invocations to > cpu 0 for consistency on this NUMA box. > > There were no significant differences in timings or memory usage between > the runs; full details (including links to timeline graphs) follow: > > Compilation of kdecore.cc at -O3 with -g for x86_64-unknown-linux-gnu: usr > control: [47.99, 47.91, 48.0, 47.99, 48.09, 48.36, 48.2, 48.11, 48.22, > 47.88, 48.14, 48.25, 48.17, 48.17, 48.16, 48.24, 48.03, 47.88, 47.98, 47.98, > 48.08, 48.02, 48.0, 47.99, 48.05, 48.21, 48.0, 48.16, 48.08, 48.05, 48.0, > 48.13, 47.92, 47.9, 48.16, 48.03, 48.07, 48.12, 48.16, 48.14] > experiment: [48.48, 48.05, 48.01, 48.19, 47.99, 47.95, 48.0, 48.0, 47.89, > 47.81, 47.75, 47.94, 48.1, 48.02, 47.88, 47.83, 48.16, 48.04, 48.03, 47.98, > 48.12, 48.05, 47.89, 48.03, 47.81, 47.88, 48.06, 47.85, 48.27, 47.99, 47.81, > 48.07, 48.01, 48.06, 48.07, 47.95, 48.03, 47.9, 47.94, 47.99] > Min: 47.880000 -> 47.750000: 1.00x faster > Avg: 48.075500 -> 47.997000: 1.00x faster > Not significant > Stddev: 0.11145 -> 0.13531: 1.2141x larger > Timeline: http://goo.gl/k3ct18 > > Compilation of kdecore.cc at -O3 with -g for x86_64-unknown-linux-gnu: sys > control: [6.19, 6.29, 6.16, 6.25, 6.18, 6.06, 6.06, 6.19, 6.2, 6.27, > 6.11, 6.09, 6.11, 6.0, 6.12, 6.25, 6.1, 6.32, 6.22, 6.23, 6.12, 6.13, 6.22, > 6.13, 6.08, 6.02, 6.15, 6.11, 6.16, 6.16, 6.15, 6.05, 6.22, 6.24, 6.09, 6.12, > 6.11, 6.14, 6.0, 5.99] > experiment: [6.12, 6.1, 6.13, 6.17, 6.16, 6.18, 6.13, 6.15, 6.18, 6.3, > 6.32, 6.17, 6.0, 6.17, 6.25, 6.25, 6.23, 6.15, 6.05, 6.13, 6.04, 6.12, 6.18, > 6.14, 6.27, 6.18, 6.12, 6.23, 6.13, 6.09, 6.24, 6.12, 6.08, 6.07, 6.02, 6.33, > 6.1, 6.19, 6.08, 6.14] > Min: 5.990000 -> 6.000000: 1.00x slower > Avg: 6.144750 -> 6.155250: 1.00x slower > Not significant > Stddev: 0.08105 -> 0.07782: 1.0415x smaller > Timeline: http://goo.gl/wiiVbY > > Compilation of kdecore.cc at -O3 with -g for x86_64-unknown-linux-gnu: wall > control: [54.9, 54.38, 54.34, 54.42, 54.45, 54.6, 54.44, 54.47, 54.6, > 54.33, 54.43, 54.52, 54.46, 54.35, 54.46, 54.67, 54.31, 54.38, 54.37, 54.4, > 54.37, 54.34, 54.41, 54.3, 54.31, 54.41, 54.34, 54.45, 54.43, 54.39, 54.33, > 54.36, 54.31, 54.31, 54.43, 54.33, 54.36, 54.44, 54.34, 54.31] > experiment: [54.96, 54.51, 54.53, 54.68, 54.56, 54.54, 54.54, 54.53, > 54.44, 54.5, 54.47, 54.5, 54.5, 54.56, 54.46, 54.47, 54.78, 54.59, 54.48, > 54.49, 54.5, 54.54, 54.46, 54.52, 54.47, 54.38, 54.47, 54.38, 54.74, 54.42, > 54.4, 54.53, 54.41, 54.48, 54.44, 54.64, 54.49, 54.4, 54.31, 54.4] > Min: 54.300000 -> 54.310000: 1.00x slower > Avg: 54.413750 -> 54.511750: 1.00x slower > Not significant > Stddev: 0.11562 -> 0.11786: 1.0194x larger > Timeline: http://goo.gl/1Xlhp6 > > Compilation of kdecore.cc at -O3 with -g for x86_64-unknown-linux-gnu: ggc > control: [1298648.0, 1298649.0, 1298651.0, 1298656.0, 1298655.0, > 1298651.0, 1298650.0, 1298652.0, 1298651.0, 1298650.0, 1298648.0, 1298644.0, > 1298654.0, 1298655.0, 1298659.0, 1298659.0, 1298650.0, 1298652.0, 1298665.0, > 1298655.0, 1298651.0, 1298655.0, 1298652.0, 1298656.0, 1298659.0, 1298653.0, > 1298645.0, 1298649.0, 1298645.0, 1298643.0, 1298657.0, 1298653.0, 1298652.0, > 1298659.0, 1298648.0, 1298651.0, 1298653.0, 1298651.0, 1298654.0, 1298651.0] > experiment: [1298653.0, 1298658.0, 1298649.0, 1298658.0, 1298654.0, > 1298654.0, 1298647.0, 1298652.0, 1298651.0, 1298652.0, 1298656.0, 1298649.0, > 1298644.0, 1298655.0, 1298648.0, 1298650.0, 1298648.0, 1298652.0, 1298651.0, > 1298649.0, 1298647.0, 1298661.0, 1298641.0, 1298652.0, 1298645.0, 1298650.0, > 1298652.0, 1298652.0, 1298651.0, 1298650.0, 1298650.0, 1298655.0, 1298663.0, > 1298655.0, 1298646.0, 1298648.0, 1298646.0, 1298652.0, 1298662.0, 1298651.0] > Mem max: 1298665.000 -> 1298663.000: 1.0000x smaller > Usage over time: http://goo.gl/aoTh9I > > Compilation of big-code.c at -O3 with -g for x86_64-unknown-linux-gnu: usr > control: [37.03, 36.97, 36.99, 37.04, 36.97, 36.94, 37.02, 36.97, > 37.05, 36.94, 36.92, 36.97, 37.03, 37.01, 37.04, 37.01, 36.99, 37.01, 37.07, > 37.05, 36.98, 36.98, 37.05, 36.97, 36.95, 37.01, 37.0, 37.0, 37.23, 37.07, > 36.78, 36.94, 37.04, 36.93, 36.98, 36.96, 36.91, 36.98, 37.05, 37.23] > experiment: [36.96, 36.98, 36.93, 36.97, 36.94, 37.27, 37.0, 36.96, 37.03, > 37.09, 36.85, 36.94, 36.99, 37.01, 37.0, 36.99, 36.98, 37.1, 37.04, 37.07, > 36.93, 36.95, 36.96, 37.01, 36.97, 37.06, 36.95, 37.01, 36.98, 36.93, 37.14, > 37.0, 36.92, 36.89, 36.98, 37.24, 36.97, 36.98, 36.96, 36.95] > Min: 36.780000 -> 36.850000: 1.00x slower > Avg: 37.001500 -> 36.997000: 1.00x faster > Not significant > Stddev: 0.07543 -> 0.08178: 1.0842x larger > Timeline: http://goo.gl/591YUR > > Compilation of big-code.c at -O3 with -g for x86_64-unknown-linux-gnu: sys > control: [1.29, 1.33, 1.32, 1.29, 1.32, 1.35, 1.3, 1.36, 1.25, 1.33, > 1.33, 1.36, 1.28, 1.3, 1.28, 1.27, 1.29, 1.25, 1.27, 1.25, 1.29, 1.31, 1.33, > 1.3, 1.3, 1.29, 1.28, 1.34, 1.3, 1.27, 1.34, 1.3, 1.28, 1.3, 1.3, 1.38, 1.37, > 1.34, 1.24, 1.3] > experiment: [1.29, 1.29, 1.3, 1.27, 1.31, 1.3, 1.3, 1.32, 1.25, 1.32, > 1.34, 1.33, 1.28, 1.25, 1.3, 1.27, 1.27, 1.32, 1.27, 1.25, 1.34, 1.32, 1.33, > 1.3, 1.29, 1.3, 1.28, 1.29, 1.3, 1.31, 1.23, 1.31, 1.3, 1.33, 1.34, 1.31, > 1.32, 1.31, 1.3, 1.3] > Min: 1.240000 -> 1.230000: 1.01x faster > Avg: 1.304500 -> 1.298500: 1.00x faster > Not significant > Stddev: 0.03412 -> 0.02637: 1.2939x smaller > Timeline: http://goo.gl/UA1Bvh > > Compilation of big-code.c at -O3 with -g for x86_64-unknown-linux-gnu: wall > control: [38.43, 38.42, 38.43, 38.45, 38.41, 38.4, 38.43, 38.44, 38.41, > 38.37, 38.37, 38.45, 38.42, 38.43, 38.43, 38.4, 38.39, 38.38, 38.45, 38.41, > 38.38, 38.4, 38.49, 38.39, 38.36, 38.41, 38.39, 38.46, 38.65, 38.46, 38.23, > 38.36, 38.44, 38.35, 38.39, 38.45, 38.39, 38.43, 38.41, 38.65] > experiment: [38.36, 38.38, 38.34, 38.35, 38.36, 38.68, 38.41, 38.39, > 38.38, 38.52, 38.3, 38.38, 38.38, 38.38, 38.42, 38.37, 38.36, 38.53, 38.42, > 38.44, 38.37, 38.38, 38.4, 38.42, 38.38, 38.48, 38.34, 38.41, 38.4, 38.36, > 38.49, 38.42, 38.33, 38.32, 38.43, 38.66, 38.41, 38.39, 38.37, 38.37] > Min: 38.230000 -> 38.300000: 1.00x slower > Avg: 38.420250 -> 38.407000: 1.00x faster > Not significant > Stddev: 0.06822 -> 0.07816: 1.1457x larger > Timeline: http://goo.gl/clfhdm > > Compilation of big-code.c at -O3 with -g for x86_64-unknown-linux-gnu: ggc > control: [662310.0, 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, > 662310.0, 662310.0, 662310.0, 662310.0, 661798.0, 661798.0, 662310.0, > 662310.0, 661798.0, 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, > 662310.0, 662310.0, 662310.0, 662310.0, 661798.0, 662310.0, 662310.0, > 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, 661798.0, 662310.0, > 662310.0, 662310.0, 661798.0, 661798.0, 662310.0, 662310.0] > experiment: [662310.0, 662310.0, 661798.0, 662310.0, 662310.0, 662310.0, > 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, > 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, > 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, > 662310.0, 661798.0, 662310.0, 662310.0, 662310.0, 661798.0, 662310.0, > 662310.0, 662310.0, 662310.0, 662310.0, 662310.0, 662310.0] > Mem max: 662310.000 -> 662310.000: no change > Usage over time: http://goo.gl/y77aRG > > *********************** > Sizes of built binaries > *********************** > With --enable-checking=release, and stripping the resulting binaries: > $ for binary in cc1 cc1obj cc1plus f951 jc1 ; do ls -al > test/*/*/build/gcc/$binary ; done > -rwxrwxr-x. 1 david david 15842584 Apr 17 14:27 > test/control/x86_64-unknown-linux-gnu/build/gcc/cc1 > -rwxrwxr-x. 1 david david 15838488 Apr 17 14:27 > test/experiment/x86_64-unknown-linux-gnu/build/gcc/cc1 > -rwxrwxr-x. 1 david david 16039192 Apr 17 14:27 > test/control/x86_64-unknown-linux-gnu/build/gcc/cc1obj > -rwxrwxr-x. 1 david david 16035096 Apr 17 14:27 > test/experiment/x86_64-unknown-linux-gnu/build/gcc/cc1obj > -rwxrwxr-x. 1 david david 17014104 Apr 17 14:27 > test/control/x86_64-unknown-linux-gnu/build/gcc/cc1plus > -rwxrwxr-x. 1 david david 17014104 Apr 17 14:27 > test/experiment/x86_64-unknown-linux-gnu/build/gcc/cc1plus > -rwxrwxr-x. 1 david david 16576600 Apr 17 14:27 > test/control/x86_64-unknown-linux-gnu/build/gcc/f951 > -rwxrwxr-x. 1 david david 16572504 Apr 17 14:27 > test/experiment/x86_64-unknown-linux-gnu/build/gcc/f951 > -rwxrwxr-x. 1 david david 15159256 Apr 17 14:27 > test/control/x86_64-unknown-linux-gnu/build/gcc/jc1 > -rwxrwxr-x. 1 david david 15159256 Apr 17 14:27 > test/experiment/x86_64-unknown-linux-gnu/build/gcc/jc1 > > i.e. the sizes are either unchanged, or got very slightly smaller. > > **** > Plan > **** > I'd like to commit this patch series to trunk for 4.10. I hope the > overall plan sounds good. May I have reviews of the individual patches? > (each patch builds on the ones before, but at any given patch in the > series they should all compile cleanly together and not affect > correctness/performance). > > I have a script that tracks how many of the gimple statement accessors > take something more concrete than a plain gimple: > > https://github.com/davidmalcolm/gcc-refactoring-scripts/blob/master/gimple_typesafety.py > which reports that the following statement subclasses are "done" in the > sense that all of their accessors now accept a subclass pointer, rather > than a plain gimple/const_gimple: > > gimple_asm_ (19 accessors) > gimple_bind_ (10 accessors) > gimple_catch_ (6 accessors) > gimple_eh_dispatch_ (2 accessors) > gimple_eh_else_ (6 accessors) > gimple_eh_must_not_throw_ (2 accessors) > gimple_label_ (2 accessors) > gimple_omp_atomic_load_ (6 accessors) > gimple_omp_atomic_store_ (3 accessors) > gimple_omp_continue_ (6 accessors) > gimple_omp_critical_ (3 accessors) > gimple_resx_ (2 accessors) > gimple_return_ (3 accessors) > gimple_switch_ (9 accessors) > gimple_transaction_ (8 accessors) > > and the following DONE/TODO breakdown by prefix: > > Accessors "concretized" by prefix > --------------------------------- > % Prefix DONE TODO > 100 gimple_transaction_ 8 0 > 100 gimple_switch_ 9 0 > 100 gimple_return_ 3 0 > 100 gimple_resx_ 2 0 > 100 gimple_omp_critical_ 3 0 > 100 gimple_omp_continue_ 6 0 > 100 gimple_omp_atomic_store_ 3 0 > 100 gimple_omp_atomic_load_ 6 0 > 100 gimple_label_ 2 0 > 100 gimple_eh_must_not_throw_ 2 0 > 100 gimple_eh_else_ 6 0 > 100 gimple_eh_dispatch_ 2 0 > 100 gimple_catch_ 6 0 > 100 gimple_bind_ 10 0 > 100 gimple_asm_ 19 0 > 83 gimple_cond_ 15 3 > 72 gimple_omp_target_ 8 3 > 72 gimple_omp_parallel_ 8 3 > 58 gimple_call_ 25 18 > 57 gimple_phi_ 8 6 > 50 gimple_goto_ 1 1 > 40 gimple_try_ 4 6 > 33 gimple_omp_teams_ 1 2 > 33 gimple_omp_single_ 1 2 > 33 gimple_eh_filter_ 2 4 > 11 gimple_omp_for_ 3 24 > 5 gimple_assign_ 1 17 > 0 gimple_wce_ 0 5 > 0 gimple_predict_ 0 4 > 0 gimple_omp_task_ 0 18 > 0 gimple_omp_sections_ 0 6 > 0 gimple_omp_section_ 0 2 > 0 gimple_omp_return_ 0 5 > 0 gimple_omp_ 0 18 > 0 gimple_debug_ 0 12 > > Overall totals > -------------- > TODO: Builder calls still returning a plain gimple: 9 > TODO: Accessors not yet converted to taking a gimple: 159 > DONE: Builder calls converted to returning a gimple subclass: 37 > DONE: Accessors converted to taking a gimple subclass: 167 > NOTE: Accessors known not to need to be converted: 67 > > So this patch series as-is takes us a little over halfway there. The > remaining accessors would involve more substantial re-indenting (typically > to introduce blocks within case statements of a switch on statement code, > so I can introduce a type-checked local). I didn't want to go down that > path until the overall approach was OKed, since those kinds of changes are > much more prone to generating conflicts as I rebase (and hence to bitrot). > > Thoughts? > > Dave > > David Malcolm (89): > 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 callbacks 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 > Update various expressions within tree-scalar-evolution.c to be > gimple_phi > Concretize get_loop_exit_condition et al to working on gimple_cond > Const-correctness of gimple_call_builtin_p > Introduce gimple_call > Introduce gimple_return > Introduce gimple_goto > Introduce gimple_asm > Introduce gimple_transaction > Introduce gimple_catch > Introduce gimple_eh_filter > Introduce gimple_eh_must_not_throw > Introduce gimple_eh_else > Introduce gimple_resx > Introduce gimple_eh_dispatch > Use subclasses of gimple in various places > Introduce gimple_try > Use more concrete types for various gimple statements > Introduce gimple_omp_atomic_load > Introduce gimple_omp_atomic_store > Introduce gimple_omp_continue > Introduce gimple_omp_critical > Introduce gimple_omp_for > Introduce gimple_omp_parallel > tree-cfg.c: Make verify_gimple_call require a gimple_call > Introduce gimple_omp_task > Introduce gimple_omp_single > Introduce gimple_omp_target > Introduce gimple_omp_teams > Introduce gimple_omp_sections > tree-parloops.c: Use gimple_phi in various places > omp-low.c: Use more concrete types of gimple statement for various > locals > Make gimple_phi_arg_def_ptr and gimple_phi_arg_has_location require a > gimple_phi > Make add_phi_arg require a gimple_phi > Make gimple_phi_arg_set_location require a gimple_phi > Update GRAPHITE to use more concrete gimple statement classes > Make gimple_phi_arg_edge require a gimple_phi > More gimple_phi > Make gimple_call_return_slot_opt_p require a gimple_call. > Use gimple_call for callgraph edges > Various gimple to gimple_call conversions in IPA > Concretize parameter to gimple_call_copy_skip_args > Make gimple_label_set_label require a gimple_label > Make gimple_goto_set_dest require a gimple_goto > Concretize gimple_catch_types > Concretize gimple_call_use_set and gimple_call_clobber_set > Concretize gimple_label_label > Concretize gimple_eh_filter_set_types and gimple_eh_filter_set_failure > Concretize gimple_try_set_catch_is_cleanup > Concretize three gimple_try_set_ accessors > Make gimple_phi_arg_location_from_edge require a gimple_phi > Make gimple_phi_arg_location require a gimple_phi. > Concretize three gimple_return_ accessors > Make gimple_cond_set_{true|false}_label require gimple_cond. > Concretize locals within expand_omp_for_init_counts > Concretize gimple_cond_make_{false|true} > Concretize gimple_switch_index and gimple_switch_index_ptr > Concretize gimple_cond_{true|false}_label > Concretize gimple_cond_set_code > Concretize gimple_cond_set_{lhs|rhs} > Concretize gimple_cond_{lhs|rhs}_ptr > Concretize various expressions from gimple to gimple_cond > Concretize gimple_call_set_nothrow > Concretize gimple_call_nothrow_p > Tweak to gimplify_modify_expr > Concretize gimple_call_set_fn > Concretize gimple_call_set_fntype > Concretize gimple_call_set_tail and gimple_call_tail_p > Concretize gimple_call_arg_flags > Concretize gimple_assign_nontemporal_move_p > Concretize gimple_call_copy_flags and ipa_modify_call_arguments > Use gimple_call in some places within tree-ssa-dom.c > Use gimple_phi in many more places. > Convert various gimple to gimple_phi within ssa-iterators.h > > gcc/asan.c | 23 +- > gcc/builtins.c | 11 +- > gcc/builtins.h | 2 +- > gcc/c-family/c-gimplify.c | 4 +- > gcc/cfgexpand.c | 58 +- > gcc/cfgloop.c | 6 +- > gcc/cfgloopmanip.c | 4 +- > gcc/cgraph.c | 22 +- > gcc/cgraph.h | 17 +- > gcc/cgraphbuild.c | 41 +- > gcc/cgraphclones.c | 6 +- > gcc/cgraphunit.c | 6 +- > gcc/coretypes.h | 126 ++++ > gcc/expr.h | 2 +- > gcc/gdbhooks.py | 19 +- > gcc/gimple-builder.c | 16 +- > gcc/gimple-builder.h | 16 +- > gcc/gimple-fold.c | 25 +- > gcc/gimple-fold.h | 2 +- > gcc/gimple-iterator.c | 12 +- > gcc/gimple-iterator.h | 11 +- > gcc/gimple-low.c | 39 +- > gcc/gimple-pretty-print.c | 183 +++-- > gcc/gimple-ssa-isolate-paths.c | 6 +- > gcc/gimple-ssa-strength-reduction.c | 30 +- > gcc/gimple-streamer-in.c | 34 +- > gcc/gimple-streamer-out.c | 46 +- > gcc/gimple-walk.c | 169 +++-- > gcc/gimple.c | 347 +++++---- > gcc/gimple.h | 1379 > +++++++++++++++++++++-------------- > gcc/gimplify-me.c | 29 +- > gcc/gimplify.c | 125 ++-- > gcc/gimplify.h | 6 +- > gcc/graphite-scop-detection.c | 22 +- > gcc/graphite-sese-to-poly.c | 141 ++-- > gcc/internal-fn.c | 42 +- > gcc/internal-fn.def | 2 +- > gcc/internal-fn.h | 2 +- > gcc/ipa-inline-analysis.c | 18 +- > gcc/ipa-prop.c | 37 +- > gcc/ipa-prop.h | 4 +- > gcc/ipa-pure-const.c | 10 +- > gcc/ipa-split.c | 95 +-- > gcc/java/java-gimplify.c | 2 +- > gcc/lto-streamer-in.c | 4 +- > gcc/lto-streamer-out.c | 15 +- > gcc/omp-low.c | 441 ++++++----- > gcc/predict.c | 56 +- > gcc/sese.c | 13 +- > gcc/ssa-iterators.h | 20 +- > gcc/stmt.c | 4 +- > gcc/trans-mem.c | 132 ++-- > gcc/tree-call-cdce.c | 29 +- > gcc/tree-cfg.c | 407 ++++++----- > gcc/tree-cfg.h | 4 +- > gcc/tree-cfgcleanup.c | 44 +- > gcc/tree-complex.c | 39 +- > gcc/tree-data-ref.c | 3 +- > gcc/tree-dfa.c | 10 +- > gcc/tree-eh.c | 274 ++++--- > gcc/tree-eh.h | 6 +- > gcc/tree-emutls.c | 13 +- > gcc/tree-if-conv.c | 15 +- > gcc/tree-inline.c | 185 +++-- > gcc/tree-inline.h | 2 +- > gcc/tree-into-ssa.c | 59 +- > gcc/tree-into-ssa.h | 2 +- > gcc/tree-loop-distribution.c | 55 +- > gcc/tree-nested.c | 45 +- > gcc/tree-nrv.c | 11 +- > gcc/tree-object-size.c | 20 +- > gcc/tree-outof-ssa.c | 23 +- > gcc/tree-parloops.c | 101 +-- > gcc/tree-phinodes.c | 41 +- > gcc/tree-phinodes.h | 8 +- > gcc/tree-predcom.c | 30 +- > gcc/tree-profile.c | 19 +- > gcc/tree-scalar-evolution.c | 73 +- > gcc/tree-scalar-evolution.h | 2 +- > gcc/tree-sra.c | 125 ++-- > gcc/tree-ssa-alias.c | 16 +- > gcc/tree-ssa-alias.h | 2 +- > gcc/tree-ssa-ccp.c | 32 +- > gcc/tree-ssa-coalesce.c | 29 +- > gcc/tree-ssa-copy.c | 11 +- > gcc/tree-ssa-copyrename.c | 12 +- > gcc/tree-ssa-dce.c | 37 +- > gcc/tree-ssa-dom.c | 76 +- > gcc/tree-ssa-forwprop.c | 21 +- > gcc/tree-ssa-ifcombine.c | 27 +- > gcc/tree-ssa-live.c | 16 +- > gcc/tree-ssa-loop-im.c | 65 +- > gcc/tree-ssa-loop-ivcanon.c | 41 +- > gcc/tree-ssa-loop-ivopts.c | 44 +- > gcc/tree-ssa-loop-manip.c | 64 +- > gcc/tree-ssa-loop-niter.c | 25 +- > gcc/tree-ssa-loop-prefetch.c | 4 +- > gcc/tree-ssa-loop-unswitch.c | 19 +- > gcc/tree-ssa-math-opts.c | 44 +- > gcc/tree-ssa-operands.c | 8 +- > gcc/tree-ssa-phiopt.c | 38 +- > gcc/tree-ssa-phiprop.c | 12 +- > gcc/tree-ssa-pre.c | 68 +- > gcc/tree-ssa-propagate.c | 30 +- > gcc/tree-ssa-propagate.h | 2 +- > gcc/tree-ssa-reassoc.c | 39 +- > gcc/tree-ssa-sccvn.c | 25 +- > gcc/tree-ssa-sccvn.h | 2 +- > gcc/tree-ssa-sink.c | 8 +- > gcc/tree-ssa-strlen.c | 13 +- > gcc/tree-ssa-structalias.c | 85 ++- > gcc/tree-ssa-tail-merge.c | 33 +- > gcc/tree-ssa-ter.c | 4 +- > gcc/tree-ssa-threadedge.c | 19 +- > gcc/tree-ssa-threadedge.h | 2 +- > gcc/tree-ssa-threadupdate.c | 16 +- > gcc/tree-ssa-uncprop.c | 7 +- > gcc/tree-ssa-uninit.c | 57 +- > gcc/tree-ssa.c | 56 +- > gcc/tree-stdarg.c | 19 +- > gcc/tree-switch-conversion.c | 57 +- > gcc/tree-tailcall.c | 46 +- > gcc/tree-vect-data-refs.c | 7 +- > gcc/tree-vect-generic.c | 13 +- > gcc/tree-vect-loop-manip.c | 74 +- > gcc/tree-vect-loop.c | 61 +- > gcc/tree-vect-patterns.c | 2 +- > gcc/tree-vect-slp.c | 15 +- > gcc/tree-vect-stmts.c | 22 +- > gcc/tree-vectorizer.h | 2 +- > gcc/tree-vrp.c | 100 +-- > gcc/tree.c | 5 +- > gcc/tree.h | 5 +- > gcc/ubsan.c | 5 +- > gcc/value-prof.c | 91 ++- > gcc/value-prof.h | 3 +- > gcc/vtable-verify.c | 2 +- > 137 files changed, 4095 insertions(+), 3070 deletions(-) > > -- > 1.8.5.3 >