| Message ID | CAFiYyc1tjbnt7SB_gp24zyERT4mX3dmRrgJ2CthHnZe_eAgMhQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 03/29/2016 02:10 PM, Richard Biener wrote: > On Tue, Mar 29, 2016 at 1:42 PM, Martin Liška <mliska@suse.cz> wrote: >> Hello. >> >> The problem with the original patch is that I'm forced to produce >> an empty BB to produce true/false edge needed for the 'index' check: >> >> /home/marxin/Programming/testhsa/run_tests/012-switch/switch-5.c:28:9: error: true/false edge after a non-GIMPLE_COND in bb 4 >> /home/marxin/Programming/testhsa/run_tests/012-switch/switch-5.c:28:9: internal compiler error: verify_flow_info failed >> 0x93121a verify_flow_info() >> ../../gcc/cfghooks.c:260 >> 0xd5ae4e execute_function_todo >> ../../gcc/passes.c:1971 >> 0xd59ea6 do_per_function >> ../../gcc/passes.c:1645 >> 0xd5afc2 execute_todo >> ../../gcc/passes.c:2011 >> >> It would nicer to not produce empty block for that purpose, but the question >> is if the change is acceptable during the stage4? > > Hmm, why don't we short-cut things earlier in execute_one_pass where > we handle TODO_discard_function? > That is, sth like > > Index: gcc/passes.c > =================================================================== > --- gcc/passes.c (revision 234453) > +++ gcc/passes.c (working copy) > @@ -2334,6 +2334,33 @@ execute_one_pass (opt_pass *pass) > > /* Do it! */ > todo_after = pass->execute (cfun); > + > + if (todo_after & TODO_discard_function) > + { > + pass_fini_dump_file (pass); > + > + gcc_assert (cfun); > + /* As cgraph_node::release_body expects release dominators info, > + we have to release it. */ > + if (dom_info_available_p (CDI_DOMINATORS)) > + free_dominance_info (CDI_DOMINATORS); > + > + if (dom_info_available_p (CDI_POST_DOMINATORS)) > + free_dominance_info (CDI_POST_DOMINATORS); > + > + tree fn = cfun->decl; > + pop_cfun (); > + gcc_assert (!cfun); > + cgraph_node::get (fn)->release_body (); > + > + current_pass = NULL; > + redirect_edge_var_map_empty (); > + > + ggc_collect (); > + > + return true; > + } > + > do_per_function (clear_last_verified, NULL); > > /* Stop timevar. */ > @@ -2373,23 +2400,6 @@ execute_one_pass (opt_pass *pass) > current_pass = NULL; > redirect_edge_var_map_empty (); > > - if (todo_after & TODO_discard_function) > - { > - gcc_assert (cfun); > - /* As cgraph_node::release_body expects release dominators info, > - we have to release it. */ > - if (dom_info_available_p (CDI_DOMINATORS)) > - free_dominance_info (CDI_DOMINATORS); > - > - if (dom_info_available_p (CDI_POST_DOMINATORS)) > - free_dominance_info (CDI_POST_DOMINATORS); > - > - tree fn = cfun->decl; > - pop_cfun (); > - gcc_assert (!cfun); > - cgraph_node::get (fn)->release_body (); > - } > - > /* Signal this is a suitable GC collection point. */ > if (!((todo_after | pass->todo_flags_finish) & TODO_do_not_ggc_collect)) > ggc_collect (); > > Hello Richi. Thanks for coming with the cleaner version of the patch. I've incorporated that patch and reg&bootstrap is running. Installable as soon as it finishes? Thanks, Martin >> Thanks, >> Martin
On Tue, Mar 29, 2016 at 3:07 PM, Martin Liška <mliska@suse.cz> wrote: > On 03/29/2016 02:10 PM, Richard Biener wrote: >> On Tue, Mar 29, 2016 at 1:42 PM, Martin Liška <mliska@suse.cz> wrote: >>> Hello. >>> >>> The problem with the original patch is that I'm forced to produce >>> an empty BB to produce true/false edge needed for the 'index' check: >>> >>> /home/marxin/Programming/testhsa/run_tests/012-switch/switch-5.c:28:9: error: true/false edge after a non-GIMPLE_COND in bb 4 >>> /home/marxin/Programming/testhsa/run_tests/012-switch/switch-5.c:28:9: internal compiler error: verify_flow_info failed >>> 0x93121a verify_flow_info() >>> ../../gcc/cfghooks.c:260 >>> 0xd5ae4e execute_function_todo >>> ../../gcc/passes.c:1971 >>> 0xd59ea6 do_per_function >>> ../../gcc/passes.c:1645 >>> 0xd5afc2 execute_todo >>> ../../gcc/passes.c:2011 >>> >>> It would nicer to not produce empty block for that purpose, but the question >>> is if the change is acceptable during the stage4? >> >> Hmm, why don't we short-cut things earlier in execute_one_pass where >> we handle TODO_discard_function? >> That is, sth like >> >> Index: gcc/passes.c >> =================================================================== >> --- gcc/passes.c (revision 234453) >> +++ gcc/passes.c (working copy) >> @@ -2334,6 +2334,33 @@ execute_one_pass (opt_pass *pass) >> >> /* Do it! */ >> todo_after = pass->execute (cfun); >> + >> + if (todo_after & TODO_discard_function) >> + { >> + pass_fini_dump_file (pass); >> + >> + gcc_assert (cfun); >> + /* As cgraph_node::release_body expects release dominators info, >> + we have to release it. */ >> + if (dom_info_available_p (CDI_DOMINATORS)) >> + free_dominance_info (CDI_DOMINATORS); >> + >> + if (dom_info_available_p (CDI_POST_DOMINATORS)) >> + free_dominance_info (CDI_POST_DOMINATORS); >> + >> + tree fn = cfun->decl; >> + pop_cfun (); >> + gcc_assert (!cfun); >> + cgraph_node::get (fn)->release_body (); >> + >> + current_pass = NULL; >> + redirect_edge_var_map_empty (); >> + >> + ggc_collect (); >> + >> + return true; >> + } >> + >> do_per_function (clear_last_verified, NULL); >> >> /* Stop timevar. */ >> @@ -2373,23 +2400,6 @@ execute_one_pass (opt_pass *pass) >> current_pass = NULL; >> redirect_edge_var_map_empty (); >> >> - if (todo_after & TODO_discard_function) >> - { >> - gcc_assert (cfun); >> - /* As cgraph_node::release_body expects release dominators info, >> - we have to release it. */ >> - if (dom_info_available_p (CDI_DOMINATORS)) >> - free_dominance_info (CDI_DOMINATORS); >> - >> - if (dom_info_available_p (CDI_POST_DOMINATORS)) >> - free_dominance_info (CDI_POST_DOMINATORS); >> - >> - tree fn = cfun->decl; >> - pop_cfun (); >> - gcc_assert (!cfun); >> - cgraph_node::get (fn)->release_body (); >> - } >> - >> /* Signal this is a suitable GC collection point. */ >> if (!((todo_after | pass->todo_flags_finish) & TODO_do_not_ggc_collect)) >> ggc_collect (); >> >> > > Hello Richi. > > Thanks for coming with the cleaner version of the patch. > I've incorporated that patch and reg&bootstrap is running. > > Installable as soon as it finishes? Yes. Richard. > Thanks, > Martin > >>> Thanks, >>> Martin >
Index: gcc/passes.c =================================================================== --- gcc/passes.c (revision 234453) +++ gcc/passes.c (working copy) @@ -2334,6 +2334,33 @@ execute_one_pass (opt_pass *pass) /* Do it! */ todo_after = pass->execute (cfun); + + if (todo_after & TODO_discard_function) + { + pass_fini_dump_file (pass); + + gcc_assert (cfun); + /* As cgraph_node::release_body expects release dominators info, + we have to release it. */ + if (dom_info_available_p (CDI_DOMINATORS)) + free_dominance_info (CDI_DOMINATORS); + + if (dom_info_available_p (CDI_POST_DOMINATORS)) + free_dominance_info (CDI_POST_DOMINATORS); + + tree fn = cfun->decl; + pop_cfun (); + gcc_assert (!cfun); + cgraph_node::get (fn)->release_body (); + + current_pass = NULL; + redirect_edge_var_map_empty (); + + ggc_collect (); + + return true; + } + do_per_function (clear_last_verified, NULL); /* Stop timevar. */ @@ -2373,23 +2400,6 @@ execute_one_pass (opt_pass *pass) current_pass = NULL; redirect_edge_var_map_empty (); - if (todo_after & TODO_discard_function) - { - gcc_assert (cfun); - /* As cgraph_node::release_body expects release dominators info, - we have to release it. */ - if (dom_info_available_p (CDI_DOMINATORS)) - free_dominance_info (CDI_DOMINATORS); - - if (dom_info_available_p (CDI_POST_DOMINATORS)) - free_dominance_info (CDI_POST_DOMINATORS); - - tree fn = cfun->decl; - pop_cfun (); - gcc_assert (!cfun); - cgraph_node::get (fn)->release_body (); - } - /* Signal this is a suitable GC collection point. */ if (!((todo_after | pass->todo_flags_finish) & TODO_do_not_ggc_collect)) ggc_collect ();