On Fri, Jun 19, 2015 at 9:47 AM, Tom de Vries <tom_devr...@mentor.com> wrote: > On 16/06/15 13:18, Richard Biener wrote: >> >> On Mon, Jun 15, 2015 at 12:38 AM, Tom de Vries <tom_devr...@mentor.com> >> wrote: >>> >>> On 14/06/15 23:49, Bernhard Reutner-Fischer wrote: >>>> >>>> >>>> On June 14, 2015 10:55:59 AM GMT+02:00, Tom de Vries >>>> <tom_devr...@mentor.com> wrote: >>>>> >>>>> >>>>> On 13/03/15 11:36, Richard Biener wrote: >>>>>> >>>>>> >>>>>> On Fri, Mar 13, 2015 at 11:32 AM, Tom de Vries >>>>> >>>>> >>>>> <tom_devr...@mentor.com> wrote: >>>>>>> >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> this patch moves a bunch of early-out tests from the parloops pass >>>>> >>>>> >>>>> to the >>>>>>> >>>>>>> >>>>>>> gate function. >>>>>>> >>>>>>> The only effect is for functions that we don't consider at all for >>>>>>> parallelization in the parloops pass. We no longer dump those in the >>>>>>> parloops dump file. >>>>>>> >>>>>>> Bootstrapped and reg-tested on x86_64. >>>>>>> >>>>>>> OK for stage1 trunk? >>>>>> >>>>>> >>>>>> >>>>>> Does it work with -fdump-passes? >>>>>> >>>>> >>>>> Hi, >>>>> >>>>> with -fdump-passes now fixed to work on a dummy function (r222129), I'm >>>>> >>>>> resubmitting this patch, split up in two patches. >>>>> >>>>> The first patch moves two trivial early-exit tests to the parloops >>>>> gate. >>>>> >>>>> The second patch moves the number_of_loops test to the parloops gate, >>>>> and adds a dummy loops structure in the dummy function for >>>>> -fdump-passes. >>>>> >>>>> Bootstrapped and reg-tested on x86_64. >>>>> >>>>> Both patches OK for trunk? >>>> >>>> >>>> >>>> diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c >>>> index 02f44eb..a1659a3 100644 >>>> --- a/gcc/tree-parloops.c >>>> +++ b/gcc/tree-parloops.c >>>> @@ -2535,10 +2535,6 @@ parallelize_loops (void) >>>> source_location loop_loc; >>>> >>>> /* Do not parallelize loops in the functions created by >>>> parallelization. */ >>>> - if (parallelized_function_p (cfun->decl)) >>>> - return false; >>>> - if (cfun->has_nonlocal_label) >>>> - return false; >>>> >>>> gcc_obstack_init (&parloop_obstack); >>>> reduction_info_table_type reduction_list (10); >>>> >>>> Now stray comment? >>>> Stopped reading here. >>>> >>> >>> Fixed in updated patch. >>> >>> Also: >>> - made sure cfun is not used in the gate function >>> - added missing update of function header comment for >>> init_loops_structure >>> - improved comment in pass_manager::dump_passes. >>> >>> >>> OK for trunk? >> >> >> For -fdump-passes this doesn't make much sense but I suppose >> you are after not getting the untouched functions dumped. Note >> that generally this is undesired for debugging (in my opinion) >> as diffs from the pass dump before parloops to parloops will >> contain a lot of spurious changes (and if the preceeding pass >> is changed similarly the function we run parloops on is maybe >> not even dumped there!). >> >> So I question the general idea of the change. >> > > I suppose there are two competing principles: > 1. ensure the state before the pass is in the previous dump > 2. only dump if changed > > Indeed in general we use the first principle, although it doesn't hold for > f.i. pass_tree_loop and a function without loops. > > The problems I'm trying to solve with this patch are the following: > - even if you're compiling a single function, part of the function > occurs twice (once in the original, once in the split-off > function), making formulating scan-tree-dumps more complicated for > parloops. > - the intuitive thing is to look in the parloops tree-dump and look at > the original function and the split-off function, and think that the > split-off function is the immediate result of the split-off that > happened in the pass, which is incorrect (since it jumps back in the > pass list and traverses other passes before arriving back at > parloops). > > Adding something like a flag -fno-dump-new-function would solve the first > problem, but not the second.
Yeah, any pass introducing new functions has this issue. > We could also dump new functions in a different dump file > src.c.new.123t.pass, and this would solve both problems. But it will cause > the 'where did that function go' confusion, certainly initially. > > Any other ideas? No, not really. But it sounds like a very special thing you try to solve. You could also dump the split off function twice - once next to the function it was split off and once when it arrives at parloops again... Richard. > Thanks, > - Tom >