On Wed, Jun 4, 2014 at 9:59 AM, Jeff Law <l...@redhat.com> wrote: > On 06/04/14 01:46, Marc Glisse wrote: >> >> On Tue, 3 Jun 2014, Jeff Law wrote: >> >>> On 06/03/14 08:08, Richard Biener wrote: >>>> >>>> All arguments get the same value (and the PHI in middle-bb is surely >>>> a singleton?), so it's way better to re-materialize the PHI as a >>>> gimple assignment at the start of the basic block. If they are >>>> singletons (which I expect), the easiest way is to propagate their >>>> single argument into all uses and simply remove them. >> >> >> I was all for propagating, but replace_uses_by calls fold_stmt on the >> using statements (that's normal). If I first move the statement and then >> replace, I may call fold_stmt on a stmt that isn't dominated by the defs >> of all its arguments. If I first replace, the statement I am supposed to >> move may have been modified so I need to find out what happened to it. >> In the end, the assignment seems easier and safer (and not too bad since >> this case almost never happens in practice). >> >>> We certainly want to get rid of them :-) >>> >>> I'd start by finding out which pass left the degenerate, ensure it's >>> not marked as SSA_NAME_OCCURS_IN_ABNORMAL_PHI, then propagate it away. >>> >>> If it's a systemic problem that a particular pass can leave degenerate >>> PHIs, then you might want to schedule the phi-only propagator to run >>> after that pass. It does const/copy propagation seeding from >>> degenerate PHIs, so it ought to be very fast. >> >> >> This one comes from VRP2 apparently. But it isn't the only pass that >> produces singleton phis. I didn't look at them closely, but I assume LIM >> is normal, DOM maybe less so, etc. > > DOM & VRP can create them via jump threading. Right now we only fire up the > pass_phi_only_cprop after DOM though.
Right, if we want to get rid of the degenerates early we have to run pass_phi_only_cprop after VRP as well. > DOM runs not too long after LIM and it will propagate away the degenerates > as well. Not sure where LIM would create degenerate PHIs, but I guess you simply see loop-closed PHIs here which are degenerate. > One could argue that propagating away the degenerates ought to occur > immediately before any phi-opt pass. I'd say we should run phi_only_cprop after the first VRPs instead and move the 2nd VRP before the existing phi_only_cprop pass. I'll do the appropriate change. >> >> bootstrap+testsuite on x86_64-linux-gnu as usual. >> >> 2014-06-04 Marc Glisse <marc.gli...@inria.fr> >> >> PR tree-optimization/61385 >> gcc/ >> * tree-ssa-phiopt.c (value_replacement): Replace singleton PHIs >> with assignments. >> gcc/testsuite/ >> * gcc.dg/tree-ssa/pr61385.c: New file. > > Don't you have to verify neither the source nor the destination are marked > with SSA_NAME_OCCURS_IN_ABNORMAL_PHI? And if so, you can't propagate away > the PHI (which may mean we need to avoid the optimization in that case). > > I also believe that when turning a PHI into an assignment you have to > preserve the parallel nature of the PHI. If the result of one PHI is used > as an input in another PHI, then you have to work harder (see the out-of-ssa > code). Perhaps this can never be the case with a degenerate. Hmm, indeed that can happen. So I'd say we should instead simply bail out if the middle-bb has a PHI node. Simple and safe. And with the pass ordering fixes above we shouldn't see degenerates anyway. Richard. > Jeff
