[PATCH] Fix PR34263: Cleaning up latch blocks

[Andrew MacLeod]

 > Following the RFC message
 > (http://gcc.gnu.org/ml/gcc/2007-12/msg00015.html), this patch tries
 > to clean up latch blocks in the out-of-ssa phase for the purpose of
 > restoring single-basic-block loops. This can be helpful not only in the
 > SMS perspective. Given the fact we are in stage 3 I can alternatively
 > address this inside the SMS (and not in out-of-ssa).

 > The patch was successfully tested a week ago (when I posted the RFC
 > message) on ppc64 and I am re-checking it now to verify it is still OK.

 > OK for mainline once testing completes?

So If I get this right, This patch addresses extra blocks that are 
created in circumstances like:

BB6:
a_2 = PHI <a_3(4), a_4(6)>
<...>
a_4 = expr
if (x) then BB6 else BB7
BB7:

which we currently translate to something like:

a_2 = a_3
BB6:
<...>
a_4 = expr
if (x) then BB14 else BB7
BB14:
a_2 = a_4
goto BB6:
BB7:

and with this change, we would then generate:
a_2 = a_3
BB6:
<...>
a_4 = expr
a_9 = a_2
a_2 = a_4
if (x) then BB6 else BB7
BB7:
a_2 = a_9

Effectively stashing the value of a_2 to load back in the case where we 
exit. Is this right? (except it is actually working on the coalesced 
root variables, not the actual SSA_NAMES)

Seems to me there is an older PR that involves a similar situation too , 
but I dont recall it.

The patch looks fine, except I would ask that all this logic be lifted 
out to a function which returns TRUE if it is a single block loop and 
performs the work.   So the code looks something like:

if (single_edge)
  {
    /* Add stmts to the edge unless processed specially as a 
single-block loop latch edge.  */
    if (!process_single_block_loop_latch (single_edge))   
      bsi_commit_one_edge_insert (single_edge, NULL);
    }

And a few extra comments in the body would help to read it too, just 
simple things  like:
 

               /* Find the successor edge which is not the latch edge.  */
+              FOR_EACH_EDGE (e, ei, b_loop->succs)
+                if (e->dest != b_loop)
+                  break;
+
+              b_exit = e->dest;
+
               /* Check that the exit block has only the loop as a predecessor, and that there
                  are no pending stmts on that edge as well.  */ 
+              if (EDGE_COUNT (b_exit->preds) != 1 || PENDING_STMT (e))
+                do_it = false;
+
               /* Find the predecessor edge which is used for loop entry.  */ 
+              FOR_EACH_EDGE (e, ei, b_loop->preds)
+                if (e->src != b_loop)
+                  break;
+


It just makes it a little easier to read.

Should there be some kind of limit to the number of copies being 
inserted? Ie, if there are 10 copies being inserted on the edge, I would 
think 10 extra copies in the loop would tend to start adding up and 
offset whatever gain you might get. 

As long as there isn't disagreement on whether the extra copies in and 
out of the loop are a problem, the patch istself is OK.

Andrew