https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68247

            Bug ID: 68247
           Summary: Remove pass_first_instance
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: other
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

The compiler contains a variable first_pass_instance:
...
$ grep -d skip first_pass_instance gcc/* | grep -v ChangeLog
gcc/passes.c:bool first_pass_instance;
gcc/passes.c:  first_pass_instance = (flags & TODO_mark_first_instance) != 0;
gcc/tree-object-size.c:   if (first_pass_instance)
gcc/tree-pass.h:extern bool first_pass_instance;
gcc/tree-ssa-ccp.c:               || first_pass_instance)))
gcc/tree-ssa-dom.c:  cfg_altered |= thread_through_all_blocks
(first_pass_instance);
gcc/tree-ssa-reassoc.c:  if (!first_pass_instance
gcc/tree-ssa-reassoc.c:       if (first_pass_instance
gcc/tree-vrp.c:  if (warn_array_bounds && first_pass_instance)
...

[ See also related discussion at
https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00452.html ]

I think it's a bad idea to have passes behave differently based on the location
in the pass list. It makes it harder to reason about the effect of the pass
list.

Take f.i. pass_dominator:
...
  /* Thread jumps, creating duplicate blocks as needed.  */
  cfg_altered |= thread_through_all_blocks (first_pass_instance);
...

Where thread_through_all_blocks has parameter may_peel_loop_headers:
...
bool
thread_through_all_blocks (bool may_peel_loop_headers)
...

Looking at the pass list, there's nothing suggesting that the first pass
instance behaves different from the second pass instance:
...
      NEXT_PASS (pass_dominator);
      ...
      NEXT_PASS (pass_dominator);
...

So it would be more clear to have this in the pass list:
...
      NEXT_PASS (pass_dominator);
      ...
      NEXT_PASS (pass_dominator_no_peel_loop_headers);
...

The problem there is that we have to add another pass, which will have a
different dump file (something that was mentioned in the related discussion as
a drawback).

I propose a different solution. We can parametrize the pass instance creation:
...
      NEXT_PASS_WITH_ARG (pass_dominator, true /* may_peel_loop_headers */);
      ...
      NEXT_PASS_WITH_ARG (pass_dominator, false /* may_peel_loop_headers */);
...

This way, it's clear in the pass list what each pass instance does and when,
and the two pass instances still share the same dump file.

Reply via email to