[Bug middle-end/38264] New: tree_forwarder_block_p says no to first basic block

[Bug middle-end/38264] New: tree_forwarder_block_p says no to first basic block

[rguenth at gcc dot gnu dot org]

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.


-- 
           Summary: tree_forwarder_block_p says no to first basic block
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Keywords: missed-optimization
          Severity: enhancement
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38264

[Bug middle-end/38264] tree_forwarder_block_p says no to first basic block

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2008-11-25 19:11 -------
Created an attachment (id=16767)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16767&action=view)
patch

Patch that miscompares.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38264

[Bug middle-end/38264] tree_forwarder_block_p says no to first basic block

[law at redhat dot com]

------- Comment #2 from law at redhat dot com  2008-11-25 20:04 -------
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


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38264

[Bug middle-end/38264] tree_forwarder_block_p says no to first basic block

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38264

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-02-02
     Ever Confirmed|0                           |1

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-02-02 17:58:52 UTC ---
Confirmed.
The code looks slightly different now as find_edge has been inlined and merged
with checking for eh edges.

[Bug middle-end/38264] tree_forwarder_block_p says no to first basic block

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(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.

[Bug middle-end/38264] tree_forwarder_block_p says no to first basic block

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2012-02-02 00:00:00         |2023-5-26

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
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)>

[Bug middle-end/38264] tree_forwarder_block_p says no to first basic block

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
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.