This is the mail archive of the
mailing list for the GCC project.
Re: [tree-ssa] Supefluous phi removal
- From: law at redhat dot com
- To: Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 06 Jan 2004 11:22:14 -0700
- Subject: Re: [tree-ssa] Supefluous phi removal
- Reply-to: law at redhat dot com
In message <20040106110717.GA32013@atrey.karlin.mff.cuni.cz>, Zdenek Dvorak wri
>> 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
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?