This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH 4/5] State cleanups -- also note for MPX work
- From: Jeff Law <law at redhat dot com>
- To: David Malcolm <dmalcolm at redhat dot com>, jit at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org, Ilya Enkovich <enkovich dot gnu at gmail dot com>
- Date: Thu, 16 Oct 2014 16:06:46 -0600
- Subject: Re: [PATCH 4/5] State cleanups -- also note for MPX work
- Authentication-results: sourceware.org; auth=none
- References: <1413222308-25753-1-git-send-email-dmalcolm at redhat dot com> <1413222308-25753-5-git-send-email-dmalcolm at redhat dot com>
On 10/13/14 11:45, David Malcolm wrote:
General note, I suspect there's going to be file scoped statics in some
of the MPX work. It was hard to say "you have to do this a different
way" and make it a precondition for acceptance with your patch still in
This patch contains various cleanups needed by the jit, so that we can
rerun the compiler code repeatedly within the same process:
* Introduce per-file finalization routines to clean up file-static
* Make various initializations idempotent (init_ggc), or cleanup their
previous state (init_stringpool).
* Introduce a "class toplev", so that we can easy control whether
timevars are reset each time, or run in a "cumulative" mode.
* cgraph.c (cgraph_c_finalize): New function.
* cgraph.h (cgraph_c_finalize): New prototype.
(cgraphunit_c_finalize): New prototype.
(ipa_cp_c_finalize): New prototype.
* cgraphunit.c (first_analyzed): Move from analyze_functions
(analyze_functions): Move static variables into file-scope.
(cgraphunit_c_finalize): New function.
* diagnostic.c (diagnostic_finish): Free the memory for
context->classify_diagnostic and context->printer, running the
destructor for the latter.
(bt_stop): Use toplev::main.
* dwarf2out.c (dwarf2out_finalize): New function.
* dwarf2out.h (dwarf2out_c_finalize): New prototype.
* gcse.c (gcse_c_finalize): New function.
* gcse.h (gcse_c_finalize): New prototype.
* ggc-page.c (init_ggc): Make idempotent.
* input.c (input_location): Initialize to UNKNOWN_LOCATION.
* ipa-cp.c (ipa_cp_c_finalize): New function.
* ipa-pure-const.c (function_insertion_hook_holder): Move to be
a field of class pass_ipa_pure_const.
(register_hooks): Convert to method...
(pass_ipa_pure_const::register_hooks): ...here, converting
static variable init_p into...
(pass_ipa_pure_const::init_p): ...new field.
(pure_const_generate_summary): Update invocation of
register_hooks to invoke as a method of current_pass.
(propagate): Convert to...
* ipa-reference.c (ipa_init): Move static bool init_p from here
(ipa_init_p): New file-scope variable, so that it can be reset
when repeatedly invoking the compiler within one process by...
(ipa_reference_c_finalize): New function.
* ipa-reference.h (ipa_reference_c_finalize): New.
* main.c (main): Replace invocation of toplev_main with
construction of a toplev instance, and call its "main" method.
* params.c (global_init_params): Add an assert that params_finished is
* params.h (params_c_finalize): New.
* passes.c (execute_ipa_summary_passes): Set "current_pass" before
invoking generate_summary, for the benefit of pass_ipa_pure_const.
(ipa_write_summaries_2): Assign "pass" to "current_pass" global
before calling write_summary hook.
(ipa_write_optimization_summaries_1): Likewise when calling
(ipa_read_summaries_1): Likewise for read_summary hook.
(ipa_read_optimization_summaries_1): Likewise for
* stringpool.c (init_stringpool): Clean up if we're called more
* timevar.c (timevar_init): Ignore repeated calls.
* toplev.c: Include "dwarf2out.h", "ipa-reference.h", "gcse.h".
(general_init): Reset "input_location" to UNKNOWN_LOCATION.
(initialize_rtl): Move static local "initialized_once"
into file scope, and rename to...
(rtl_initialized): New variable.
(do_compile): Move timevar initialization from here to
(toplev::toplev, toplev::~toplev, toplev::start_timevars,
toplev::finalize): New functions.
(toplev_main): Rename to...
* toplev.h (class toplev): New class.
gcc/cgraph.c | 14 +++++++
gcc/cgraph.h | 6 +++
gcc/cgraphunit.c | 20 ++++++++-
gcc/diagnostic.c | 11 ++++-
gcc/dwarf2out.c | 87 +++++++++++++++++++++++++++++++++++++++
gcc/dwarf2out.h | 2 +
gcc/gcse.c | 9 ++++
gcc/gcse.h | 2 +
gcc/ggc-page.c | 5 +++
gcc/input.c | 2 +-
gcc/ipa-cp.c | 12 ++++++
gcc/ipa-pure-const.c | 113 +++++++++++++++++++++++++++++----------------------
gcc/ipa-reference.c | 17 ++++++--
gcc/ipa-reference.h | 1 +
gcc/main.c | 6 ++-
gcc/params.c | 14 +++++++
gcc/params.h | 4 ++
gcc/passes.c | 6 +++
gcc/stringpool.c | 5 +++
gcc/timevar.c | 3 ++
gcc/toplev.c | 67 +++++++++++++++++++++++-------
gcc/toplev.h | 19 ++++++++-
22 files changed, 351 insertions(+), 74 deletions(-)
So that means if the MPX work goes in first, you'll need to adjust this
patch. If it happens the other way, the MPX work will need adjustment.
Anyway, just thought it was worth explicitly pointing out that these two
hunks of work, while they're tackling totally different issues may
conflict because of an implementation of the MPX bits.
Combine this with the declaration of init_lowered_empty_function so that
the references to cgraphunit's functions are together.
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 20b5c4e..794403d 100644
@@ -2042,6 +2043,8 @@ bool resolution_used_from_other_file_p (enum ld_plugin_symbol_resolution);
extern bool gimple_check_call_matching_types (gimple, tree, bool);
/* In cgraphunit.c */
+void cgraphunit_c_finalize (void);
/* Initialize datastructures so DECL is a function in lowered gimple form.
IN_SSA is true if the gimple is in SSA. */
basic_block init_lowered_empty_function (tree, bool);
@@ -2061,6 +2064,9 @@ void record_references_in_initializer (tree, bool);
void cgraph_build_static_cdtor (char which, tree body, int priority);
void ipa_discover_readonly_nonaddressable_vars (void);
+/* In ipa-cp.c */
+void ipa_cp_c_finalize (void);
Is there a better place for this? ipa-prop.h?
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 59c05ed..a085e0a 100644
@@ -24614,4 +24614,91 @@ dwarf2out_finish (const char *filename)
+/* Reset all state within dwarf2out.c so that we can rerun the compiler
+ within the same process. For use by toplev::finalize. */
+ last_var_location_insn = NULL;
[ ... ]
Makes me wonder if some of this stuff belongs in a structure. And if
some does, then obviously memset becomes an option for finalization.
But that's not something I think you need to own for this to go forward.
Presumably it doesn't make sense to push this down into
diff --git a/gcc/passes.c b/gcc/passes.c
index 5001c3d..772993d 100644
@@ -1944,6 +1944,7 @@ execute_ipa_summary_passes (ipa_opt_pass_d *ipa_pass)
+ current_pass = pass;
OK with the nits above addressed.