From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 83258 invoked by alias); 15 Nov 2016 21:07:54 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 83249 invoked by uid 89); 15 Nov 2016 21:07:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_50,KAM_STOCKGEN,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=productions, Congratulations, force_output, merely X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 15 Nov 2016 21:07:43 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0A0D71E2F0; Tue, 15 Nov 2016 21:07:42 +0000 (UTC) Received: from vpn-226-89.phx2.redhat.com (vpn-226-89.phx2.redhat.com [10.3.226.89]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uAFL7foG014681; Tue, 15 Nov 2016 16:07:41 -0500 Message-ID: <1479244060.7673.66.camel@redhat.com> Subject: Re: [PATCH 9/9] Add "__RTL" to cc1 (v4) From: David Malcolm To: Richard Biener Cc: GCC Patches Date: Tue, 15 Nov 2016 21:07:00 -0000 In-Reply-To: References: <1478898935-46932-1-git-send-email-dmalcolm@redhat.com> <1478898935-46932-10-git-send-email-dmalcolm@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg01550.txt.bz2 On Mon, 2016-11-14 at 16:14 +0100, Richard Biener wrote: > On Fri, Nov 11, 2016 at 10:15 PM, David Malcolm > wrote: > > Changed in this version: > > > > * Rather than running just one pass, run *all* passes, but start at > > the given pass; support for "dg-do run" tests that execute the > > resulting code. > > * Updated test cases to new "compact" dump format; more test cases; > > use "dg-do run" in various places. > > * Lots of bugfixing > > > > Links to previous versions: > > https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00263.html > > https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00500.html > Does running the RTL passes right from the parser work with -fsyntax > -only? It depends what you mean by "work". If I run it with -fsyntax-only, then pass_rest_of_compilation::gate returns false, and none of the RTL passes are run. Is this behavior correct? > Doing it like __GIMPLE has the advantage of not exposing > "rest_of_compilation", etc.. The gimple part of the compiler supports having multiple functions in memory at once, and then compiling them in arbitrary order based on decisions made by the callgraph. By contrast, the RTL part of the compiler is full of singleton state: things like crtl (aka x_rtl), the state of emit-rtl.c, "reload_completed", etc etc. To try to limit the scope of the project, for the RTL frontend work I'm merely attempting to restore the singleton RTL state from a dump, rather than to support having per function stashes of RTL state. Hence the rest of compilation gets invoked directly from the frontend for the __RTL case, since it will get overwritten if there's a second __RTL function in the source file (which sounds like an idea for a test case; I'll attempt such a test case). I hope this is a reasonable approach. If not, I suppose I can attempt to bundle it up into some kind of RTL function state, but that seems like significant scope creep. > I'm now handling __GIMPLE from within declspecs (the GIMPLE FE stuff > has been committed), it would be nice to match the __RTL piece here. (Congratulations on getting the GIMPLE FE stuff in) I'm not sure I understand you here - do you want me to rewrite the __RTL parsing to match the __GIMPLE piece, or the other way around? If the former, presumably I should reuse (and rename) c_parser_gimple_pass_list? > > gcc/ChangeLog: > > * Makefile.in (OBJS): Add run-rtl-passes.o. > > > > gcc/c-family/ChangeLog: > > * c-common.c (c_common_reswords): Add "__RTL". > > * c-common.h (enum rid): Add RID_RTL. > > > > gcc/c/ChangeLog: > > * c-parser.c: Include "read-rtl-function.h" and > > "run-rtl-passes.h". > > (c_parser_declaration_or_fndef): In the "GNU extensions" > > part of > > the leading comment, add an alternate production for > > "function-definition", along with new "rtl-body-specifier" > > and > > "rtl-body-pass-specifier" productions. Handle "__RTL" by > > calling > > c_parser_parse_rtl_body. Convert a timevar_push/pop pair > > to an auto_timevar, to cope with early exit. > > (c_parser_parse_rtl_body): New function. > > > > gcc/ChangeLog: > > * cfg.c (free_original_copy_tables): Remove assertion > > on original_copy_bb_pool. > > How can that trigger? It happens when running pass_outof_cfg_layout_mode::execute; seen with gcc.dg/rtl/x86_64/test-return-const.c.before-fwprop.c. The input file is a dump taken in cfg_layout mode (although in this case it's a trivial 3-basic-block CFG, but ideally there would be cases with non-trivial control flow); it has "fwprop1" as its starting pass. Running without -quiet shows: skipping pass: *rest_of_compilation skipping pass: vregs skipping pass: into_cfglayout skipping pass: jump skipping pass: subreg1 skipping pass: cse1 found starting pass: fwprop1 i.e. it skips the into_cfglayout (amongst others), to start with fwprop1. In theory skipping a pass ought to be a no-op, assuming that we're faithfully reconstructing all RTL state. However, RTL state management is fiddly, so the patch introduces some logic in passes.c to do some things when skipping a pass; in particular, it has: /* Update the cfg hooks as appropriate. */ if (strcmp (pass->name, "into_cfglayout") == 0) { cfg_layout_rtl_register_cfg_hooks (); cfun->curr_properties |= PROP_cfglayout; } if (strcmp (pass->name, "outof_cfglayout") == 0) { rtl_register_cfg_hooks (); cfun->curr_properties &= ~PROP_cfglayout; } so that even when skipping "into_cfglayout", the CFG hooks are at least correct. The assertion fires when running outof_cfglayout later on (rather than skipping it); the assertion: gcc_assert (original_copy_bb_pool); assumes that into_cfglayout was actually run, rather than just the simple "skipping" version of it. > > * cgraph.h (symtab_node::native_rtl_p): New decl. > > * cgraphunit.c (symtab_node::native_rtl_p): New function. > > (symtab_node::needed_p): Don't assert for early assembly > > output > > for __RTL functions. > > (cgraph_node::finalize_function): Set "force_output" for > > __RTL > > functions. > > (cgraph_node::analyze): Bail out early for __RTL functions. > > (analyze_functions): Update assertion to support __RTL > > functions. > > (cgraph_node::expand): Bail out early for __RTL functions. > > * emit-rtl.c (unshare_all_rtl_again): Wrap set_used_decls > > call > > in check for DECL_INITIAL. > > You should simply set DECL_INITIAL of your function decl (make_node > (BLOCK);). > There's too much code assuming that is not NULL (and I've fixed quite > a bit of > code during stage1 not doing that). Thanks; fixed. > > * final.c (rest_of_clean_state): Don't call delete_tree_ssa > > for > > _RTL functions. > > * function.h (struct function): Add field "native_RTL". > > I wonder if you could simply use ->curr_properties & PROP_rtl? (and > set that > property during parsing, of course) I tried to doing that; it mostly works, but this assertion fails: 237 symtab_node::needed_p (void) 238 { 239 /* Double check that no one output the function into assembly file 240 early. */ 241 if (!native_rtl_p ()) 242 gcc_checking_assert 243 (!DECL_ASSEMBLER_NAME_SET_P (decl) 244 || !TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl))); [I added the "if (!native_rtl_p ())" guard in the patch] The issue here is that when this is run, the __RTL function has been compiled and cleaned up, and the curr_properties & PROP_rtl has been cleared: (gdb) p decl->function_decl->f->curr_properties $7 = 0 I could set the flag again after running the passes; on doing so, the test suite successfully runs. Would you prefer I use curr_properties & PROP_rtl for this? [...snip...] Thanks Dave