| Message ID | gkrlfu6uphd.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | IPA-CP release transformation summary (PR jit/91928) | expand |
Hi, On Mon, Sep 30 2019, Andrea Corallo wrote: > Hi all, > I'd like to submit this patch. > It release the ipa cp transformation summary after functions being expanded. > This is to fix the compiler when used with libgccjit on subsequent > compilations (every new compilation should have a clean transformation > summary). if this is a general problem then I think we should instead add another hook to class ipa_opt_pass_d to free transformation summary, call it for all IPA passes at the appropriate time and implement it for IPA-CP. That way it will work for all IPA passes which might have a transformation summary. Martin > > Bootstrap on arm64 and X86-64. > > Bests > Andrea > > gcc/ChangeLog > 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> > > * cgraphunit.c (expand_all_functions): Release ipcp_transformation_sum > when finished. > * ipa-prop.c (ipcp_free_transformation_sum): New function. > * ipa-prop.h (ipcp_free_transformation_sum): Add declaration.
Martin Jambor writes: > Hi, > > On Mon, Sep 30 2019, Andrea Corallo wrote: >> Hi all, >> I'd like to submit this patch. >> It release the ipa cp transformation summary after functions being expanded. >> This is to fix the compiler when used with libgccjit on subsequent >> compilations (every new compilation should have a clean transformation >> summary). > > if this is a general problem then I think we should instead add another > hook to class ipa_opt_pass_d to free transformation summary, call it for > all IPA passes at the appropriate time and implement it for IPA-CP. That > way it will work for all IPA passes which might have a transformation > summary. > > Martin > > >> >> Bootstrap on arm64 and X86-64. >> >> Bests >> Andrea >> >> gcc/ChangeLog >> 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> >> >> * cgraphunit.c (expand_all_functions): Release ipcp_transformation_sum >> when finished. >> * ipa-prop.c (ipcp_free_transformation_sum): New function. >> * ipa-prop.h (ipcp_free_transformation_sum): Add declaration. Hi, actually looking around in order to implement the suggestions I realized that already some code was put in place in toplev::finalize calling then ipa_cp_c_finalize exactly for this purpose. I've updated the patch accordingly. Bootstraped on aarch64. Is it okay for trunk? Bests Andrea gcc/ChangeLog 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> * ipa-cp.c (ipa_cp_c_finalize): Release ipcp_transformation_sum. * ipa-prop.c (ipcp_free_transformation_sum): New function. * ipa-prop.h (ipcp_free_transformation_sum): Add declaration.
On 10/1/19 4:11 AM, Andrea Corallo wrote: > Martin Jambor writes: > >> Hi, >> >> On Mon, Sep 30 2019, Andrea Corallo wrote: >>> Hi all, >>> I'd like to submit this patch. >>> It release the ipa cp transformation summary after functions being expanded. >>> This is to fix the compiler when used with libgccjit on subsequent >>> compilations (every new compilation should have a clean transformation >>> summary). >> if this is a general problem then I think we should instead add another >> hook to class ipa_opt_pass_d to free transformation summary, call it for >> all IPA passes at the appropriate time and implement it for IPA-CP. That >> way it will work for all IPA passes which might have a transformation >> summary. >> >> Martin >> >> >>> Bootstrap on arm64 and X86-64. >>> >>> Bests >>> Andrea >>> >>> gcc/ChangeLog >>> 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> >>> >>> * cgraphunit.c (expand_all_functions): Release ipcp_transformation_sum >>> when finished. >>> * ipa-prop.c (ipcp_free_transformation_sum): New function. >>> * ipa-prop.h (ipcp_free_transformation_sum): Add declaration. > Hi, > actually looking around in order to implement the suggestions I realized > that already some code was put in place in toplev::finalize calling > then ipa_cp_c_finalize exactly for this purpose. > > I've updated the patch accordingly. > > Bootstraped on aarch64. > > Is it okay for trunk? > > Bests > Andrea > > gcc/ChangeLog > 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> > > * ipa-cp.c (ipa_cp_c_finalize): Release ipcp_transformation_sum. > * ipa-prop.c (ipcp_free_transformation_sum): New function. > * ipa-prop.h (ipcp_free_transformation_sum): Add declaration. OK for the trunk. jeff
Jeff Law writes: > On 10/1/19 4:11 AM, Andrea Corallo wrote: >> Martin Jambor writes: >> >>> Hi, >>> >>> On Mon, Sep 30 2019, Andrea Corallo wrote: >>>> Hi all, >>>> I'd like to submit this patch. >>>> It release the ipa cp transformation summary after functions being expanded. >>>> This is to fix the compiler when used with libgccjit on subsequent >>>> compilations (every new compilation should have a clean transformation >>>> summary). >>> if this is a general problem then I think we should instead add another >>> hook to class ipa_opt_pass_d to free transformation summary, call it for >>> all IPA passes at the appropriate time and implement it for IPA-CP. That >>> way it will work for all IPA passes which might have a transformation >>> summary. >>> >>> Martin >>> >>> >>>> Bootstrap on arm64 and X86-64. >>>> >>>> Bests >>>> Andrea >>>> >>>> gcc/ChangeLog >>>> 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> >>>> >>>> * cgraphunit.c (expand_all_functions): Release ipcp_transformation_sum >>>> when finished. >>>> * ipa-prop.c (ipcp_free_transformation_sum): New function. >>>> * ipa-prop.h (ipcp_free_transformation_sum): Add declaration. >> Hi, >> actually looking around in order to implement the suggestions I realized >> that already some code was put in place in toplev::finalize calling >> then ipa_cp_c_finalize exactly for this purpose. >> >> I've updated the patch accordingly. >> >> Bootstraped on aarch64. >> >> Is it okay for trunk? >> >> Bests >> Andrea >> >> gcc/ChangeLog >> 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> >> >> * ipa-cp.c (ipa_cp_c_finalize): Release ipcp_transformation_sum. >> * ipa-prop.c (ipcp_free_transformation_sum): New function. >> * ipa-prop.h (ipcp_free_transformation_sum): Add declaration. > OK for the trunk. > > jeff Applied as r276507. The same patch applies cleanly onto gcc-9-branch, I think would be good to back port it cause libgccjit is not very usable without. Should we back port it? Bests Andrea
On 10/3/19 6:47 AM, Andrea Corallo wrote: > > Jeff Law writes: > >> On 10/1/19 4:11 AM, Andrea Corallo wrote: >>> Martin Jambor writes: >>> >>>> Hi, >>>> >>>> On Mon, Sep 30 2019, Andrea Corallo wrote: >>>>> Hi all, >>>>> I'd like to submit this patch. >>>>> It release the ipa cp transformation summary after functions being expanded. >>>>> This is to fix the compiler when used with libgccjit on subsequent >>>>> compilations (every new compilation should have a clean transformation >>>>> summary). >>>> if this is a general problem then I think we should instead add another >>>> hook to class ipa_opt_pass_d to free transformation summary, call it for >>>> all IPA passes at the appropriate time and implement it for IPA-CP. That >>>> way it will work for all IPA passes which might have a transformation >>>> summary. >>>> >>>> Martin >>>> >>>> >>>>> Bootstrap on arm64 and X86-64. >>>>> >>>>> Bests >>>>> Andrea >>>>> >>>>> gcc/ChangeLog >>>>> 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> >>>>> >>>>> * cgraphunit.c (expand_all_functions): Release ipcp_transformation_sum >>>>> when finished. >>>>> * ipa-prop.c (ipcp_free_transformation_sum): New function. >>>>> * ipa-prop.h (ipcp_free_transformation_sum): Add declaration. >>> Hi, >>> actually looking around in order to implement the suggestions I realized >>> that already some code was put in place in toplev::finalize calling >>> then ipa_cp_c_finalize exactly for this purpose. >>> >>> I've updated the patch accordingly. >>> >>> Bootstraped on aarch64. >>> >>> Is it okay for trunk? >>> >>> Bests >>> Andrea >>> >>> gcc/ChangeLog >>> 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> >>> >>> * ipa-cp.c (ipa_cp_c_finalize): Release ipcp_transformation_sum. >>> * ipa-prop.c (ipcp_free_transformation_sum): New function. >>> * ipa-prop.h (ipcp_free_transformation_sum): Add declaration. >> OK for the trunk. >> >> jeff > > Applied as r276507. > > The same patch applies cleanly onto gcc-9-branch, I think would be good to > back port it cause libgccjit is not very usable without. > Should we back port it? It's a bit out of the scope of what we usually backport, but I think this is a reasonable exception. So, yea, if you want, go ahead. Thanks jeff
Jeff Law writes: > On 10/3/19 6:47 AM, Andrea Corallo wrote: >> >> Jeff Law writes: >> >>> On 10/1/19 4:11 AM, Andrea Corallo wrote: >>>> Martin Jambor writes: >>>> >>>>> Hi, >>>>> >>>>> On Mon, Sep 30 2019, Andrea Corallo wrote: >>>>>> Hi all, >>>>>> I'd like to submit this patch. >>>>>> It release the ipa cp transformation summary after functions being expanded. >>>>>> This is to fix the compiler when used with libgccjit on subsequent >>>>>> compilations (every new compilation should have a clean transformation >>>>>> summary). >>>>> if this is a general problem then I think we should instead add another >>>>> hook to class ipa_opt_pass_d to free transformation summary, call it for >>>>> all IPA passes at the appropriate time and implement it for IPA-CP. That >>>>> way it will work for all IPA passes which might have a transformation >>>>> summary. >>>>> >>>>> Martin >>>>> >>>>> >>>>>> Bootstrap on arm64 and X86-64. >>>>>> >>>>>> Bests >>>>>> Andrea >>>>>> >>>>>> gcc/ChangeLog >>>>>> 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> >>>>>> >>>>>> * cgraphunit.c (expand_all_functions): Release ipcp_transformation_sum >>>>>> when finished. >>>>>> * ipa-prop.c (ipcp_free_transformation_sum): New function. >>>>>> * ipa-prop.h (ipcp_free_transformation_sum): Add declaration. >>>> Hi, >>>> actually looking around in order to implement the suggestions I realized >>>> that already some code was put in place in toplev::finalize calling >>>> then ipa_cp_c_finalize exactly for this purpose. >>>> >>>> I've updated the patch accordingly. >>>> >>>> Bootstraped on aarch64. >>>> >>>> Is it okay for trunk? >>>> >>>> Bests >>>> Andrea >>>> >>>> gcc/ChangeLog >>>> 2019-??-?? Andrea Corallo <andrea.corallo@arm.com> >>>> >>>> * ipa-cp.c (ipa_cp_c_finalize): Release ipcp_transformation_sum. >>>> * ipa-prop.c (ipcp_free_transformation_sum): New function. >>>> * ipa-prop.h (ipcp_free_transformation_sum): Add declaration. >>> OK for the trunk. >>> >>> jeff >> >> Applied as r276507. >> >> The same patch applies cleanly onto gcc-9-branch, I think would be good to >> back port it cause libgccjit is not very usable without. >> Should we back port it? > It's a bit out of the scope of what we usually backport, but I think > this is a reasonable exception. So, yea, if you want, go ahead. > > Thanks > jeff Thanks, committed as r276625. Bests Andrea
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index cb08efe..0251fa4 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -2342,6 +2342,7 @@ expand_all_functions (void) profiled_func_count, expanded_func_count); symtab->process_new_functions (); + ipcp_free_transformation_sum (); free_gimplify_stack (); free (order); diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 30948fb..0ff8085 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -561,6 +561,7 @@ struct GTY(()) ipcp_transformation void ipa_set_node_agg_value_chain (struct cgraph_node *node, struct ipa_agg_replacement_value *aggvals); void ipcp_transformation_initialize (void); +void ipcp_free_transformation_sum (void); /* ipa_edge_args stores information related to a callsite and particularly its arguments. It can be accessed by the IPA_EDGE_REF macro. */ diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c index 2f2b070..158dbe7 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -3758,6 +3758,18 @@ ipcp_transformation_initialize (void) ipcp_transformation_sum = ipcp_transformation_t::create_ggc (symtab); } +/* Release the IPA CP transformation summary. */ + +void +ipcp_free_transformation_sum (void) +{ + if (!ipcp_transformation_sum) + return; + + ipcp_transformation_sum->release (); + ipcp_transformation_sum = NULL; +} + /* Set the aggregate replacements of NODE to be AGGVALS. */ void