Re: [RFC] Toplev.c reorganization

[Zack Weinberg <zack at codesourcery dot com>]

Zdenek Dvorak <rakdver@atrey.karlin.mff.cuni.cz> writes:

> Hello,
>
> after discussing it with Honza Hubicka, I have written the following
> attempt to reorganize toplev.c. It has the following goals:
>
> 1) Make it easier to change order of passes, by building/updating
>    automatically the requiered structures and verifying prerequisites.
>    The final state should be that any ordering of passes would either
>    work or say you why it is impossible.
> 2) Decrease the number of unnecessary rebuilds of structures, by keeping
>    track of those that are up-to-date.
> 3) Perhaps also make things better organized.

I like all these goals, and your approach is also quite interesting.
I'd like to see something like this make it into 3.4.

That said, I think this patch tries to do too much all at once, and
the .def files you introduce are a terrible mess.  A more incremental
approach would be a good idea.  To start with, how about we make it so
that the highest-level interface to any given optimizer pass -- the
one called by rest_of_compilation -- always has the signature

 void name_of_pass (tree decl, FILE *dump_file);

and that function is responsible for everything which can be
considered logically part of that pass, but not for the timevar
pushes/pops, the generation of the dump file, calling garbage
collection, or deciding whether or not the optimization is going to be
done.  So, for example,

  if (optimize > 0 && flag_loop_optimize)
    {
      int do_unroll, do_prefetch;

      timevar_push (TV_LOOP);
      delete_dead_jumptables ();
      cleanup_cfg (CLEANUP_EXPENSIVE | CLEANUP_PRE_LOOP);
      open_dump_file (DFI_loop, decl);
      /* CFG is no longer maintained up-to-date.  */
      free_bb_for_insn ();

      do_unroll = flag_unroll_loops ? LOOP_UNROLL : LOOP_AUTO_UNROLL;
      do_prefetch = flag_prefetch_loop_arrays ? LOOP_PREFETCH : 0;
      if (flag_rerun_loop_opt)
        {
          cleanup_barriers ();

          /* We only want to perform unrolling once.  */
          loop_optimize (insns, rtl_dump_file, do_unroll);
          do_unroll = 0;

          /* The first call to loop_optimize makes some instructions
             trivially dead.  We delete those instructions now in the
             hope that doing so will make the heuristics in loop work
             better and possibly speed up compilation.  */
          delete_trivially_dead_insns (insns, max_reg_num ());

          /* The regscan pass is currently necessary as the alias
                  analysis code depends on this information.  */
          reg_scan (insns, max_reg_num (), 1);
        }
      cleanup_barriers ();
      loop_optimize (insns, rtl_dump_file, do_unroll | LOOP_BCT | do_prefetch);

      /* Loop can create trivially dead instructions.  */
      delete_trivially_dead_insns (insns, max_reg_num ());
      close_dump_file (DFI_loop, print_rtl, insns);
      timevar_pop (TV_LOOP);
      find_basic_blocks (insns, max_reg_num (), rtl_dump_file);

      ggc_collect ();
    }

would become

/* in loop.c */
void
loop_optimize_pass (decl, dump_file)
    tree decl ATTRIBUTE_UNUSED;
    FILE *dump_file;
{
  int do_unroll, do_prefetch;
  rtx insns = get_insns ();

  delete_dead_jumptables ();
  cleanup_cfg (CLEANUP_EXPENSIVE | CLEANUP_PRE_LOOP);
  /* CFG is no longer maintained up-to-date.  */
  free_bb_for_insn ();

  do_unroll = flag_unroll_loops ? LOOP_UNROLL : LOOP_AUTO_UNROLL;
  do_prefetch = flag_prefetch_loop_arrays ? LOOP_PREFETCH : 0;
  if (flag_rerun_loop_opt)
    {
      cleanup_barriers ();

      /* We only want to perform unrolling once.  */
      loop_optimize (insns, dump_file, do_unroll);
      do_unroll = 0;

      /* The first call to loop_optimize makes some instructions
         trivially dead.  We delete those instructions now in the
         hope that doing so will make the heuristics in loop work
         better and possibly speed up compilation.  */
      delete_trivially_dead_insns (insns, max_reg_num ());

      /* The regscan pass is currently necessary as the alias
              analysis code depends on this information.  */
      reg_scan (insns, max_reg_num (), 1);
    }
  cleanup_barriers ();
  loop_optimize (insns, dump_file, do_unroll | LOOP_BCT | do_prefetch);

  /* Loop can create trivially dead instructions.  */
  delete_trivially_dead_insns (insns, max_reg_num ());
  find_basic_blocks (insns, max_reg_num (), dump_file);
}

/* rest_of_compilation */
  if (optimize > 0 && flag_loop_optimize)
    {
      timevar_push (TV_LOOP);
      open_dump_file (DFI_loop, decl);
      loop_optimize_pass (decl, rtl_dump_file);
      close_dump_file (DFI_loop, print_rtl, get_insns ());
      timevar_pop (TV_LOOP);
      ggc_collect ();
    }

This produces a rest_of_compilation that's a nice simple chain of
tests and calls to optimizer passes, all with the same interface.
That can then be macroized very easily.  And then, with a more rigid
interface enforced between toplev.c and the individual optimizer
passes, we ought to be able to do your structure monitoring stuff
without making quite so much of a mess.

zw