| Message ID | 20210903135723.735732-1-aldyh@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Do not assume loop header threading in backward threader. | expand |
On 9/3/2021 7:57 AM, Aldy Hernandez wrote: > The registry's thread_through_all_blocks() has a may_peel_loop_headers > argument. When refactoring the backward threader code, I removed this > argument for the local passthru method because it was always TRUE. This > may not necessarily be true in the future, if the backward threader is > called from another context. This patch removes the default definition, > in favor of an argument that is exactly the same as the identically > named function in tree-ssa-threadupdate.c. I think this also makes it > less confusing when looking at both methods across the source base. > > Tested on x86-64 Linux. > > OK? > > gcc/ChangeLog: > > * tree-ssa-threadbackward.c (back_threader::thread_through_all_blocks): > (back_threader_registry::thread_through_all_blocks): > (try_thread_blocks): > (pass_early_thread_jumps::execute): Presumably this is preparing for addressing some of the issues we discussed via email a little while ago -- ie to avoid having the backwards threader mucking up loops and allowing the loop optimizer to make more decisions about things like peeling because the loop optimizer has a cost model for this stuff whereas the jump threaders to not. OK. jeff
On 9/3/21 5:04 PM, Jeff Law wrote: > > > On 9/3/2021 7:57 AM, Aldy Hernandez wrote: >> The registry's thread_through_all_blocks() has a may_peel_loop_headers >> argument. When refactoring the backward threader code, I removed this >> argument for the local passthru method because it was always TRUE. This >> may not necessarily be true in the future, if the backward threader is >> called from another context. This patch removes the default definition, >> in favor of an argument that is exactly the same as the identically >> named function in tree-ssa-threadupdate.c. I think this also makes it >> less confusing when looking at both methods across the source base. >> >> Tested on x86-64 Linux. >> >> OK? >> >> gcc/ChangeLog: >> >> * tree-ssa-threadbackward.c >> (back_threader::thread_through_all_blocks): >> (back_threader_registry::thread_through_all_blocks): >> (try_thread_blocks): >> (pass_early_thread_jumps::execute): > Presumably this is preparing for addressing some of the issues we > discussed via email a little while ago -- ie to avoid having the > backwards threader mucking up loops and allowing the loop optimizer to > make more decisions about things like peeling because the loop optimizer > has a cost model for this stuff whereas the jump threaders to not. Yes. Currently the forward and the backward threaders have different profitability models. The backward threader has one function to control it all, whereas the forward threader has ad-hoc code sprinkled throughout. I would like to combine both models, though I'm not sure whether I'll be able to get everything done in this release. Also, both threaders have different block duplication code. The forward threader's seems more permissive than the other, so this should also be combined. See for instance, EDGE_FSM_THREAD versus EDGE_*BLOCK. Right now I'm trying to replace the forward threader with an enhanced version of the current backward threader, and I'm running into issues because the backward threader is handcuffed as to what it can do (see profitable_path_p). It will take some untangling to figure out what to allow it to do, without it going ape-shit and threading iterations of loops, etc. Anywhoo...it's a work in progress. I have upcoming patches to both the forward threader clients (to rid them of their dependence on VRP/evrp), and to the backward threader to enhance it further so it can get pretty much everything the forward threader currently gets. Gawwwd, this is so convoluted. I hope most of it made sense. Aldy
diff --git a/gcc/tree-ssa-threadbackward.c b/gcc/tree-ssa-threadbackward.c index b9a0d9a60ad..2fa22f8e328 100644 --- a/gcc/tree-ssa-threadbackward.c +++ b/gcc/tree-ssa-threadbackward.c @@ -53,7 +53,7 @@ class back_threader_registry public: back_threader_registry (int max_allowable_paths); bool register_path (const vec<basic_block> &, edge taken); - bool thread_through_all_blocks (); + bool thread_through_all_blocks (bool may_peel_loop_headers); private: jump_thread_path_registry m_lowlevel_registry; const int m_max_allowable_paths; @@ -80,7 +80,7 @@ public: back_threader (bool speed_p); ~back_threader (); void maybe_thread_block (basic_block bb); - bool thread_through_all_blocks (); + bool thread_through_all_blocks (bool may_peel_loop_headers); private: void find_paths (basic_block bb, tree name); void maybe_register_path (edge taken_edge); @@ -497,9 +497,9 @@ back_threader::maybe_thread_block (basic_block bb) // Perform the actual jump threading for the all queued paths. bool -back_threader::thread_through_all_blocks () +back_threader::thread_through_all_blocks (bool may_peel_loop_headers) { - return m_registry.thread_through_all_blocks (); + return m_registry.thread_through_all_blocks (may_peel_loop_headers); } // Dump a sequence of BBs through the CFG. @@ -553,9 +553,9 @@ back_threader_registry::back_threader_registry (int max_allowable_paths) } bool -back_threader_registry::thread_through_all_blocks () +back_threader_registry::thread_through_all_blocks (bool may_peel_loop_headers) { - return m_lowlevel_registry.thread_through_all_blocks (true); + return m_lowlevel_registry.thread_through_all_blocks (may_peel_loop_headers); } /* Examine jump threading path PATH and return TRUE if it is profitable to @@ -947,7 +947,7 @@ try_thread_blocks (function *fun) if (EDGE_COUNT (bb->succs) > 1) threader.maybe_thread_block (bb); } - return threader.thread_through_all_blocks (); + return threader.thread_through_all_blocks (/*peel_loop_headers=*/true); } unsigned int @@ -1016,7 +1016,7 @@ pass_early_thread_jumps::execute (function *fun) if (EDGE_COUNT (bb->succs) > 1) threader.maybe_thread_block (bb); } - threader.thread_through_all_blocks (); + threader.thread_through_all_blocks (/*peel_loop_headers=*/true); loop_optimizer_finalize (); return 0;