[tree-ssa] Supefluous phi removal

law@redhat.com law@redhat.com
Tue Jan 6 17:20:00 GMT 2004


In message <20040106104342.GA27344@atrey.karlin.mff.cuni.cz>, Zdenek Dvorak wri
tes:
 >> On a high level, don't you have to verify that you don't have dependencies
 >> between PHI nodes before copy propagating values like this?   ie, if the
 >> output of a PHI you are removing is used as an input by another PHI in
 >> the same block, then simple copy propagation may not be appropriate
 >> (remember conceptually the RHS of all PHIs are evaluated at the same time,
 >> then assignments take place.  When you turn them into copies (even those
 >> which you propagate away) you effectively serialize the PHI nodes which is
 >> not always correct.
 >
 >I am not sure, I will have to think about it more.  But would not
 >exactly the same mistake then be in half of the other passes (ccp, dom)?
Hmmm.  Hmmm.  Hmmm. 

Ahh, now I remember more details about these issues.

You only run into the serialization problem if you turn a PHI into a copy
and can not fully copy propagate the copy.

If you fully propagate the copy then the right things will "just happen".

Your code avoids this problem completely by never emitting the copy to
begin with -- the test for elimination of the PHI is the same as the test
for propagating its value.  Yet another way your code is better than the
prototype stuff I've got lying around here :-)

 >> It also seems to me that this test is wrong:
 >> 
 >> + 	      if (TREE_CODE (stmt) != PHI_NODE
 >> + 		  || SSA_NAME_OCCURS_IN_ABNORMAL_PHI (t))
 >> + 		{
 >> + 		  eq_to[aver] = t;
 >> + 		  raise_value (phi, t, eq_to);
 >> + 		}
 >> 
 >> Shouldn't that be ! SSA_NAME_OCCURS_IN_ABNORMAL_PHI (t)) since it would
 >> seem to me that we do _not_ want to record an equivalence for PHI alternati
 >ve
 >> associated with an abnormal edge.
 >
 >At this place I am preventing copy propagation into such a use (eq_to
 >contains the variable/constant the given ssa name should be replaced
 >with, so by setting it to itself I prevent it from being replaced by
 >anything else).
Which is a good indication some documentation is needed :-)

jeff




More information about the Gcc-patches mailing list