| Message ID | 20190124195117.ozr7i6s2cptp37s3@smtp.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Wrap 'expand_all_functions' and 'ipa_passes' around timevars | expand |
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. > >
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. > > > >
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. > > > > > >
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
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
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
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
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
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")