Re: [tree-ssa] Patch ping

[Andrew MacLeod <amacleod at redhat dot com>]

On Tue, 2003-12-02 at 14:06, Zdenek Dvorak wrote: 
> Hello,
> 
> a few patches I spend too much time with to just let them be missed without
> a word:
> 
> http://gcc.gnu.org/ml/gcc-patches/2003-11/msg01881.html
>   -- a patch to save some memory by not having the unnecessary
>      GOTO_EXPRS in branches of COND_EXPRs.
> 
I dont think I like this patch because it gives us a dual mode
COND_EXPRs. Sometimes its code and sometimes its just a label, which
makes the GOTO_EXPR code implicit instead of explicit. With this change,
the meaning of a COND_EXPR is not compatible with the definition in
GENERIC/GIMPLE. With things like mudflap running after tree-ssa, this is
going to impact them as well.

You are right, it is a waste of memory, and its meaning could be
changed. Its what, about 48 bytes extra per COND_EXPR node? I dont think
the memory is significant enough personally, but others may differ :-)  

It was also mentioned briefly in
http://gcc.gnu.org/ml/gcc-patches/2003-10/msg02235.html
and we were suppose to implement the accessor macros to hide the
GOTO_EXPRs and go directly to the labels, which I think we forgot to do
and ought to be done. 


> http://gcc.gnu.org/ml/gcc-patches/2003-11/msg02131.html
>   -- a patch to make the manipulation with optimization passes over
>      tree-ssa simpler and better organized
> 
I dont know that we need to go through this yet, I find it simple enough
at the moment just to make the changes where required. Im only 1 step
from being indifferent about it tho.


> http://gcc.gnu.org/ml/gcc-patches/2003-11/msg02380.html
>   -- a boring pieces neccessary for a loop optimizer pass
> 
    /* Redirect back edges we want to keep.  */
    for (e = dummy->pred; e; e = next_e)
      {
        next_e = e->pred_next;
!       if (e != except
! 	  && ((redirect_latch && LATCH_EDGE (e))
! 	      || (redirect_nonlatch && !LATCH_EDGE (e))))
! 	continue;
! 
!       dummy->frequency -= EDGE_FREQUENCY (e);
!       dummy->count -= e->count;
!       if (dummy->frequency < 0)
! 	dummy->frequency = 0;
!       if (dummy->count < 0)
! 	dummy->count = 0;
! 
!       new_e = tree_redirect_edge_and_branch_1 (e, bb, true);
! 
!       if (first)
! 	{
! 	  first = false;
! 
! 	  /* The first time we redirect a branch we must create new phi nodes
! 	     on the start of bb.  */
! 	  for (phi = phi_nodes (dummy); phi; phi = TREE_CHAIN (phi))
! 	    {
! 	      var = PHI_RESULT (phi);
! 	      new_phi = create_phi_node (var, bb);
! 	      SSA_NAME_DEF_STMT (var) = new_phi;
! 	      PHI_RESULT (phi) = make_ssa_name (SSA_NAME_VAR (var), phi);
! 	      add_phi_arg (&new_phi, PHI_RESULT (phi), fallthru);
! 	    }
! 
! 	  /* Ensure that the phi node chains are in the same order.  */
! 	  bb_ann (bb)->phi_nodes = nreverse (phi_nodes (bb));
! 	}
! 
!       /* Move the argument of the phi node.  */
!       for (phi = phi_nodes (dummy), new_phi = phi_nodes (bb);
! 	   phi;
! 	   phi = TREE_CHAIN (phi), new_phi = TREE_CHAIN (new_phi))
! 	{
! 	  var = PHI_ARG_DEF (phi, phi_arg_from_edge (phi, e));
! 	  add_phi_arg (&new_phi, var, new_e);
! 	  remove_phi_arg (phi, e->src);
  	}


So lets see if I follow. the original BB is left as is, except all the
labels are removed and added the the new BB, dummy. And everything
except the back edge(s) will be moved to point to the new dummy BB?

If you only redirect one edge, aren't you going to create PHI nodes with
one argument? I beleive those are illegal.
And you may need PHI arguments from dummy->BB which I dont think are
being added.

BB5  PHI a_4=<a_1(2), a_2(3), a_3(4)>

if BB4->BB5 is the back edge, you need something like:

BB6 PHI a_5=<a_1(2), a_2(3)>      /* This is block dummy.  */
BB5 PHI a_4=<a_5(6), a_3(4)>


Might the back edge be one of the arms of a COND_EXPR, which means that
one of those labels being moved to dummy is actually the target of the
back edge? so moving that label would be wrong.

This is the only bit I saw any problem with.

Andrew