On Mon, 2016-11-14 at 16:14 +0100, Richard Biener wrote:
> On Fri, Nov 11, 2016 at 10:15 PM, David Malcolm <dmalc...@redhat.com>
> 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
