This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [tree-ssa] Supefluous phi removal


In message <20040106110717.GA32013@atrey.karlin.mff.cuni.cz>, Zdenek Dvorak wri
tes:
 >Hello,
 >
 >> For kill_redundant_phi_nodes, it seems to me you ought to document the code
 >> a little.  Specifically I think documenting how EQ_TO is used would be wise
 >,
 >> particularly the case when it indicates that a PHI should not be eliminated
 >> and a copy should not be propagated.  
 >
 >here is the piece of patch with comments extended.
I think you still need some comments.

First, there is no documentation for the DEF array.  A simple statement
indicating that it holds the SSA_NAMEs would help.  Hell, you might as well
call it ssa_names or something like that since that's a lot more accurate
given how that array is actually used.


+ 	      if (TREE_CODE (stmt) != PHI_NODE
+ 		  /* Prevent copy propagation into arguments of phi nodes
+ 		     corresponding to abnormal edges, by setting their
+ 		     value to top.  */
+ 		  || SSA_NAME_OCCURS_IN_ABNORMAL_PHI (t))
+ 		{
+ 		  eq_to[aver] = t;
+ 		  raise_value (phi, t, eq_to);


So what's not terribly obvious is that the assignment to eq_to here is
preventing copy propagation of the PHI argument, but that the PHI result
is still eligible for copy propagation.

Instead I would write something like this

  /* Prevent copy propagation of SSA_NAMES which occur in abnormal PHI
     nodes.  */
  if (SSA_NAME_OCCURS_IN_ABORMAL_PHI (t))
    eq_to[aver] = t;

  /* Raise the result of PHI by joining it to T.  */
  raise_value (phi, t, eq_to);

It seems to me that you always want to call raise_value, regardless of
whether or not T is set from a normal statement or a PHI node.  Or am
I missing something?

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]