I want to eliminate/minimize global state within gcc, since I think doing so is a key part of making gcc more modular.
Currently there's a lot of global state associated with passes: * the pass tree itself: a single global tree of passes, with callbacks ("gate" and "execute") * globals within individual pass .c files My plan is to instead have the passes be dynamically created instances of pass subclasses, where each pass has a custom subclass of the appropriate pass class: So, for example, mudflap would become something like: class pass_mudflap_2 : public gimple_opt_pass { public: bool gate () { return gate_mudflap(); } unsigned int execute () { return execute_mudflap_function_ops(); } }; where these subclasses are hidden inside the .c files (in this case inside gcc/tree-mudflap.c). All that's exposed to the headers would then be a factory function: gimple_opt_pass * make_pass_mudflap_2 (context &ctxt) { return new pass_mudflap_2 (ctxt); } Globals within a .c file that are specific to a particular pass may then be movable to instance data of the pass subclass (on a case-by-case basis). Each pass also has a reference back to a new "context" class, shared by all the passes: this is currently empty, but I see it being useful as a place to add any global state that isn't part of the pass itself: given that this patch is touching hundreds of files I'd rather add it now, rather than having to go back and add it later. I started doing this by hand, however, there are hundreds of passes, so to avoid lots of error-prone typing I've written a refactoring script. You can see the test suite for the refactoring script here: https://github.com/davidmalcolm/gcc-refactoring-scripts/blob/master/test_refactor.py which should give a better idea of the before/after when applying the refactoring (and the use of a test suite hopefully increases the credibility that the resulting patch doesn't change the effective behavior of the code). When run, the refactoring script generates this: 117 files changed, 7668 insertions(+), 4336 deletions(-) There's also a new "pipeline" class representing the pass hierarchy, which stores the pointers to the pass instances, so that it's easy to access them from gdb when debugging. One wart in my plan is that a lot of the existing callbacks are checked against NULL and if non-NULL, then extra things happen before/after the call. I don't know of a portable way to do this for a C++ virtual function, so each callback becomes *two* vtable entries: bool has_FOO () // equivalent to (pass->FOO != NULL) in old code and impl_FOO () // equivalent to (pass->FOO ()) in old code I'm currently working off of this git repo: git://gcc.gnu.org/git/gcc.git in the hope of getting this into 4.9 during stage 1. This is my first patch to GCC. I'm aware of the following remaining issues with this: * I haven't yet integrated "context" or "pipeline" with the garbage collector * pass instances are currently allocated using "new" * there's no error checking for detecting if allocating a pass fails (e.g. under very low memory conditions at startup) * I'm using C++ references for things that can't legally be NULL. Is this OK? I don't see any mention of them in http://gcc.gnu.org/codingconventions.html#Cxx_Language but given that they've been in the language for decades and are "just a pointer" internally, I assumed that they're OK. * Should I be manually updating dependencies anywhere? (this patch sequence adds three new source file - two .h and a .def) Having said that, I've successfully done the 3-stage bootstrap (against gcc-4.7.2-2.fc17.x86_64 on this Fedora 17 x86_64 box), and the great majority of tests pass: $ for f in $(find gcc/testsuite -name "*.log") ; do \ printf "%s: \n" $f; \ printf " PASS: " ; grep -e "^PASS" $f | wc -l ; \ printf " FAIL: " ; grep -e "^FAIL" $f | wc -l ; \ done gcc/testsuite/g++/g++.log: PASS: 54447 FAIL: 50 gcc/testsuite/gfortran/gfortran.log: PASS: 44038 FAIL: 13 gcc/testsuite/objc/objc.log: PASS: 2982 FAIL: 0 gcc/testsuite/gcc/gcc.log: PASS: 96473 FAIL: 629 so I thought it was worth posting for more feedback. The change is split into the following 6 patches: Introduce macros when constructing the tree of passes Move the construction of the pass hierarchy into a new passes.def file Autogenerated part of conversion of passes to instances of C++ classes Handwritten part of conversion of passes to instances of C++ classes Introduce virtual functions in testsuite/gcc.dg/plugin/one_time_plugin.c Example of converting global state to per-pass state gcc/ChangeLog | 1578 +++++++++++++++++++++++++ gcc/asan.c | 94 +- gcc/auto-inc-dec.c | 46 +- gcc/bb-reorder.c | 140 ++- gcc/bt-load.c | 96 +- gcc/cfgcleanup.c | 92 +- gcc/cfgexpand.c | 49 +- gcc/cfgrtl.c | 138 ++- gcc/cgraphbuild.c | 138 ++- gcc/cgraphunit.c | 32 +- gcc/combine-stack-adj.c | 48 +- gcc/combine.c | 46 +- gcc/compare-elim.c | 50 +- gcc/config/epiphany/epiphany.h | 4 +- gcc/config/epiphany/mode-switch-use.c | 46 +- gcc/config/epiphany/resolve-sw-modes.c | 47 +- gcc/config/i386/i386.c | 59 +- gcc/config/sparc/sparc.c | 48 +- gcc/context.h | 35 + gcc/cprop.c | 47 +- gcc/cse.c | 143 ++- gcc/dce.c | 92 +- gcc/df-core.c | 142 ++- gcc/dse.c | 96 +- gcc/dwarf2cfi.c | 48 +- gcc/except.c | 96 +- gcc/final.c | 186 +-- gcc/function.c | 197 +-- gcc/fwprop.c | 94 +- gcc/gcse.c | 94 +- gcc/gimple-low.c | 46 +- gcc/gimple-ssa-strength-reduction.c | 48 +- gcc/ifcvt.c | 143 ++- gcc/init-regs.c | 48 +- gcc/ipa-cp.c | 90 +- gcc/ipa-inline-analysis.c | 48 +- gcc/ipa-inline.c | 130 +- gcc/ipa-pure-const.c | 127 +- gcc/ipa-reference.c | 85 +- gcc/ipa-split.c | 94 +- gcc/ipa.c | 341 ++++-- gcc/ira.c | 92 +- gcc/jump.c | 46 +- gcc/loop-init.c | 324 +++-- gcc/lower-subreg.c | 93 +- gcc/lto-cgraph.c | 9 +- gcc/lto-streamer-out.c | 162 ++- gcc/lto/lto.c | 6 +- gcc/mode-switching.c | 47 +- gcc/modulo-sched.c | 50 +- gcc/omp-low.c | 138 ++- gcc/passes.c | 899 +++++--------- gcc/passes.def | 405 +++++++ gcc/pipeline.h | 71 ++ gcc/postreload-gcse.c | 47 +- gcc/postreload.c | 47 +- gcc/predict.c | 98 +- gcc/recog.c | 297 +++-- gcc/ree.c | 47 +- gcc/reg-stack.c | 92 +- gcc/regcprop.c | 47 +- gcc/reginfo.c | 46 +- gcc/regmove.c | 46 +- gcc/regrename.c | 47 +- gcc/reorg.c | 92 +- gcc/sched-rgn.c | 98 +- gcc/stack-ptr-mod.c | 46 +- gcc/statistics.c | 8 +- gcc/store-motion.c | 47 +- gcc/testsuite/g++.dg/plugin/dumb_plugin.c | 49 +- gcc/testsuite/g++.dg/plugin/selfassign.c | 49 +- gcc/testsuite/gcc.dg/plugin/one_time_plugin.c | 58 +- gcc/testsuite/gcc.dg/plugin/selfassign.c | 49 +- gcc/toplev.c | 4 + gcc/tracer.c | 47 +- gcc/trans-mem.c | 336 +++--- gcc/tree-call-cdce.c | 46 +- gcc/tree-cfg.c | 242 ++-- gcc/tree-cfgcleanup.c | 46 +- gcc/tree-complex.c | 94 +- gcc/tree-eh.c | 231 ++-- gcc/tree-emutls.c | 46 +- gcc/tree-if-conv.c | 47 +- gcc/tree-into-ssa.c | 49 +- gcc/tree-loop-distribution.c | 46 +- gcc/tree-mudflap.c | 95 +- gcc/tree-nomudflap.c | 92 +- gcc/tree-nrv.c | 92 +- gcc/tree-object-size.c | 46 +- gcc/tree-optimize.c | 94 +- gcc/tree-pass.h | 707 ++++++----- gcc/tree-profile.c | 46 +- gcc/tree-sra.c | 140 ++- gcc/tree-ssa-ccp.c | 95 +- gcc/tree-ssa-copy.c | 48 +- gcc/tree-ssa-copyrename.c | 46 +- gcc/tree-ssa-dce.c | 143 ++- gcc/tree-ssa-dom.c | 98 +- gcc/tree-ssa-dse.c | 46 +- gcc/tree-ssa-forwprop.c | 49 +- gcc/tree-ssa-ifcombine.c | 47 +- gcc/tree-ssa-loop-ch.c | 48 +- gcc/tree-ssa-loop.c | 870 ++++++++------ gcc/tree-ssa-math-opts.c | 188 +-- gcc/tree-ssa-phiopt.c | 96 +- gcc/tree-ssa-phiprop.c | 47 +- gcc/tree-ssa-pre.c | 93 +- gcc/tree-ssa-reassoc.c | 48 +- gcc/tree-ssa-sink.c | 49 +- gcc/tree-ssa-strlen.c | 46 +- gcc/tree-ssa-structalias.c | 144 ++- gcc/tree-ssa-uncprop.c | 46 +- gcc/tree-ssa-uninit.c | 48 +- gcc/tree-ssa.c | 140 ++- gcc/tree-ssanames.c | 46 +- gcc/tree-stdarg.c | 46 +- gcc/tree-switch-conversion.c | 49 +- gcc/tree-tailcall.c | 92 +- gcc/tree-vect-generic.c | 100 +- gcc/tree-vectorizer.c | 96 +- gcc/tree-vrp.c | 51 +- gcc/tree.c | 48 +- gcc/tsan.c | 92 +- gcc/var-tracking.c | 49 +- gcc/web.c | 46 +- 125 files changed, 9450 insertions(+), 5119 deletions(-) create mode 100644 gcc/context.h create mode 100644 gcc/passes.def create mode 100644 gcc/pipeline.h -- 1.7.11.7