Re: [PATCH] [GIMPLE FE] allow to unit test loop passes
Richard Biener writes: > The following arranges for the GIMPLE frontend to parse an > additional loops(...) specification, currently consisting of > 'normal' and 'lcssa'. The intent is to allow unit testing > of passes inside the GIMPLE loop optimization pipeline where > we keep the IL in loop-closed SSA and have SCEV initialized. > > The GIMPLE parser side is only half of the story - the rest > of the patch makes sure we do not destroy loop state when > moving the IL through the skipped portion of the pass pipeline > which shows we are not careful with IPA passes here and those > tend to call loop_optimizer_init which destroys state. The > patch arranges for IPA passes to honor fn->pass_startwith and > if set, refrain from considering the function. > > Since the SCEV state is global we cannot initialize it during > GIMPLE parsing but we have to arrange for that to happen before > the pass we want to start with. The patch heuristically > enables the loop-init pass based on the fact whether the IL > is in loop-closed SSA state and makes that aware of GIMPLE > testcases already arriving with properly initialized loops. > > That's all quite some hacks but I think it's worth the ability > to unit test loop passes. > > I've tried to do this before but PR104931 now triggered me to > try again (I have still to see whether that's enough to make > a GIMPLE testcase trigger that bug). I've skipped existing > GIMPLE testcases for -flto since when starting after IPA > using LTO doesn't make much sense and my IPA mangling ends up > crashing in the LTO writing. There's possibly some better > way to "hide" the late to be started functions from IPA > (but we would still need to stream the body). > > Bootstrapped and tested on x86_64-unknown-linux-gnu. Don't think I'm really qualified to comment, but it seems OK to me FWIW. Given that you've already found 4 places that need: (DECL_STRUCT_FUNCTION (node->decl) && DECL_STRUCT_FUNCTION (node->decl)->pass_startwith) I guess it might be worth having a helper for it. Thanks, Richard > > Any comments? > > Thanks, > Richard. > > 2022-03-17 Richard Biener > > gcc/c/ > * c-tree.h (c_declspecs::loops_state): Add. > * c-parser.cc (c_parser_declaration_or_fndef): Pass down > loops_state to c_parser_parse_gimple_body. > * gimple-parser.h (c_parser_parse_gimple_body): Adjust. > * gimple-parser.cc (c_parser_parse_gimple_body): Get > and initialize loops state according to specification. > (c_parser_gimple_or_rtl_pass_list): Parse loops[(spec...)] > with supported spec 'normal' and 'lcssa'. > > gcc/ > * passes.cc (should_skip_pass_p): When in LC SSA do not > skip loopinit. > * tree-ssa-loop.cc (pass_tree_loop_init::execute): Handle > functions already in LC SSA. > * ipa-cp.cc (ipcp_cloning_candidate_p): Skip functions > with pending startwith pass. > * ipa-fnsummary.c (ipa_fn_summary_generate): Likewise. > * ipa-inline.cc (inline_small_functions): Likewise. > (ipa_inline): Likewise. > * ipa-modref.cc (modref_generate): Likewise. > > gcc/testsuite/ > * gcc.dg/gimplefe-50.c: New testcase. > * gcc.dg/torture/pr89595.c: Skip -flto. > * gcc.dg/vect/bb-slp-48.c: Likewise. > * gcc.dg/vect/slp-reduc-10a.c: Likewise. > * gcc.dg/vect/slp-reduc-10b.c: Likewise. > * gcc.dg/vect/slp-reduc-10c.c: Likewise. > * gcc.dg/vect/slp-reduc-10d.c: Likewise. > * gcc.dg/vect/slp-reduc-10e.c: Likewise. > * gcc.dg/vect/vect-cond-arith-2.c: Likewise. > --- > gcc/c/c-parser.cc | 1 + > gcc/c/c-tree.h| 3 ++ > gcc/c/gimple-parser.cc| 37 +- > gcc/c/gimple-parser.h | 1 + > gcc/ipa-cp.cc | 4 +- > gcc/ipa-fnsummary.cc | 4 +- > gcc/ipa-inline.cc | 8 +++- > gcc/ipa-modref.cc | 2 +- > gcc/passes.cc | 7 +++ > gcc/testsuite/gcc.dg/gimplefe-50.c| 48 +++ > gcc/testsuite/gcc.dg/torture/pr89595.c| 1 + > gcc/testsuite/gcc.dg/vect/bb-slp-48.c | 1 + > gcc/testsuite/gcc.dg/vect/slp-reduc-10a.c | 1 + > gcc/testsuite/gcc.dg/vect/slp-reduc-10b.c | 1 + > gcc/testsuite/gcc.dg/vect/slp-reduc-10c.c | 1 + > gcc/testsuite/gcc.dg/vect/slp-reduc-10d.c | 1 + > gcc/testsuite/gcc.dg/vect/slp-reduc-10e.c | 1 + > gcc/testsuite/gcc.dg/vect/vect-cond-arith-2.c | 1 + > gcc/tree-ssa-loop.cc | 11 +++-- > 19 files changed, 125 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/gimplefe-50.c > > diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc > index 129dd727ef3..80091d81bb4 100644 > --- a/gcc/c/c-parser.cc > +++ b/gcc/c/c-parser.cc > @@ -2537,6 +2537,7 @@
[PATCH] [GIMPLE FE] allow to unit test loop passes
The following arranges for the GIMPLE frontend to parse an additional loops(...) specification, currently consisting of 'normal' and 'lcssa'. The intent is to allow unit testing of passes inside the GIMPLE loop optimization pipeline where we keep the IL in loop-closed SSA and have SCEV initialized. The GIMPLE parser side is only half of the story - the rest of the patch makes sure we do not destroy loop state when moving the IL through the skipped portion of the pass pipeline which shows we are not careful with IPA passes here and those tend to call loop_optimizer_init which destroys state. The patch arranges for IPA passes to honor fn->pass_startwith and if set, refrain from considering the function. Since the SCEV state is global we cannot initialize it during GIMPLE parsing but we have to arrange for that to happen before the pass we want to start with. The patch heuristically enables the loop-init pass based on the fact whether the IL is in loop-closed SSA state and makes that aware of GIMPLE testcases already arriving with properly initialized loops. That's all quite some hacks but I think it's worth the ability to unit test loop passes. I've tried to do this before but PR104931 now triggered me to try again (I have still to see whether that's enough to make a GIMPLE testcase trigger that bug). I've skipped existing GIMPLE testcases for -flto since when starting after IPA using LTO doesn't make much sense and my IPA mangling ends up crashing in the LTO writing. There's possibly some better way to "hide" the late to be started functions from IPA (but we would still need to stream the body). Bootstrapped and tested on x86_64-unknown-linux-gnu. Any comments? Thanks, Richard. 2022-03-17 Richard Biener gcc/c/ * c-tree.h (c_declspecs::loops_state): Add. * c-parser.cc (c_parser_declaration_or_fndef): Pass down loops_state to c_parser_parse_gimple_body. * gimple-parser.h (c_parser_parse_gimple_body): Adjust. * gimple-parser.cc (c_parser_parse_gimple_body): Get and initialize loops state according to specification. (c_parser_gimple_or_rtl_pass_list): Parse loops[(spec...)] with supported spec 'normal' and 'lcssa'. gcc/ * passes.cc (should_skip_pass_p): When in LC SSA do not skip loopinit. * tree-ssa-loop.cc (pass_tree_loop_init::execute): Handle functions already in LC SSA. * ipa-cp.cc (ipcp_cloning_candidate_p): Skip functions with pending startwith pass. * ipa-fnsummary.c (ipa_fn_summary_generate): Likewise. * ipa-inline.cc (inline_small_functions): Likewise. (ipa_inline): Likewise. * ipa-modref.cc (modref_generate): Likewise. gcc/testsuite/ * gcc.dg/gimplefe-50.c: New testcase. * gcc.dg/torture/pr89595.c: Skip -flto. * gcc.dg/vect/bb-slp-48.c: Likewise. * gcc.dg/vect/slp-reduc-10a.c: Likewise. * gcc.dg/vect/slp-reduc-10b.c: Likewise. * gcc.dg/vect/slp-reduc-10c.c: Likewise. * gcc.dg/vect/slp-reduc-10d.c: Likewise. * gcc.dg/vect/slp-reduc-10e.c: Likewise. * gcc.dg/vect/vect-cond-arith-2.c: Likewise. --- gcc/c/c-parser.cc | 1 + gcc/c/c-tree.h| 3 ++ gcc/c/gimple-parser.cc| 37 +- gcc/c/gimple-parser.h | 1 + gcc/ipa-cp.cc | 4 +- gcc/ipa-fnsummary.cc | 4 +- gcc/ipa-inline.cc | 8 +++- gcc/ipa-modref.cc | 2 +- gcc/passes.cc | 7 +++ gcc/testsuite/gcc.dg/gimplefe-50.c| 48 +++ gcc/testsuite/gcc.dg/torture/pr89595.c| 1 + gcc/testsuite/gcc.dg/vect/bb-slp-48.c | 1 + gcc/testsuite/gcc.dg/vect/slp-reduc-10a.c | 1 + gcc/testsuite/gcc.dg/vect/slp-reduc-10b.c | 1 + gcc/testsuite/gcc.dg/vect/slp-reduc-10c.c | 1 + gcc/testsuite/gcc.dg/vect/slp-reduc-10d.c | 1 + gcc/testsuite/gcc.dg/vect/slp-reduc-10e.c | 1 + gcc/testsuite/gcc.dg/vect/vect-cond-arith-2.c | 1 + gcc/tree-ssa-loop.cc | 11 +++-- 19 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-50.c diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index 129dd727ef3..80091d81bb4 100644 --- a/gcc/c/c-parser.cc +++ b/gcc/c/c-parser.cc @@ -2537,6 +2537,7 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, in_late_binary_op = true; c_parser_parse_gimple_body (parser, specs->gimple_or_rtl_pass, specs->declspec_il, + specs->loops_state, specs->entry_bb_count); in_late_binary_op = saved; } diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index
