| Message ID | 59E445EA.5030909@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] Add a pass counter for "are we there yet" purposes | expand |
On October 16, 2017 7:38:50 AM GMT+02:00, Sandra Loosemore <sandra@codesourcery.com> wrote: >This patch is a first cut at solving the problem discussed in this >thread > >https://gcc.gnu.org/ml/gcc/2017-10/msg00016.html > >where I have some nios2 backend patches in my queue that need a way of >knowing whether the split1 pass has run yet. There seemed to be >agreement that a general way to query the pass manager for this >information would be useful. > >The approach I took here is to add a new counter field, so that I can >do >the test I want with > > opt_pass *split1_pass = g->get_passes ()->get_pass_split_all_insns (); > if (current_pass->pass_number > split1_pass->pass_number) > ... > >Well, mostly. :-P There are some gotchas. > >* TARGET_ASM_OUTPUT_MI_THUNK is called outside the pass manager (so >current_pass is NULL) and on many targets this hook is implemented by >setting reload_completed to 1, generating some RTL, and invoking some >passes directly to emit code. > >* modulo-sched.c also plays tricks with setting reload_completed to >pretend to be something it's not. > >* Possibly other places? E.g. I'm not familiar with how plugins work. > >For my purposes it's good enough to check reload_completed before the >test in the code snippet above, but trying to determine whether a >particular post-reload pass has run won't work. So this isn't as >general as it ought to be, at least not until we get rid of the >reload_completed hackery. > >Since this patch isn't useful without something that uses the pass >counters, I tested it on nios2-linux-gnu with my not-yet-posted patch >set, by wiring it up in parallel with my previously-implemented >solution >of adding a target-specific pass to set a flag, with various assertions > >to check for consistency. I also had some temporary debugging code in >there at one point to print the pass numbers. > >WDYT? Is this the right direction? I'm somewhat worried that we're >getting late in stage 1 and I'd really like to finish up my nios2 >patches; so if getting this right looks like a tar pit, I think I >should >probably stick with my previous implementation for now. I missed the post of why you need to know this. But as you noticed we're using reload_completed for similar purpose. There's also the possibility of setting/adding a pass property that split could provide and which you could query. We're using this to signal the various different lowering stages in GIMPLE for example. Richard. > >-Sandra
On 10/16/2017 12:53 AM, Richard Biener wrote: > On October 16, 2017 7:38:50 AM GMT+02:00, Sandra Loosemore <sandra@codesourcery.com> wrote: >> This patch is a first cut at solving the problem discussed in this >> thread >> >> https://gcc.gnu.org/ml/gcc/2017-10/msg00016.html >> >> where I have some nios2 backend patches in my queue that need a way of >> knowing whether the split1 pass has run yet. There seemed to be >> agreement that a general way to query the pass manager for this >> information would be useful. >> >> [snip] > > I missed the post of why you need to know this. But as you noticed we're using reload_completed for similar purpose. There's also the possibility of setting/adding a pass property that split could provide and which you could query. We're using this to signal the various different lowering stages in GIMPLE for example. See the thread linked above. There was interest in a general mechanism to query the pass manager state instead of adding the bookkeeping to the nios2 backend or adding something to track the split1 pass to the target-independent parts of the compiler. After fiddling with it, though, I'm not sure this is an improvement. Maybe it would help the discussion if I got my nios2 patch set posted so that everybody could see the actual use case? It'll take me a few days to finish cleaning it up. -Sandra
On October 16, 2017 5:46:35 PM GMT+02:00, Sandra Loosemore <sandra@codesourcery.com> wrote: >On 10/16/2017 12:53 AM, Richard Biener wrote: >> On October 16, 2017 7:38:50 AM GMT+02:00, Sandra Loosemore ><sandra@codesourcery.com> wrote: >>> This patch is a first cut at solving the problem discussed in this >>> thread >>> >>> https://gcc.gnu.org/ml/gcc/2017-10/msg00016.html >>> >>> where I have some nios2 backend patches in my queue that need a way >of >>> knowing whether the split1 pass has run yet. There seemed to be >>> agreement that a general way to query the pass manager for this >>> information would be useful. >>> >>> [snip] >> >> I missed the post of why you need to know this. But as you noticed >we're using reload_completed for similar purpose. There's also the >possibility of setting/adding a pass property that split could provide >and which you could query. We're using this to signal the various >different lowering stages in GIMPLE for example. > >See the thread linked above. There was interest in a general mechanism > >to query the pass manager state instead of adding the bookkeeping to >the >nios2 backend or adding something to track the split1 pass to the >target-independent parts of the compiler. After fiddling with it, >though, I'm not sure this is an improvement. > >Maybe it would help the discussion if I got my nios2 patch set posted >so >that everybody could see the actual use case? It'll take me a few days > >to finish cleaning it up. I guess that might help. I have the feeling that querying for 'did pass X run' is wrong conceptually. Richard. >-Sandra
On 10/16/2017 10:15 AM, Richard Biener wrote: > On October 16, 2017 5:46:35 PM GMT+02:00, Sandra Loosemore <sandra@codesourcery.com> wrote: >> On 10/16/2017 12:53 AM, Richard Biener wrote: >>> On October 16, 2017 7:38:50 AM GMT+02:00, Sandra Loosemore >> <sandra@codesourcery.com> wrote: >>>> This patch is a first cut at solving the problem discussed in this >>>> thread >>>> >>>> https://gcc.gnu.org/ml/gcc/2017-10/msg00016.html >>>> >>>> where I have some nios2 backend patches in my queue that need a way >> of >>>> knowing whether the split1 pass has run yet. There seemed to be >>>> agreement that a general way to query the pass manager for this >>>> information would be useful. >>>> >>>> [snip] >>> >>> I missed the post of why you need to know this. But as you noticed >> we're using reload_completed for similar purpose. There's also the >> possibility of setting/adding a pass property that split could provide >> and which you could query. We're using this to signal the various >> different lowering stages in GIMPLE for example. >> >> See the thread linked above. There was interest in a general mechanism >> >> to query the pass manager state instead of adding the bookkeeping to >> the >> nios2 backend or adding something to track the split1 pass to the >> target-independent parts of the compiler. After fiddling with it, >> though, I'm not sure this is an improvement. >> >> Maybe it would help the discussion if I got my nios2 patch set posted >> so >> that everybody could see the actual use case? It'll take me a few days >> >> to finish cleaning it up. > > I guess that might help. I have the feeling that querying for 'did pass X run' is wrong conceptually. Now posted in this thread. https://gcc.gnu.org/ml/gcc-patches/2017-10/msg01309.html -Sandra
Hi, On Mon, Oct 16, 2017 at 06:15:06PM +0200, Richard Biener wrote: > I guess that might help. I have the feeling that querying for 'did > pass X run' is wrong conceptually. The reason why I liked the idea is that I could unify SRA and early-SRA passes and their behavior would only differ according to a "did pass_starg run yet" query. Admittedly, it is not a big deal, I just always dislike typing "-fdump-tree-esra-details -fdump-tree-sra-details" when debugging :-) Martin
On Mon, Oct 23, 2017 at 12:18:58PM +0200, Martin Jambor wrote: > Hi, > > On Mon, Oct 16, 2017 at 06:15:06PM +0200, Richard Biener wrote: > > I guess that might help. I have the feeling that querying for 'did > > pass X run' is wrong conceptually. > > The reason why I liked the idea is that I could unify SRA and > early-SRA passes and their behavior would only differ according to a > "did pass_starg run yet" query. > > Admittedly, it is not a big deal, I just always dislike typing > "-fdump-tree-esra-details -fdump-tree-sra-details" when debugging :-) -fdump-tree-{,e}sra-details when using sane shell ;) ? Jakub
On Mon, Oct 23, 2017 at 12:18 PM, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > On Mon, Oct 16, 2017 at 06:15:06PM +0200, Richard Biener wrote: >> I guess that might help. I have the feeling that querying for 'did >> pass X run' is wrong conceptually. > > The reason why I liked the idea is that I could unify SRA and > early-SRA passes and their behavior would only differ according to a > "did pass_starg run yet" query. I think that "did pass_stdarg run yet" query isn't necessary anymore given we don't lower va-arg during gimplification. Richard. > Admittedly, it is not a big deal, I just always dislike typing > "-fdump-tree-esra-details -fdump-tree-sra-details" when debugging :-) > > Martin
Index: gcc/pass_manager.h =================================================================== --- gcc/pass_manager.h (revision 253327) +++ gcc/pass_manager.h (working copy) @@ -103,6 +103,7 @@ private: void set_pass_for_id (int id, opt_pass *pass); void register_dump_files (opt_pass *pass); void create_pass_tab () const; + void enumerate_passes () const; private: context *m_ctxt; Index: gcc/tree-pass.h =================================================================== --- gcc/tree-pass.h (revision 253327) +++ gcc/tree-pass.h (working copy) @@ -108,6 +108,11 @@ public: /* Static pass number, used as a fragment of the dump file name. */ int static_pass_number; + /* Pass number used for enumerating the pass list. This differs from + static_pass_number in that the numbering is sequential and all passes + have a meaningful number assigned. */ + int pass_number; + protected: gcc::context *m_ctxt; }; Index: gcc/passes.c =================================================================== --- gcc/passes.c (revision 253327) +++ gcc/passes.c (working copy) @@ -899,6 +899,35 @@ pass_manager::create_pass_tab (void) con static bool override_gate_status (opt_pass *, tree, bool); +/* Enumerate pass list PASS starting with counter COUNTER. */ + +static int +enumerate_pass_list (opt_pass *pass, int counter) +{ + do + { + pass->pass_number = ++counter; + if (pass->sub) + counter = enumerate_pass_list (pass->sub, counter); + pass = pass->next; + } + while (pass); + return counter; +} + +/* Enumerate all optimization passes. */ + +void +pass_manager::enumerate_passes () const +{ + int counter = 0; + counter = enumerate_pass_list (all_lowering_passes, counter); + counter = enumerate_pass_list (all_small_ipa_passes, counter); + counter = enumerate_pass_list (all_regular_ipa_passes, counter); + counter = enumerate_pass_list (all_late_ipa_passes, counter); + counter = enumerate_pass_list (all_passes, counter); +} + /* Dump the instantiated name for PASS. IS_ON indicates if PASS is turned on or not. */ @@ -1508,6 +1537,9 @@ pass_manager::register_pass (struct regi XDELETE (added_pass_nodes); added_pass_nodes = next_node; } + + /* Update the pass enumeration. */ + enumerate_passes (); } /* Construct the pass tree. The sequencing of passes is driven by @@ -1614,6 +1646,9 @@ pass_manager::pass_manager (context *ctx register_dump_files (all_regular_ipa_passes); register_dump_files (all_late_ipa_passes); register_dump_files (all_passes); + + /* Enumerate passes. */ + enumerate_passes (); } static void
This patch is a first cut at solving the problem discussed in this thread https://gcc.gnu.org/ml/gcc/2017-10/msg00016.html where I have some nios2 backend patches in my queue that need a way of knowing whether the split1 pass has run yet. There seemed to be agreement that a general way to query the pass manager for this information would be useful. The approach I took here is to add a new counter field, so that I can do the test I want with opt_pass *split1_pass = g->get_passes ()->get_pass_split_all_insns (); if (current_pass->pass_number > split1_pass->pass_number) ... Well, mostly. :-P There are some gotchas. * TARGET_ASM_OUTPUT_MI_THUNK is called outside the pass manager (so current_pass is NULL) and on many targets this hook is implemented by setting reload_completed to 1, generating some RTL, and invoking some passes directly to emit code. * modulo-sched.c also plays tricks with setting reload_completed to pretend to be something it's not. * Possibly other places? E.g. I'm not familiar with how plugins work. For my purposes it's good enough to check reload_completed before the test in the code snippet above, but trying to determine whether a particular post-reload pass has run won't work. So this isn't as general as it ought to be, at least not until we get rid of the reload_completed hackery. Since this patch isn't useful without something that uses the pass counters, I tested it on nios2-linux-gnu with my not-yet-posted patch set, by wiring it up in parallel with my previously-implemented solution of adding a target-specific pass to set a flag, with various assertions to check for consistency. I also had some temporary debugging code in there at one point to print the pass numbers. WDYT? Is this the right direction? I'm somewhat worried that we're getting late in stage 1 and I'd really like to finish up my nios2 patches; so if getting this right looks like a tar pit, I think I should probably stick with my previous implementation for now. -Sandra