| Message ID | 5502BCA2.2010802@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Mar 13, 2015 at 11:32 AM, Tom de Vries <Tom_deVries@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? Richard. > Thanks, > - Tom
On 13-03-15 11:36, Richard Biener wrote: > On Fri, Mar 13, 2015 at 11:32 AM, Tom de Vries <Tom_deVries@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? Hmm, I never heard of the option, interesting, thanks. Let's see what it is supposed to do: ... @item -fdump-passes @opindex fdump-passes Dump the list of optimization passes that are turned on and off by the current command-line options. ... fdump-passes seems to be using the gate function to find out the effect of the command line options on the pass. But then f.i. in pass_stdarg, using fun->stdarg in the gate function violates that AFAIU. Does this mean the gate function of pass_stdarg should be fixed by moving the fun->stdarg test from the gate to the execute function? Thanks, - Tom
On Fri, Mar 13, 2015 at 12:12 PM, Tom de Vries <Tom_deVries@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_deVries@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? > > > Hmm, I never heard of the option, interesting, thanks. > > Let's see what it is supposed to do: > ... > @item -fdump-passes > @opindex fdump-passes > Dump the list of optimization passes that are turned on and off by > the current command-line options. > ... > > fdump-passes seems to be using the gate function to find out the effect of > the command line options on the pass. > > But then f.i. in pass_stdarg, using fun->stdarg in the gate function > violates that AFAIU. > > Does this mean the gate function of pass_stdarg should be fixed by moving > the fun->stdarg test from the gate to the execute function? Not really (I don't like -fdump-passes ...), but we need to make sure that -fdump-passes doesn't crash (because it runs very early and with cfun == NULL I think) Richard. > Thanks, > - Tom
On Fri, Mar 13, 2015 at 01:04:57PM +0100, Richard Biener wrote: > Not really (I don't like -fdump-passes ...), but we need to make sure > that -fdump-passes doesn't crash (because it runs very early and > with cfun == NULL I think) If it runs with cfun == NULL, then supposedly the gates that are dependent on current function should for -fdump-passes purposes also return true if cfun == NULL (well, of course do all the unconditional checks). Though of course, with optimize/target attributes this is harder, as different functions can use different options. Jakub
On Fri, Mar 13, 2015 at 1:07 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Fri, Mar 13, 2015 at 01:04:57PM +0100, Richard Biener wrote: >> Not really (I don't like -fdump-passes ...), but we need to make sure >> that -fdump-passes doesn't crash (because it runs very early and >> with cfun == NULL I think) > > If it runs with cfun == NULL, then supposedly the gates that are dependent > on current function should for -fdump-passes purposes also return true > if cfun == NULL (well, of course do all the unconditional checks). > Though of course, with optimize/target attributes this is harder, as > different functions can use different options. Yes, one reason why I think -fdump-passes is just broken implementation-wise. Richard. > Jakub
On 13-03-15 13:36, Richard Biener wrote: > On Fri, Mar 13, 2015 at 1:07 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> On Fri, Mar 13, 2015 at 01:04:57PM +0100, Richard Biener wrote: >>> Not really (I don't like -fdump-passes ...), but we need to make sure >>> that -fdump-passes doesn't crash (because it runs very early and >>> with cfun == NULL I think) >> >> If it runs with cfun == NULL, then supposedly the gates that are dependent >> on current function should for -fdump-passes purposes also return true >> if cfun == NULL (well, of course do all the unconditional checks). >> Though of course, with optimize/target attributes this is harder, as >> different functions can use different options. > > Yes, one reason why I think -fdump-passes is just broken implementation-wise. > Atm fdump-passes doesn't run with cfun == NULL. From pass_manager::dump_passes: ... FOR_EACH_FUNCTION (n) if (DECL_STRUCT_FUNCTION (n->decl)) { node = n; break; } if (!node) return; push_cfun (DECL_STRUCT_FUNCTION (node->decl)); ... This was discussed here: https://gcc.gnu.org/ml/gcc-patches/2011-06/msg00856.html Thanks, - Tom
On Fri, Mar 13, 2015 at 4:28 PM, Tom de Vries <Tom_deVries@mentor.com> wrote: > On 13-03-15 13:36, Richard Biener wrote: >> >> On Fri, Mar 13, 2015 at 1:07 PM, Jakub Jelinek <jakub@redhat.com> wrote: >>> >>> On Fri, Mar 13, 2015 at 01:04:57PM +0100, Richard Biener wrote: >>>> >>>> Not really (I don't like -fdump-passes ...), but we need to make sure >>>> that -fdump-passes doesn't crash (because it runs very early and >>>> with cfun == NULL I think) >>> >>> >>> If it runs with cfun == NULL, then supposedly the gates that are >>> dependent >>> on current function should for -fdump-passes purposes also return true >>> if cfun == NULL (well, of course do all the unconditional checks). >>> Though of course, with optimize/target attributes this is harder, as >>> different functions can use different options. >> >> >> Yes, one reason why I think -fdump-passes is just broken >> implementation-wise. >> > > Atm fdump-passes doesn't run with cfun == NULL. > > From pass_manager::dump_passes: > ... > FOR_EACH_FUNCTION (n) > if (DECL_STRUCT_FUNCTION (n->decl)) > { > node = n; > break; > } > > if (!node) > return; > > push_cfun (DECL_STRUCT_FUNCTION (node->decl)); Um - this now picks a random function which may be one with an optimize or target attribute associated to it. Richard. > ... > > This was discussed here: > https://gcc.gnu.org/ml/gcc-patches/2011-06/msg00856.html > > Thanks, > - Tom
2015-03-10 Tom de Vries <tom@codesourcery.com> * tree-parloops.c (parallelize_loops): Move early-out tests from here ... (pass_parallelize_loops::execute): ... and here ... (pass_parallelize_loops::gate): ... to here. diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c index 5f7c1bc..3a80cea 100644 --- a/gcc/tree-parloops.c +++ b/gcc/tree-parloops.c @@ -2163,12 +2163,6 @@ parallelize_loops (void) HOST_WIDE_INT estimated; 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); init_stmt_vec_info_vec (); @@ -2286,7 +2280,15 @@ public: {} /* opt_pass methods: */ - virtual bool gate (function *) { return flag_tree_parallelize_loops > 1; } + virtual bool gate (function *fun) + { + return (flag_tree_parallelize_loops > 1 + /* Do not parallelize loops in the functions created by + parallelization. */ + && !parallelized_function_p (fun->decl) + && !fun->has_nonlocal_label + && number_of_loops (fun) > 1); + } virtual unsigned int execute (function *); }; // class pass_parallelize_loops @@ -2294,9 +2296,6 @@ public: unsigned pass_parallelize_loops::execute (function *fun) { - if (number_of_loops (fun) <= 1) - return 0; - if (parallelize_loops ()) { fun->curr_properties &= ~(PROP_gimple_eomp); -- 1.9.1