Richard Biener <rguent...@suse.de> 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 <rguent...@suse.de>
>
> 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 c70f0ba5ab6..b2a5b75b05f 100644
> --- a/gcc/c/c-tree.h
> +++ b/gcc/c/c-tree.h
> @@ -343,6 +343,9 @@ struct c_declspecs {
> /* Any type specifier keyword used such as "int", not reflecting
> modifiers such as "short", or cts_none if none. */
> ENUM_BITFIELD (c_typespec_keyword) typespec_word : 8;
> + /* The loop specific IL state for __GIMPLE and __RTL, maps to
> + the enum in cfgloop.h. */
> + unsigned loops_state : 8;
> /* The kind of type specifier if one has been seen, ctsk_none
> otherwise. */
> ENUM_BITFIELD (c_typespec_kind) typespec_kind : 4;
> diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
> index d1afd42556c..095684f549f 100644
> --- a/gcc/c/gimple-parser.cc
> +++ b/gcc/c/gimple-parser.cc
> @@ -211,6 +211,7 @@ c_parser_gimple_parse_bb_spec_edge_probability (tree val,
> void
> c_parser_parse_gimple_body (c_parser *cparser, char *gimple_pass,
> enum c_declspec_il cdil,
> + unsigned loops_state,
> profile_count entry_bb_count)
> {
> gimple_parser parser (cparser);
> @@ -240,7 +241,11 @@ c_parser_parse_gimple_body (c_parser *cparser, char
> *gimple_pass,
> mark headers and leave the rest to fixup. */
> set_loops_for_fn (cfun, ggc_cleared_alloc<struct loops> ());
> init_loops_structure (cfun, loops_for_fn (cfun), 1);
> - loops_state_set (cfun,
> LOOPS_NEED_FIXUP|LOOPS_MAY_HAVE_MULTIPLE_LATCHES);
> + if (loops_state == 0)
> + loops_state_set (cfun,
> + LOOPS_NEED_FIXUP|LOOPS_MAY_HAVE_MULTIPLE_LATCHES);
> + else
> + loops_state_set (cfun, LOOPS_NEED_FIXUP|loops_state);
> cfun->curr_properties
> |= PROP_gimple_lcf | PROP_gimple_leh | PROP_cfg | PROP_loops;
> if (cdil == cdil_gimple_ssa)
> @@ -1975,6 +1980,36 @@ c_parser_gimple_or_rtl_pass_list (c_parser *parser,
> c_declspecs *specs)
> if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>"))
> return;
> }
> + else if (! strcmp (op, "loops"))
> + {
> + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN))
> + {
> + c_parser_consume_token (parser);
> + while (c_parser_next_token_is (parser, CPP_NAME))
> + {
> + const char *lop
> + = IDENTIFIER_POINTER (c_parser_peek_token (parser)->value);
> + c_parser_consume_token (parser);
> + if (! strcmp (lop, "normal"))
> + specs->loops_state |= LOOPS_NORMAL;
> + else if (! strcmp (lop, "lcssa"))
> + specs->loops_state |= LOOP_CLOSED_SSA;
> + else
> + {
> + error_at (c_parser_peek_token (parser)->location,
> + "invalid loop state");
> + return;
> + }
> + if (c_parser_next_token_is (parser, CPP_COMMA))
> + c_parser_consume_token (parser);
> + }
> + if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>"))
> + return;
> + }
> + else
> + /* LOOPS_NORMAL */
> + specs->loops_state = LOOPS_NORMAL;
> + }
> else if (specs->declspec_il != cdil_gimple)
> /* Allow only one IL specifier and none on RTL. */
> ;
> diff --git a/gcc/c/gimple-parser.h b/gcc/c/gimple-parser.h
> index 2881a657640..6aed6010343 100644
> --- a/gcc/c/gimple-parser.h
> +++ b/gcc/c/gimple-parser.h
> @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see
> /* Gimple parsing functions. */
> extern void c_parser_parse_gimple_body (c_parser *, char *,
> enum c_declspec_il,
> + unsigned loops_state,
> profile_count);
> extern void c_parser_gimple_or_rtl_pass_list (c_parser *, c_declspecs *);
>
> diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
> index 18047c209a8..348ba878331 100644
> --- a/gcc/ipa-cp.cc
> +++ b/gcc/ipa-cp.cc
> @@ -777,7 +777,9 @@ ipcp_cloning_candidate_p (struct cgraph_node *node)
>
> gcc_checking_assert (node->has_gimple_body_p ());
>
> - if (!opt_for_fn (node->decl, flag_ipa_cp_clone))
> + if (!opt_for_fn (node->decl, flag_ipa_cp_clone)
> + || (DECL_STRUCT_FUNCTION (node->decl)
> + && DECL_STRUCT_FUNCTION (node->decl)->pass_startwith))
> {
> if (dump_file)
> fprintf (dump_file, "Not considering %s for cloning; "
> diff --git a/gcc/ipa-fnsummary.cc b/gcc/ipa-fnsummary.cc
> index 0d7e395c846..10b679bbb27 100644
> --- a/gcc/ipa-fnsummary.cc
> +++ b/gcc/ipa-fnsummary.cc
> @@ -4354,7 +4354,9 @@ ipa_fn_summary_generate (void)
> FOR_EACH_DEFINED_FUNCTION (node)
> if (!node->alias
> && (flag_generate_lto || flag_generate_offload|| flag_wpa
> - || opt_for_fn (node->decl, optimize)))
> + || opt_for_fn (node->decl, optimize))
> + && !(DECL_STRUCT_FUNCTION (node->decl)
> + && DECL_STRUCT_FUNCTION (node->decl)->pass_startwith))
> inline_analyze_function (node);
> }
>
> diff --git a/gcc/ipa-inline.cc b/gcc/ipa-inline.cc
> index f8bb072c422..8352e9f0dce 100644
> --- a/gcc/ipa-inline.cc
> +++ b/gcc/ipa-inline.cc
> @@ -1974,7 +1974,9 @@ inline_small_functions (void)
> {
> if (!node->alias && node->analyzed
> && (node->has_gimple_body_p () || node->thunk)
> - && opt_for_fn (node->decl, optimize))
> + && opt_for_fn (node->decl, optimize)
> + && !(DECL_STRUCT_FUNCTION (node->decl)
> + && DECL_STRUCT_FUNCTION (node->decl)->pass_startwith))
> {
> class ipa_fn_summary *info = ipa_fn_summaries->get (node);
> struct ipa_dfs_info *dfs = (struct ipa_dfs_info *) node->aux;
> @@ -2792,7 +2794,9 @@ ipa_inline (void)
> bool update=false;
>
> if (!opt_for_fn (node->decl, optimize)
> - || !opt_for_fn (node->decl, flag_inline_functions_called_once))
> + || !opt_for_fn (node->decl, flag_inline_functions_called_once)
> + || (DECL_STRUCT_FUNCTION (node->decl)
> + && DECL_STRUCT_FUNCTION (node->decl)->pass_startwith))
> continue;
>
> for (edge = node->callees; edge; edge = next)
> diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc
> index acfd7d80ff8..0f9c98bcc03 100644
> --- a/gcc/ipa-modref.cc
> +++ b/gcc/ipa-modref.cc
> @@ -3370,7 +3370,7 @@ modref_generate (void)
> FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
> {
> function *f = DECL_STRUCT_FUNCTION (node->decl);
> - if (!f)
> + if (!f || f->pass_startwith)
> continue;
> push_cfun (f);
> analyze_function (true);
> diff --git a/gcc/passes.cc b/gcc/passes.cc
> index 00f856e0754..4dbc405cfd4 100644
> --- a/gcc/passes.cc
> +++ b/gcc/passes.cc
> @@ -2510,6 +2510,13 @@ should_skip_pass_p (opt_pass *pass)
> if (strstr (pass->name, "build_cgraph_edges") != NULL)
> return false;
>
> + /* When the function is in LCSSA arrange for SCEV to be initialized
> + via pass_tree_loop_init. */
> + if (loops_for_fn (cfun)
> + && loops_state_satisfies_p (cfun, LOOP_CLOSED_SSA)
> + && strstr (pass->name, "loopinit") != NULL)
> + return false;
> +
> /* Don't skip df init; later RTL passes need it. */
> if (strstr (pass->name, "dfinit") != NULL
> || strstr (pass->name, "dfinish") != NULL)
> diff --git a/gcc/testsuite/gcc.dg/gimplefe-50.c
> b/gcc/testsuite/gcc.dg/gimplefe-50.c
> new file mode 100644
> index 00000000000..3565615e80b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/gimplefe-50.c
> @@ -0,0 +1,48 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fgimple" } */
> +
> +void __GIMPLE (ssa,startwith("ivcanon"),loops(normal,lcssa))
> +foo (int n, double * x)
> +{
> + int i;
> + __SIZETYPE__ _1;
> + __SIZETYPE__ _2;
> + double * _3;
> + double _4;
> +
> + __BB(2):
> + if (n_8(D) > 0)
> + goto __BB5;
> + else
> + goto __BB4;
> +
> + __BB(5):
> + goto __BB3;
> +
> + __BB(3,loop_header(1)):
> + i_14 = __PHI (__BB6: i_11, __BB5: 0);
> + _1 = (__SIZETYPE__) i_14;
> + _2 = _1 * _Literal (__SIZETYPE__) 8u;
> + _3 = x_9(D) + _2;
> + _4 = (double) i_14;
> + __MEM <double> (_3) = _4;
> + i_11 = i_14 + 1;
> + if (n_8(D) > i_11)
> + goto __BB6;
> + else
> + goto __BB4;
> +
> + __BB(6):
> + goto __BB3;
> +
> + __BB(4):
> + return;
> +
> +}
> +
> +int main()
> +{
> + double x[1024];
> + foo (1024, x);
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/torture/pr89595.c
> b/gcc/testsuite/gcc.dg/torture/pr89595.c
> index f45dc98c3f0..65c375f69cb 100644
> --- a/gcc/testsuite/gcc.dg/torture/pr89595.c
> +++ b/gcc/testsuite/gcc.dg/torture/pr89595.c
> @@ -1,4 +1,5 @@
> /* { dg-do run } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> /* { dg-additional-options "-fgimple" } */
>
> int __attribute__((noipa))
> diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-48.c
> b/gcc/testsuite/gcc.dg/vect/bb-slp-48.c
> index dfae6177b21..b54bc580920 100644
> --- a/gcc/testsuite/gcc.dg/vect/bb-slp-48.c
> +++ b/gcc/testsuite/gcc.dg/vect/bb-slp-48.c
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> /* { dg-additional-options "-fgimple -fdump-tree-optimized" } */
> /* { dg-require-effective-target vect_double } */
>
> diff --git a/gcc/testsuite/gcc.dg/vect/slp-reduc-10a.c
> b/gcc/testsuite/gcc.dg/vect/slp-reduc-10a.c
> index d3c2c2d7f54..790e1e7eb14 100644
> --- a/gcc/testsuite/gcc.dg/vect/slp-reduc-10a.c
> +++ b/gcc/testsuite/gcc.dg/vect/slp-reduc-10a.c
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> /* { dg-require-effective-target vect_int } */
> /* { dg-additional-options "-fgimple" } */
>
> diff --git a/gcc/testsuite/gcc.dg/vect/slp-reduc-10b.c
> b/gcc/testsuite/gcc.dg/vect/slp-reduc-10b.c
> index 6a0d55def30..25c261dfa82 100644
> --- a/gcc/testsuite/gcc.dg/vect/slp-reduc-10b.c
> +++ b/gcc/testsuite/gcc.dg/vect/slp-reduc-10b.c
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> /* { dg-require-effective-target vect_int } */
> /* { dg-additional-options "-fgimple" } */
>
> diff --git a/gcc/testsuite/gcc.dg/vect/slp-reduc-10c.c
> b/gcc/testsuite/gcc.dg/vect/slp-reduc-10c.c
> index 20df2626764..f3502fd3e18 100644
> --- a/gcc/testsuite/gcc.dg/vect/slp-reduc-10c.c
> +++ b/gcc/testsuite/gcc.dg/vect/slp-reduc-10c.c
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> /* { dg-require-effective-target vect_int } */
> /* { dg-additional-options "-fgimple" } */
>
> diff --git a/gcc/testsuite/gcc.dg/vect/slp-reduc-10d.c
> b/gcc/testsuite/gcc.dg/vect/slp-reduc-10d.c
> index 8a512d5c14d..77cf2fc4d9e 100644
> --- a/gcc/testsuite/gcc.dg/vect/slp-reduc-10d.c
> +++ b/gcc/testsuite/gcc.dg/vect/slp-reduc-10d.c
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> /* { dg-require-effective-target vect_int } */
> /* { dg-additional-options "-fgimple" } */
>
> diff --git a/gcc/testsuite/gcc.dg/vect/slp-reduc-10e.c
> b/gcc/testsuite/gcc.dg/vect/slp-reduc-10e.c
> index 268ec9db77d..879d34708b4 100644
> --- a/gcc/testsuite/gcc.dg/vect/slp-reduc-10e.c
> +++ b/gcc/testsuite/gcc.dg/vect/slp-reduc-10e.c
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> /* { dg-require-effective-target vect_int } */
> /* { dg-additional-options "-fgimple" } */
>
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-cond-arith-2.c
> b/gcc/testsuite/gcc.dg/vect/vect-cond-arith-2.c
> index 38994ea82a5..49d3a17108a 100644
> --- a/gcc/testsuite/gcc.dg/vect/vect-cond-arith-2.c
> +++ b/gcc/testsuite/gcc.dg/vect/vect-cond-arith-2.c
> @@ -1,4 +1,5 @@
> /* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
> /* { dg-additional-options "-fgimple -fdump-tree-optimized -ffast-math" } */
>
> double __GIMPLE (ssa, startwith("loop"))
> diff --git a/gcc/tree-ssa-loop.cc b/gcc/tree-ssa-loop.cc
> index 73aa46627b4..21878342aed 100644
> --- a/gcc/tree-ssa-loop.cc
> +++ b/gcc/tree-ssa-loop.cc
> @@ -354,9 +354,14 @@ pass_tree_loop_init::execute (function *fun
> ATTRIBUTE_UNUSED)
> | LOOP_CLOSED_SSA)
> && scev_initialized_p ())
> */
> - loop_optimizer_init (LOOPS_NORMAL
> - | LOOPS_HAVE_RECORDED_EXITS);
> - rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa);
> + /* When a __GIMPLE testcase starts with a pass inside the loop pipeline
> + we already are in loop-closed SSA but still need to initialize SCEV. */
> + if (!loops_state_satisfies_p (fun, LOOP_CLOSED_SSA))
> + {
> + loop_optimizer_init (LOOPS_NORMAL
> + | LOOPS_HAVE_RECORDED_EXITS);
> + rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa);
> + }
> scev_initialize ();
>
> return 0;
