IPA-CP release transformation summary (PR jit/91928)

Message ID gkrlfu6uphd.fsf@arm.com
State New
Headers show
Series
  • IPA-CP release transformation summary (PR jit/91928)
Related show

Commit Message

Andrea Corallo Sept. 30, 2019, 9:07 a.m.
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).

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.

Comments

Martin Jambor Sept. 30, 2019, 9:19 a.m. | #1
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.
Andrea Corallo Oct. 1, 2019, 10:11 a.m. | #2
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.
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index b4fb74e..2b40220 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -5305,4 +5305,5 @@ ipa_cp_c_finalize (void)
   max_count = profile_count::uninitialized ();
   overall_size = 0;
   max_new_size = 0;
+  ipcp_free_transformation_sum ();
 }
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
Jeff Law Oct. 1, 2019, 8:35 p.m. | #3
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
Andrea Corallo Oct. 3, 2019, 12:47 p.m. | #4
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
Jeff Law Oct. 4, 2019, 1:27 a.m. | #5
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
Andrea Corallo Oct. 5, 2019, 9:06 a.m. | #6
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

Patch

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