Re: [tree-ssa] Patch ping

[Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>]

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

yes, this is from where the idea is taken; you may note that I did not
like it very much, on the other hand, saving 50 bytes on every fifth
statement of a program seemed good enough argument to me.

> 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. 

Why? The labels are basically never accessed (except for single place in
cfg construction).

> > 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.

Say that you want to swap order two passes. You must update things on 3
different places.  Of course it is not something crucial, just it seems
nice to me to have these technical things as simple and clean as
possible.

> > 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);

(XXX)

> ! 	    }
> ! 
> ! 	  /* 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.

No they are not.  In fact quite many of them are created during
optimizations (for example due to jump threading, or when some blocks
are removed as unreachable).  I am working on a cleanup that removes them,
but it still has some problems.

> And you may need PHI arguments from dummy->BB which I dont think are
> being added.

They are, in the moment new phi nodes are created -- marked (XXX) above.

> 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,

Yes.

> 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.

No -- we use the ordinary edge redirection that updates the labels.

Zdenek