From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 9680D3858D3C for ; Fri, 18 Mar 2022 16:18:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9680D3858D3C Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 28B001515; Fri, 18 Mar 2022 09:18:36 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.88]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 715683F7B4; Fri, 18 Mar 2022 09:18:35 -0700 (PDT) From: Richard Sandiford To: Richard Biener Mail-Followup-To: Richard Biener , gcc-patches@gcc.gnu.org, Jan Hubicka , richard.sandiford@arm.com Cc: gcc-patches@gcc.gnu.org, Jan Hubicka Subject: Re: [PATCH] [GIMPLE FE] allow to unit test loop passes References: <20220317110001.B962013B4B@imap2.suse-dmz.suse.de> Date: Fri, 18 Mar 2022 16:18:34 +0000 In-Reply-To: <20220317110001.B962013B4B@imap2.suse-dmz.suse.de> (Richard Biener's message of "Thu, 17 Mar 2022 12:00:01 +0100 (CET)") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Mar 2022 16:18:39 -0000 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 @@ 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 ()); > 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 (_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;