[PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[Giuliano Belinassi]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

This patch adds two variables named 'TV_CGRAPH_FUNC_EXPANSION' and
'TV_CGRAPH_IPA_PASSES' that count the elapsed time of the functions
'expand_all_functions' and 'ipa_passes', respectivelly.

The main point of this is that these functions takes a very long time
when compiling the 'gimple-match.c' file, and therefore may also take
a long time when compiling other large files.

I also accept suggestions about how to improve this :-)

ChangeLog:

2019-01-24	Giuliano Belinassi <giuliano.belinassi@usp.br>

	* cgraph_unit.c (compile): TV_CGRAPH_FUNC_EXPANSION and
	TV_CGRAPH_IPA_PASSES start, stop.
	* timevar.def (TV_CGRAPH_IPA_PASSES, TV_CGRAPH_FUNC_EXPANSION): New.



[-- Attachment #2: fine_grain_opt.patch --]
[-- Type: text/x-diff, Size: 1593 bytes --]

Index: gcc/cgraphunit.c
===================================================================
--- gcc/cgraphunit.c	(revision 267983)
+++ gcc/cgraphunit.c	(working copy)
@@ -2615,8 +2615,11 @@
 
   /* Don't run the IPA passes if there was any error or sorry messages.  */
   if (!seen_error ())
+  {
+    timevar_start (TV_CGRAPH_IPA_PASSES);
     ipa_passes ();
-
+    timevar_stop (TV_CGRAPH_IPA_PASSES);
+  }
   /* Do nothing else if any IPA pass found errors or if we are just streaming LTO.  */
   if (seen_error ()
       || ((!in_lto_p || flag_incremental_link == INCREMENTAL_LINK_LTO)
@@ -2682,7 +2685,11 @@
   /* Output first asm statements and anything ordered. The process
      flag is cleared for these nodes, so we skip them later.  */
   output_in_order ();
+
+  timevar_start (TV_CGRAPH_FUNC_EXPANSION);
   expand_all_functions ();
+  timevar_stop (TV_CGRAPH_FUNC_EXPANSION);
+
   output_variables ();
 
   process_new_functions ();
Index: gcc/timevar.def
===================================================================
--- gcc/timevar.def	(revision 267983)
+++ gcc/timevar.def	(working copy)
@@ -68,6 +68,8 @@
 
 DEFTIMEVAR (TV_CGRAPH                , "callgraph construction")
 DEFTIMEVAR (TV_CGRAPHOPT             , "callgraph optimization")
+DEFTIMEVAR (TV_CGRAPH_FUNC_EXPANSION , "callgraph functions expansion")
+DEFTIMEVAR (TV_CGRAPH_IPA_PASSES     , "callgraph ipa passes")
 DEFTIMEVAR (TV_IPA_FNSUMMARY         , "ipa function summary")
 DEFTIMEVAR (TV_IPA_UNREACHABLE       , "ipa dead code removal")
 DEFTIMEVAR (TV_IPA_INHERITANCE       , "ipa inheritance graph")

Re: [PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[Richard Biener]

On Thu, Jan 24, 2019 at 8:51 PM Giuliano Belinassi
<giuliano.belinassi@usp.br> wrote:
>
> This patch adds two variables named 'TV_CGRAPH_FUNC_EXPANSION' and
> 'TV_CGRAPH_IPA_PASSES' that count the elapsed time of the functions
> 'expand_all_functions' and 'ipa_passes', respectivelly.
>
> The main point of this is that these functions takes a very long time
> when compiling the 'gimple-match.c' file, and therefore may also take
> a long time when compiling other large files.

But as implemented those shouldn't accumulate a lot of time because other
timevars are pushed for them.

I think you want to instead split TV_PHASE_OPT_GEN into
TV_PHASE_OPT_GIMPLE TV_PHASE_IPA and TV_PHASE_RTL
which probably can be easiest managed by the pass manager
pushing/popping those appropriately depending on the pass kind
that is invoked?

Richard.

> I also accept suggestions about how to improve this :-)
>
> ChangeLog:
>
> 2019-01-24      Giuliano Belinassi <giuliano.belinassi@usp.br>
>
>         * cgraph_unit.c (compile): TV_CGRAPH_FUNC_EXPANSION and
>         TV_CGRAPH_IPA_PASSES start, stop.
>         * timevar.def (TV_CGRAPH_IPA_PASSES, TV_CGRAPH_FUNC_EXPANSION): New.
>
>

Re: [PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[Giuliano Belinassi]

On 01/28, Richard Biener wrote:
> On Thu, Jan 24, 2019 at 8:51 PM Giuliano Belinassi
> <giuliano.belinassi@usp.br> wrote:
> >
> > This patch adds two variables named 'TV_CGRAPH_FUNC_EXPANSION' and
> > 'TV_CGRAPH_IPA_PASSES' that count the elapsed time of the functions
> > 'expand_all_functions' and 'ipa_passes', respectivelly.
> >
> > The main point of this is that these functions takes a very long time
> > when compiling the 'gimple-match.c' file, and therefore may also take
> > a long time when compiling other large files.
> 
> But as implemented those shouldn't accumulate a lot of time because other
> timevars are pushed for them.
> 

It seems not to be what is happening. For instance:

 callgraph functions expansion      :  49.42 ( 73%)   1.18 ( 29%)  50.60 ( 70%)  770140 kB ( 48%)
 callgraph ipa passes               :  13.10 ( 19%)   1.47 ( 36%)  14.57 ( 20%)  388727 kB ( 24%)

I also tried push/pop'ing these variables, however the reported time
was 0

> I think you want to instead split TV_PHASE_OPT_GEN into
> TV_PHASE_OPT_GIMPLE TV_PHASE_IPA and TV_PHASE_RTL
> which probably can be easiest managed by the pass manager
> pushing/popping those appropriately depending on the pass kind
> that is invoked?
>
I will check this out.
Just a question: Is the function expansion a part of GIMPLE?

Giuliano.

> Richard.
> 
> > I also accept suggestions about how to improve this :-)
> >
> > ChangeLog:
> >
> > 2019-01-24      Giuliano Belinassi <giuliano.belinassi@usp.br>
> >
> >         * cgraph_unit.c (compile): TV_CGRAPH_FUNC_EXPANSION and
> >         TV_CGRAPH_IPA_PASSES start, stop.
> >         * timevar.def (TV_CGRAPH_IPA_PASSES, TV_CGRAPH_FUNC_EXPANSION): New.
> >
> >

Re: [PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[Richard Biener]

On Mon, Jan 28, 2019 at 2:16 PM Giuliano Belinassi
<giuliano.belinassi@usp.br> wrote:
>
> On 01/28, Richard Biener wrote:
> > On Thu, Jan 24, 2019 at 8:51 PM Giuliano Belinassi
> > <giuliano.belinassi@usp.br> wrote:
> > >
> > > This patch adds two variables named 'TV_CGRAPH_FUNC_EXPANSION' and
> > > 'TV_CGRAPH_IPA_PASSES' that count the elapsed time of the functions
> > > 'expand_all_functions' and 'ipa_passes', respectivelly.
> > >
> > > The main point of this is that these functions takes a very long time
> > > when compiling the 'gimple-match.c' file, and therefore may also take
> > > a long time when compiling other large files.
> >
> > But as implemented those shouldn't accumulate a lot of time because other
> > timevars are pushed for them.
> >
>
> It seems not to be what is happening. For instance:
>
>  callgraph functions expansion      :  49.42 ( 73%)   1.18 ( 29%)  50.60 ( 70%)  770140 kB ( 48%)
>  callgraph ipa passes               :  13.10 ( 19%)   1.47 ( 36%)  14.57 ( 20%)  388727 kB ( 24%)
>
> I also tried push/pop'ing these variables, however the reported time
> was 0
>
> > I think you want to instead split TV_PHASE_OPT_GEN into
> > TV_PHASE_OPT_GIMPLE TV_PHASE_IPA and TV_PHASE_RTL
> > which probably can be easiest managed by the pass manager
> > pushing/popping those appropriately depending on the pass kind
> > that is invoked?
> >
> I will check this out.
> Just a question: Is the function expansion a part of GIMPLE?

It covers both GIMPLE and RTL I think (maybe also parts of IPA).

Richard.

> Giuliano.
>
> > Richard.
> >
> > > I also accept suggestions about how to improve this :-)
> > >
> > > ChangeLog:
> > >
> > > 2019-01-24      Giuliano Belinassi <giuliano.belinassi@usp.br>
> > >
> > >         * cgraph_unit.c (compile): TV_CGRAPH_FUNC_EXPANSION and
> > >         TV_CGRAPH_IPA_PASSES start, stop.
> > >         * timevar.def (TV_CGRAPH_IPA_PASSES, TV_CGRAPH_FUNC_EXPANSION): New.
> > >
> > >

Re: [PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[Jeff Law]

On 1/24/19 12:51 PM, Giuliano Belinassi wrote:
> This patch adds two variables named 'TV_CGRAPH_FUNC_EXPANSION' and
> 'TV_CGRAPH_IPA_PASSES' that count the elapsed time of the functions
> 'expand_all_functions' and 'ipa_passes', respectivelly.
> 
> The main point of this is that these functions takes a very long time
> when compiling the 'gimple-match.c' file, and therefore may also take
> a long time when compiling other large files.
> 
> I also accept suggestions about how to improve this :-)
> 
> ChangeLog:
> 
> 2019-01-24	Giuliano Belinassi <giuliano.belinassi@usp.br>
> 
> 	* cgraph_unit.c (compile): TV_CGRAPH_FUNC_EXPANSION and
> 	TV_CGRAPH_IPA_PASSES start, stop.
> 	* timevar.def (TV_CGRAPH_IPA_PASSES, TV_CGRAPH_FUNC_EXPANSION): New.
So I'm guessing you want the accumulated time for the ipa_passes and
expansion.  So independent counters using timevar_{start,stop} seem right.

The alternate approach would be to use the timevar_{push,pop}. This
would change what timevar is on the top of the stack when we call
ipa_passes and expand_all_functions -- it would, in effect, allow us to
determine how much time is spent in those functions which is _not_
attributable to existing timevars.

I just want to confirm your intent before acking/naking.

jeff

Re: [PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[Giuliano Belinassi]

Hi,

Oh, I have completely forgotten about this patch

On 06/19, Jeff Law wrote:
> On 1/24/19 12:51 PM, Giuliano Belinassi wrote:
> > This patch adds two variables named 'TV_CGRAPH_FUNC_EXPANSION' and
> > 'TV_CGRAPH_IPA_PASSES' that count the elapsed time of the functions
> > 'expand_all_functions' and 'ipa_passes', respectivelly.
> > 
> > The main point of this is that these functions takes a very long time
> > when compiling the 'gimple-match.c' file, and therefore may also take
> > a long time when compiling other large files.
> > 
> > I also accept suggestions about how to improve this :-)
> > 
> > ChangeLog:
> > 
> > 2019-01-24	Giuliano Belinassi <giuliano.belinassi@usp.br>
> > 
> > 	* cgraph_unit.c (compile): TV_CGRAPH_FUNC_EXPANSION and
> > 	TV_CGRAPH_IPA_PASSES start, stop.
> > 	* timevar.def (TV_CGRAPH_IPA_PASSES, TV_CGRAPH_FUNC_EXPANSION): New.
> So I'm guessing you want the accumulated time for the ipa_passes and
> expansion.  So independent counters using timevar_{start,stop} seem right.

Yes, my point was to accumulate the total time spent in those function,
including everything these functions calls.

With regard to breaking the timevar with IPA, GIMPLE and RTL expansion
passes, I can do this but it will require splitting `all_passes` into
`all_passes` and `all_rtl_passes`, as suggested by richi in the
parallelization thread. I can do this fairly easily since I have
already done it done in my branch. Is it OK?

> 
> The alternate approach would be to use the timevar_{push,pop}. This
> would change what timevar is on the top of the stack when we call
> ipa_passes and expand_all_functions -- it would, in effect, allow us to
> determine how much time is spent in those functions which is _not_
> attributable to existing timevars.
> 
> I just want to confirm your intent before acking/naking.
> 
> jeff

Giuliano

Re: [PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[Jeff Law]

On 6/19/19 2:26 PM, Giuliano Belinassi wrote:

> On 06/19, Jeff Law wrote:
>> On 1/24/19 12:51 PM, Giuliano Belinassi wrote:
>>> This patch adds two variables named 'TV_CGRAPH_FUNC_EXPANSION' and
>>> 'TV_CGRAPH_IPA_PASSES' that count the elapsed time of the functions
>>> 'expand_all_functions' and 'ipa_passes', respectivelly.
>>>
>>> The main point of this is that these functions takes a very long time
>>> when compiling the 'gimple-match.c' file, and therefore may also take
>>> a long time when compiling other large files.
>>>
>>> I also accept suggestions about how to improve this :-)
>>>
>>> ChangeLog:
>>>
>>> 2019-01-24	Giuliano Belinassi <giuliano.belinassi@usp.br>
>>>
>>> 	* cgraph_unit.c (compile): TV_CGRAPH_FUNC_EXPANSION and
>>> 	TV_CGRAPH_IPA_PASSES start, stop.
>>> 	* timevar.def (TV_CGRAPH_IPA_PASSES, TV_CGRAPH_FUNC_EXPANSION): New.
>> So I'm guessing you want the accumulated time for the ipa_passes and
>> expansion.  So independent counters using timevar_{start,stop} seem right.
> 
> Yes, my point was to accumulate the total time spent in those function,
> including everything these functions calls.
OK.  Then let's go with your patch as-is if you'd still like it included
on the trunk.

> 
> With regard to breaking the timevar with IPA, GIMPLE and RTL expansion
> passes, I can do this but it will require splitting `all_passes` into
> `all_passes` and `all_rtl_passes`, as suggested by richi in the
> parallelization thread. I can do this fairly easily since I have
> already done it done in my branch. Is it OK?
I think that'll be fine when you're ready to merge from your branch to
the trunk.

jeff

Re: [PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[Jeff Law]

On 1/24/19 12:51 PM, Giuliano Belinassi wrote:
> This patch adds two variables named 'TV_CGRAPH_FUNC_EXPANSION' and
> 'TV_CGRAPH_IPA_PASSES' that count the elapsed time of the functions
> 'expand_all_functions' and 'ipa_passes', respectivelly.
> 
> The main point of this is that these functions takes a very long time
> when compiling the 'gimple-match.c' file, and therefore may also take
> a long time when compiling other large files.
> 
> I also accept suggestions about how to improve this :-)
> 
> ChangeLog:
> 
> 2019-01-24	Giuliano Belinassi <giuliano.belinassi@usp.br>
> 
> 	* cgraph_unit.c (compile): TV_CGRAPH_FUNC_EXPANSION and
> 	TV_CGRAPH_IPA_PASSES start, stop.
> 	* timevar.def (TV_CGRAPH_IPA_PASSES, TV_CGRAPH_FUNC_EXPANSION): New.
> 
> 
Per our discussion WRT timevar_{start,stop} vs timevar_{push_pop}, this
is fine for the trunk if you still want to include it.

jeff

Re: [PATCH] Wrap 'expand_all_functions' and 'ipa_passes' around timevars

[Giuliano Belinassi]

Hi, Jeff.

On 07/02, Jeff Law wrote:
> On 1/24/19 12:51 PM, Giuliano Belinassi wrote:
> > This patch adds two variables named 'TV_CGRAPH_FUNC_EXPANSION' and
> > 'TV_CGRAPH_IPA_PASSES' that count the elapsed time of the functions
> > 'expand_all_functions' and 'ipa_passes', respectivelly.
> > 
> > The main point of this is that these functions takes a very long time
> > when compiling the 'gimple-match.c' file, and therefore may also take
> > a long time when compiling other large files.
> > 
> > I also accept suggestions about how to improve this :-)
> > 
> > ChangeLog:
> > 
> > 2019-01-24	Giuliano Belinassi <giuliano.belinassi@usp.br>
> > 
> > 	* cgraph_unit.c (compile): TV_CGRAPH_FUNC_EXPANSION and
> > 	TV_CGRAPH_IPA_PASSES start, stop.
> > 	* timevar.def (TV_CGRAPH_IPA_PASSES, TV_CGRAPH_FUNC_EXPANSION): New.
> > 
> > 
> Per our discussion WRT timevar_{start,stop} vs timevar_{push_pop}, this
> is fine for the trunk if you still want to include it.

It is fine for me :)

> 
> jeff