[PATCH][RFC] Teach GIMPLE FE to build proper CFG + SSA (+ loops)
This makes an attempt at fixing the most annoying parts of the GIMPLE FE unit testing - the lack of proper CFG preservation and hoops you need to jump through to make the CFG and SSA builders happy. Due to this the __GIMPLE specifiers takes two new flags, "cfg" for GIMPLE-with-a-CFG and "ssa" for GIMPLE-with-a-CFG-and-SSA. When there is a CFG you need to start basic-block boundaries with __BB(index): where 'index' is the basic-block index. That implicitely defines a label __BBindex for use in goto stmts and friends (but doesn't actually add a GIMPLE_LABEL). The parsing code isn't defensive right now so you need to watch out to not use index 0 or 1 (entry and exit block) which are only implicitely present. As a proof of concept I added one BB annotation - loop_header(num) where "num" is the loop number. This means you can now also have loop structures preserved (to some extent - the loop tree is not explicitely represented nor are loop fathers, both are re-built via fixup). I've not yet adjusted -gimple dumping. I've adjusted all testcases I could find. The CFG build inside the frontend is a bit awkward (spread out...), likewise some functionality is asking for split-out. This is mainly a RFC for whether you think this is forward looking enough. Future enhancements would include EH and abnormal edges via stmt annotations: __BB(2): foo () goto __BB3 __EDGE_EH; // EH edge to BB3 __BB(3): setjmp () goto __BB4 __EDGE_AB; // abnormal edge to BB4 __BB(4): _1 = _5 / 0. __NOTHROW; // gimple_nothrow_p is true _3 = _2 / _4 goto __BB5 __EDGE_EH; // EH edge to BB5 etc. extra edge flags: goto __BB5 __EDGE_EXECUTABLE; I guess now is the time for bike-shedding. Richard. 2019-03-12 Richard Biener c/ * c-tree.h (enum c_declspec_il): New. (struct c_declspecs): Merge gimple_p and rtl_p into declspec_il enum bitfield. * c-parser.c (c_parser_declaration_or_fndef): Adjust accordingly. Pass start pass and declspec_il to c_parser_parse_gimple_body. (c_parser_declspecs): Adjust. * gimple-parser.c: Include cfg.h, cfghooks.h, cfganal.h, tree-cfg.h, gimple-iterator.h, cfgloop.h, tree-phinodes.h, tree-into-ssa.h and bitmap.h. (struct gimple_parser_edge): New. (edges, current_bb): New globals. (c_parser_gimple_parse_bb_spec): New helper. (c_parser_parse_gimple_body): Get start pass and IL specification. Initialize SSA and CFG. (c_parser_gimple_compound_statement): Handle CFG build. (c_parser_gimple_statement): Change intermittend __PHI internal function arg. (c_parser_gimple_or_rtl_pass_list): Handle ssa, cfg flags. (c_parser_gimple_goto_stmt): Record edges to build. (c_parser_gimple_if_stmt): Likewise. * gimple-parser.h (c_parser_parse_gimple_body): Adjust. (c_parser_gimple_or_rtl_pass_list): Likewise. * gcc.dg/gimplefe-13.c: Adjust. ... Index: gcc/c/c-parser.c === --- gcc/c/c-parser.c(revision 269604) +++ gcc/c/c-parser.c(working copy) @@ -2324,19 +2324,9 @@ c_parser_declaration_or_fndef (c_parser DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus = c_parser_peek_token (parser)->location; - /* If the definition was marked with __GIMPLE then parse the - function body as GIMPLE. */ - if (specs->gimple_p) - { - cfun->pass_startwith = specs->gimple_or_rtl_pass; - bool saved = in_late_binary_op; - in_late_binary_op = true; - c_parser_parse_gimple_body (parser); - in_late_binary_op = saved; - } - /* Similarly, if it was marked with __RTL, use the RTL parser now, + /* If the definition was marked with __RTL, use the RTL parser now, consuming the function body. */ - else if (specs->rtl_p) + if (specs->declspec_il == cdil_rtl) { c_parser_parse_rtl_body (parser, specs->gimple_or_rtl_pass); @@ -2350,6 +2340,16 @@ c_parser_declaration_or_fndef (c_parser finish_function (); return; } + /* If the definition was marked with __GIMPLE then parse the + function body as GIMPLE. */ + else if (specs->declspec_il != cdil_none) + { + bool saved = in_late_binary_op; + in_late_binary_op = true; + c_parser_parse_gimple_body (parser, specs->gimple_or_rtl_pass, + specs->declspec_il); + in_late_binary_op = saved; + } else fnbody = c_parser_compound_statement (parser); tree fndecl = current_function_decl; @@ -2372,8 +2372,8 @@ c_parser_declaration_or_fndef (c_parser add_stmt (fnbody); finish_function (); } - /* Get rid of the empty stmt list for GIMPLE. */ - if (specs->gimple_p) + /* Get rid of the empty stmt list fo
Re: [PATCH][RFC] Teach GIMPLE FE to build proper CFG + SSA (+ loops)
On Wed, Mar 13, 2019 at 3:58 AM Richard Biener wrote: > > > This makes an attempt at fixing the most annoying parts of the GIMPLE > FE unit testing - the lack of proper CFG preservation and hoops you > need to jump through to make the CFG and SSA builders happy. > > Due to this the __GIMPLE specifiers takes two new flags, "cfg" > for GIMPLE-with-a-CFG and "ssa" for GIMPLE-with-a-CFG-and-SSA. > When there is a CFG you need to start basic-block boundaries with > > __BB(index): > > where 'index' is the basic-block index. That implicitely defines > a label __BBindex for use in goto stmts and friends (but doesn't > actually add a GIMPLE_LABEL). > > The parsing code isn't defensive right now so you need to watch > out to not use index 0 or 1 (entry and exit block) which are > only implicitely present. > > As a proof of concept I added one BB annotation - loop_header(num) > where "num" is the loop number. This means you can now also > have loop structures preserved (to some extent - the loop tree is > not explicitely represented nor are loop fathers, both are re-built > via fixup). > > I've not yet adjusted -gimple dumping. > > I've adjusted all testcases I could find. > > The CFG build inside the frontend is a bit awkward (spread out...), > likewise some functionality is asking for split-out. > > This is mainly a RFC for whether you think this is forward looking > enough. Future enhancements would include EH and abnormal edges > via stmt annotations: Thanks very much for doing this. Do we have a centralized wiki/document on the formal definition? It would be very helpful even it's still evolving, otherwise we would need to dig into messages for fragmented clues. Thanks, bin > > __BB(2): > foo () goto __BB3 __EDGE_EH; // EH edge to BB3 > > __BB(3): > setjmp () goto __BB4 __EDGE_AB; // abnormal edge to BB4 > > __BB(4): > _1 = _5 / 0. __NOTHROW; // gimple_nothrow_p is true > _3 = _2 / _4 goto __BB5 __EDGE_EH; // EH edge to BB5 > > etc. extra edge flags: > > goto __BB5 __EDGE_EXECUTABLE; > > I guess now is the time for bike-shedding. > > Richard. > > 2019-03-12 Richard Biener > > c/ > * c-tree.h (enum c_declspec_il): New. > (struct c_declspecs): Merge gimple_p and rtl_p into declspec_il > enum bitfield. > * c-parser.c (c_parser_declaration_or_fndef): Adjust accordingly. > Pass start pass and declspec_il to c_parser_parse_gimple_body. > (c_parser_declspecs): Adjust. > * gimple-parser.c: Include cfg.h, cfghooks.h, cfganal.h, tree-cfg.h, > gimple-iterator.h, cfgloop.h, tree-phinodes.h, tree-into-ssa.h > and bitmap.h. > (struct gimple_parser_edge): New. > (edges, current_bb): New globals. > (c_parser_gimple_parse_bb_spec): New helper. > (c_parser_parse_gimple_body): Get start pass and IL specification. > Initialize SSA and CFG. > (c_parser_gimple_compound_statement): Handle CFG build. > (c_parser_gimple_statement): Change intermittend __PHI internal > function arg. > (c_parser_gimple_or_rtl_pass_list): Handle ssa, cfg flags. > (c_parser_gimple_goto_stmt): Record edges to build. > (c_parser_gimple_if_stmt): Likewise. > * gimple-parser.h (c_parser_parse_gimple_body): Adjust. > (c_parser_gimple_or_rtl_pass_list): Likewise. > > * gcc.dg/gimplefe-13.c: Adjust. > ... > > Index: gcc/c/c-parser.c > === > --- gcc/c/c-parser.c(revision 269604) > +++ gcc/c/c-parser.c(working copy) > @@ -2324,19 +2324,9 @@ c_parser_declaration_or_fndef (c_parser >DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus > = c_parser_peek_token (parser)->location; > > - /* If the definition was marked with __GIMPLE then parse the > - function body as GIMPLE. */ > - if (specs->gimple_p) > - { > - cfun->pass_startwith = specs->gimple_or_rtl_pass; > - bool saved = in_late_binary_op; > - in_late_binary_op = true; > - c_parser_parse_gimple_body (parser); > - in_late_binary_op = saved; > - } > - /* Similarly, if it was marked with __RTL, use the RTL parser now, > + /* If the definition was marked with __RTL, use the RTL parser now, > consuming the function body. */ > - else if (specs->rtl_p) > + if (specs->declspec_il == cdil_rtl) > { > c_parser_parse_rtl_body (parser, specs->gimple_or_rtl_pass); > > @@ -2350,6 +2340,16 @@ c_parser_declaration_or_fndef (c_parser > finish_function (); > return; > } > + /* If the definition was marked with __GIMPLE then parse the > + function body as GIMPLE. */ > + else if (specs->declspec_il != cdil_none) > + { > + bool saved = in_late_binary_op; > + in_late_binary_op = true; > + c_parser_parse_gimple_body (parser, spe
Re: [PATCH][RFC] Teach GIMPLE FE to build proper CFG + SSA (+ loops)
On Wed, 13 Mar 2019, Bin.Cheng wrote: > On Wed, Mar 13, 2019 at 3:58 AM Richard Biener wrote: > > > > > > This makes an attempt at fixing the most annoying parts of the GIMPLE > > FE unit testing - the lack of proper CFG preservation and hoops you > > need to jump through to make the CFG and SSA builders happy. > > > > Due to this the __GIMPLE specifiers takes two new flags, "cfg" > > for GIMPLE-with-a-CFG and "ssa" for GIMPLE-with-a-CFG-and-SSA. > > When there is a CFG you need to start basic-block boundaries with > > > > __BB(index): > > > > where 'index' is the basic-block index. That implicitely defines > > a label __BBindex for use in goto stmts and friends (but doesn't > > actually add a GIMPLE_LABEL). > > > > The parsing code isn't defensive right now so you need to watch > > out to not use index 0 or 1 (entry and exit block) which are > > only implicitely present. > > > > As a proof of concept I added one BB annotation - loop_header(num) > > where "num" is the loop number. This means you can now also > > have loop structures preserved (to some extent - the loop tree is > > not explicitely represented nor are loop fathers, both are re-built > > via fixup). > > > > I've not yet adjusted -gimple dumping. > > > > I've adjusted all testcases I could find. > > > > The CFG build inside the frontend is a bit awkward (spread out...), > > likewise some functionality is asking for split-out. > > > > This is mainly a RFC for whether you think this is forward looking > > enough. Future enhancements would include EH and abnormal edges > > via stmt annotations: > Thanks very much for doing this. Do we have a centralized > wiki/document on the formal definition? It would be very helpful even > it's still evolving, otherwise we would need to dig into messages for > fragmented clues. Earlier this week I transformed the old https://gcc.gnu.org/wiki/GimpleFrontEnd page into one documenting the current state but this doesn't yet include any kind of documentation. I suppose special syntax documentation should go into our texinfo docs, I guess into a new section in the internals manual. Currently we only have documentation for the -fgimple switch. Note that I view -gimple dumping as the thing that should give you a clue about expected syntax. I hope to spend a little more time on the patch to make it ready for GCC9. Richard. > Thanks, > bin > > > > __BB(2): > > foo () goto __BB3 __EDGE_EH; // EH edge to BB3 > > > > __BB(3): > > setjmp () goto __BB4 __EDGE_AB; // abnormal edge to BB4 > > > > __BB(4): > > _1 = _5 / 0. __NOTHROW; // gimple_nothrow_p is true > > _3 = _2 / _4 goto __BB5 __EDGE_EH; // EH edge to BB5 > > > > etc. extra edge flags: > > > > goto __BB5 __EDGE_EXECUTABLE; > > > > I guess now is the time for bike-shedding. > > > > Richard. > > > > 2019-03-12 Richard Biener > > > > c/ > > * c-tree.h (enum c_declspec_il): New. > > (struct c_declspecs): Merge gimple_p and rtl_p into declspec_il > > enum bitfield. > > * c-parser.c (c_parser_declaration_or_fndef): Adjust accordingly. > > Pass start pass and declspec_il to c_parser_parse_gimple_body. > > (c_parser_declspecs): Adjust. > > * gimple-parser.c: Include cfg.h, cfghooks.h, cfganal.h, tree-cfg.h, > > gimple-iterator.h, cfgloop.h, tree-phinodes.h, tree-into-ssa.h > > and bitmap.h. > > (struct gimple_parser_edge): New. > > (edges, current_bb): New globals. > > (c_parser_gimple_parse_bb_spec): New helper. > > (c_parser_parse_gimple_body): Get start pass and IL specification. > > Initialize SSA and CFG. > > (c_parser_gimple_compound_statement): Handle CFG build. > > (c_parser_gimple_statement): Change intermittend __PHI internal > > function arg. > > (c_parser_gimple_or_rtl_pass_list): Handle ssa, cfg flags. > > (c_parser_gimple_goto_stmt): Record edges to build. > > (c_parser_gimple_if_stmt): Likewise. > > * gimple-parser.h (c_parser_parse_gimple_body): Adjust. > > (c_parser_gimple_or_rtl_pass_list): Likewise. > > > > * gcc.dg/gimplefe-13.c: Adjust. > > ... > > > > Index: gcc/c/c-parser.c > > === > > --- gcc/c/c-parser.c(revision 269604) > > +++ gcc/c/c-parser.c(working copy) > > @@ -2324,19 +2324,9 @@ c_parser_declaration_or_fndef (c_parser > >DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus > > = c_parser_peek_token (parser)->location; > > > > - /* If the definition was marked with __GIMPLE then parse the > > - function body as GIMPLE. */ > > - if (specs->gimple_p) > > - { > > - cfun->pass_startwith = specs->gimple_or_rtl_pass; > > - bool saved = in_late_binary_op; > > - in_late_binary_op = true; > > - c_parser_parse_gimple_body (parser); > > - in_late_binary_op = saved;
Re: [PATCH][RFC] Teach GIMPLE FE to build proper CFG + SSA (+ loops)
On Wed, 13 Mar 2019, Richard Biener wrote: > On Wed, 13 Mar 2019, Bin.Cheng wrote: > > > On Wed, Mar 13, 2019 at 3:58 AM Richard Biener wrote: > > > > > > > > > This makes an attempt at fixing the most annoying parts of the GIMPLE > > > FE unit testing - the lack of proper CFG preservation and hoops you > > > need to jump through to make the CFG and SSA builders happy. > > > > > > Due to this the __GIMPLE specifiers takes two new flags, "cfg" > > > for GIMPLE-with-a-CFG and "ssa" for GIMPLE-with-a-CFG-and-SSA. > > > When there is a CFG you need to start basic-block boundaries with > > > > > > __BB(index): > > > > > > where 'index' is the basic-block index. That implicitely defines > > > a label __BBindex for use in goto stmts and friends (but doesn't > > > actually add a GIMPLE_LABEL). > > > > > > The parsing code isn't defensive right now so you need to watch > > > out to not use index 0 or 1 (entry and exit block) which are > > > only implicitely present. > > > > > > As a proof of concept I added one BB annotation - loop_header(num) > > > where "num" is the loop number. This means you can now also > > > have loop structures preserved (to some extent - the loop tree is > > > not explicitely represented nor are loop fathers, both are re-built > > > via fixup). > > > > > > I've not yet adjusted -gimple dumping. > > > > > > I've adjusted all testcases I could find. > > > > > > The CFG build inside the frontend is a bit awkward (spread out...), > > > likewise some functionality is asking for split-out. > > > > > > This is mainly a RFC for whether you think this is forward looking > > > enough. Future enhancements would include EH and abnormal edges > > > via stmt annotations: > > Thanks very much for doing this. Do we have a centralized > > wiki/document on the formal definition? It would be very helpful even > > it's still evolving, otherwise we would need to dig into messages for > > fragmented clues. > > Earlier this week I transformed the old > https://gcc.gnu.org/wiki/GimpleFrontEnd page into one documenting > the current state but this doesn't yet include any kind of documentation. > > I suppose special syntax documentation should go into our texinfo > docs, I guess into a new section in the internals manual. Currently > we only have documentation for the -fgimple switch. > > Note that I view -gimple dumping as the thing that should give you > a clue about expected syntax. I hope to spend a little more time > on the patch to make it ready for GCC9. I've settled on another change, forcing explicit gotos for fallthru edges to the next block. Otherwise I've cleaned up the patch and removed no longer necessary support for IFN_PHI from the middle-end. During cleanup I introduced a gimple_parser class to hold the extra parsing state and I added some extra error handling. Bootstrapped on x86_64-unknown-linux-gnu, testing is still in progress. I will commit this to make testcase extraction to GIMPLE more feasible for GCC 9+ (we could even backport the GIMPLE FE changes to GCC 8 if needed I guess). At least it will give me motivation to do further required changes when I run into the next big LTO testcase running into a loop optimizer issue... (non-loop passes should work fine already). Richard. >From 5cbec540caa8e86f2167ac980159dbe6933717c4 Mon Sep 17 00:00:00 2001 From: Richard Guenther Date: Wed, 13 Mar 2019 13:58:10 +0100 Subject: [PATCH] gimplefe-cfg-ssa 2019-03-12 Richard Biener c/ * c-tree.h (enum c_declspec_il): New. (struct c_declspecs): Merge gimple_p and rtl_p into declspec_il enum bitfield. * c-parser.c (c_parser_declaration_or_fndef): Adjust accordingly. Pass start pass and declspec_il to c_parser_parse_gimple_body. (c_parser_declspecs): Adjust. * gimple-parser.c: Include cfg.h, cfghooks.h, cfganal.h, tree-cfg.h, gimple-iterator.h, cfgloop.h, tree-phinodes.h, tree-into-ssa.h and bitmap.h. (struct gimple_parser): New. (gimple_parser::push_edge): New method. (c_parser_gimple_parse_bb_spec): New helper. (c_parser_parse_gimple_body): Get start pass and IL specification. Initialize SSA and CFG. (c_parser_gimple_compound_statement): Handle CFG and SSA build. Build a gimple_parser parsing state and pass it along. (c_parser_gimple_statement): Change intermittend __PHI internal function argument for the edge. (c_parser_gimple_or_rtl_pass_list): Handle ssa, cfg flags. (c_parser_gimple_goto_stmt): Record edges to build. (c_parser_gimple_if_stmt): Likewise. * gimple-parser.h (c_parser_parse_gimple_body): Adjust. (c_parser_gimple_or_rtl_pass_list): Likewise. * gimple-pretty-print.c: Include cfgloop.h. (dump_gimple_phi): Adjust. (dump_gimple_bb_header): Dump loop header for GIMPLE. (pp_cfg_jump): Adjust. (dump_implicit_edges): Dump fallthru to next block fo
Re: [PATCH][RFC] Teach GIMPLE FE to build proper CFG + SSA (+ loops)
On 3/14/19 4:43 AM, Richard Biener wrote: > On Wed, 13 Mar 2019, Richard Biener wrote: > >> On Wed, 13 Mar 2019, Bin.Cheng wrote: >> >>> On Wed, Mar 13, 2019 at 3:58 AM Richard Biener wrote: This makes an attempt at fixing the most annoying parts of the GIMPLE FE unit testing - the lack of proper CFG preservation and hoops you need to jump through to make the CFG and SSA builders happy. Due to this the __GIMPLE specifiers takes two new flags, "cfg" for GIMPLE-with-a-CFG and "ssa" for GIMPLE-with-a-CFG-and-SSA. When there is a CFG you need to start basic-block boundaries with __BB(index): where 'index' is the basic-block index. That implicitely defines a label __BBindex for use in goto stmts and friends (but doesn't actually add a GIMPLE_LABEL). The parsing code isn't defensive right now so you need to watch out to not use index 0 or 1 (entry and exit block) which are only implicitely present. As a proof of concept I added one BB annotation - loop_header(num) where "num" is the loop number. This means you can now also have loop structures preserved (to some extent - the loop tree is not explicitely represented nor are loop fathers, both are re-built via fixup). I've not yet adjusted -gimple dumping. I've adjusted all testcases I could find. The CFG build inside the frontend is a bit awkward (spread out...), likewise some functionality is asking for split-out. This is mainly a RFC for whether you think this is forward looking enough. Future enhancements would include EH and abnormal edges via stmt annotations: >>> Thanks very much for doing this. Do we have a centralized >>> wiki/document on the formal definition? It would be very helpful even >>> it's still evolving, otherwise we would need to dig into messages for >>> fragmented clues. >> >> Earlier this week I transformed the old >> https://gcc.gnu.org/wiki/GimpleFrontEnd page into one documenting >> the current state but this doesn't yet include any kind of documentation. >> >> I suppose special syntax documentation should go into our texinfo >> docs, I guess into a new section in the internals manual. Currently >> we only have documentation for the -fgimple switch. >> >> Note that I view -gimple dumping as the thing that should give you >> a clue about expected syntax. I hope to spend a little more time >> on the patch to make it ready for GCC9. > > I've settled on another change, forcing explicit gotos for fallthru > edges to the next block. > > Otherwise I've cleaned up the patch and removed no longer necessary > support for IFN_PHI from the middle-end. During cleanup I introduced > a gimple_parser class to hold the extra parsing state and I added > some extra error handling. > > Bootstrapped on x86_64-unknown-linux-gnu, testing is still in progress. > > I will commit this to make testcase extraction to GIMPLE more feasible > for GCC 9+ (we could even backport the GIMPLE FE changes to GCC 8 if > needed I guess). At least it will give me motivation to do further > required changes when I run into the next big LTO testcase running > into a loop optimizer issue... (non-loop passes should work fine > already). If it makes building testcases easier, then I'm all for it ;-) jeff