From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22588 invoked by alias); 16 Oct 2014 22:06:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 22570 invoked by uid 89); 16 Oct 2014 22:06:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 16 Oct 2014 22:06:49 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9GM6m5g014692 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 16 Oct 2014 18:06:48 -0400 Received: from stumpy.slc.redhat.com (ovpn-113-110.phx2.redhat.com [10.3.113.110]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s9GM6kVu014472; Thu, 16 Oct 2014 18:06:47 -0400 Message-ID: <54404176.6030309@redhat.com> Date: Thu, 16 Oct 2014 22:08:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: David Malcolm , jit@gcc.gnu.org, gcc-patches@gcc.gnu.org, Ilya Enkovich Subject: Re: [PATCH 4/5] State cleanups -- also note for MPX work References: <1413222308-25753-1-git-send-email-dmalcolm@redhat.com> <1413222308-25753-5-git-send-email-dmalcolm@redhat.com> In-Reply-To: <1413222308-25753-5-git-send-email-dmalcolm@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-10/txt/msg01633.txt.bz2 On 10/13/14 11:45, David Malcolm wrote: > 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 > variables. > > * 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. > > gcc/ChangeLog: > * 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 > to file-scope. > (first_analyzed_var): Likewise. > (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. > (node_duplication_hook_holder): Likewise. > (node_removal_hook_holder): Likewise. > (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. > (pure_const_read_summary): Likewise. > (propagate): Convert to... > (pass_ipa_pure_const::execute): ...method. > * ipa-reference.c (ipa_init): Move static bool init_p from here > to... > (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 > false. > (params_c_finalize): New. > * 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 > write_optimization_summary hook. > (ipa_read_summaries_1): Likewise for read_summary hook. > (ipa_read_optimization_summaries_1): Likewise for > read_optimization_summary hook. > (execute_ipa_stmt_fixups): Likewise. > * stringpool.c (init_stringpool): Clean up if we're called more > than once. > * 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::start_timevars. > (toplev::toplev, toplev::~toplev, toplev::start_timevars, > toplev::finalize): New functions. > (toplev_main): Rename to... > (toplev::main): ...this. > * 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(-) 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 progress. 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. > > diff --git a/gcc/cgraph.h b/gcc/cgraph.h > index 20b5c4e..794403d 100644 > --- a/gcc/cgraph.h > +++ b/gcc/cgraph.h > @@ -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); Combine this with the declaration of init_lowered_empty_function so that the references to cgraphunit's functions are together. > + > /* 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 > --- a/gcc/dwarf2out.c > +++ b/gcc/dwarf2out.c > @@ -24614,4 +24614,91 @@ dwarf2out_finish (const char *filename) > output_indirect_strings (); > } > > +/* Reset all state within dwarf2out.c so that we can rerun the compiler > + within the same process. For use by toplev::finalize. */ > + > +void > +dwarf2out_c_finalize (void) > +{ > + 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. > diff --git a/gcc/passes.c b/gcc/passes.c > index 5001c3d..772993d 100644 > --- a/gcc/passes.c > +++ b/gcc/passes.c > @@ -1944,6 +1944,7 @@ execute_ipa_summary_passes (ipa_opt_pass_d *ipa_pass) > if (pass->tv_id) > timevar_push (pass->tv_id); > > + current_pass = pass; > ipa_pass->generate_summary (); Presumably it doesn't make sense to push this down into ipa_pass->generate_summary? OK with the nits above addressed. Jeff