Created attachment 16767 [details]
patch

Patch that miscompares.

Subject: Re:   New: tree_forwarder_block_p says no to
 first basic block

rguenth at gcc dot gnu dot org wrote:
> tree_forwarder_block_p has
>
>   if (find_edge (ENTRY_BLOCK_PTR, bb))
>     return false;
>
> without explanation.  This test was added by you, Jeff - do you remember why?
>
> Removing this check triggers some ICEs in the testsuite because remove_bb
> (called from remove_forwarder_block) unconditionally moves labels from the
> removed block to prev_bb (yuck!) - which is of course invalid if that happens
> to be the entry bb.  Luckily remove_forwarder_block already contains code
> to do the label-move job itself - it is just conditional on seen abnormal
> incoming edges.  Enabling this code to run by default causes a bootstrap
> comparison failure though.
>
>
>   
I'm not sure -- that code was introduced in 2003 and looks like it was a 
mix of some of Zdenek's work as well as mine.  It could well have been 
the label issue, or some horrid problem dealing with our control 
structures (which were still in the IL at that time), or simply my 
mis-translation of some of Zdenek's code.

Clearly remove_bb has to do something with the labels.  I think it was 
decided that the labels could go anywhere and the previous block 
reasonably convenient -- the theory was the block was unreachable, so 
the location of a named label in the block was unimportant.  In the case 
of a forwarder block the label was reachable and needs to be moved into 
a sane location -- removal of forwarder blocks probably wasn't something 
we considered when discussing the named label issues.

Jeff

Confirmed.
The code looks slightly different now as find_edge has been inlined and merged with checking for eh edges.

(In reply to Andrew Pinski from comment #3)
> Confirmed.
> The code looks slightly different now as find_edge has been inlined and
> merged with checking for eh edges.

Which was done with r0-95726-g1d65f45cfaefa0 .

The code is still there today.
I am not sure if this issue still applies, almost 15 years after it was filed.

I filed this because for

void bar ();
void foo (int n)
{
  int i = 0;
  do
    bar ();
  while (++i < n);
}

we never remove the preheader forwarder created by loop passes on GIMPLE:

void foo (int n)
{
  int i;

  <bb 2> [local count: 118111600]:

  <bb 3> [local count: 1073741824]:
  # i_1 = PHI <0(2), i_5(3)>

but if you add a bar() call before the loop we do, because the forwarder
isn't the first block then.

void foo (int n)
{
  int i;

  <bb 2> [local count: 118111600]:
  bar ();

  <bb 3> [local count: 1073741824]:
  # i_1 = PHI <0(2), i_6(3)>

I actually just ran into a problem caused by having this check here.

I was modifying tree-ssa-ifcombine to optimize the case where we have the same condition on both bb from a bb like:
```
int g();
int h();

int j, l;

int f(int a, int *b)
{
        if (a == 0)
        {
                if (b == &j) goto L9; else goto L7;
        }
        else
        {
                if (b == &j) goto L9; else goto L7;
        }
L7: return g();
L9: return h();
}
```

I go and try to remove one of the bb (which have the same condition) and that should have updated dominators in a reasonable way because the removal should have done a forwarder from bb 2 to bb3. But instead of doing the forwarding of the bb, we end up with still bb3 and the dominator needs to be updated in non-trival ways.