On Wed, 8 May 2019, Jeff Law wrote: > On 5/8/19 6:20 AM, Richard Biener wrote: > > On Wed, 8 May 2019, Jiufu Guo wrote: > > > >> Hi, > >> > >> Thanks Richard, Segher, Andrew and all. > >> > >> Segher Boessenkool <seg...@kernel.crashing.org> writes: > >> > >>> Let me try to answer some of this... > >>> > >>> On Tue, May 07, 2019 at 03:31:27PM +0200, Richard Biener wrote: > >>>> On Mon, 6 May 2019, Jiufu Guo wrote: > >>>>> This patch implements the optimization in PR77820. The optimization > >>>>> eliminates phi and phi's basic block, if the phi is used only by > >>>>> condition branch, and the phi's incoming value in the result of a > >>>>> CMP result. > >>> > >>>> I'm not sure I like a new pass here ;) The transform is basically > >>>> tail-duplicating the PHI block because the exit conditional can > >>>> be "simplfied" - that's something jump threading generally does > >>>> though it relies on "simplified" being a little bit more simplified > >>>> than above. > >>> > >> Adding a new pass is just because it may be relatively easy to extend > >> and maintain. > >> > >> Adding this micor-optimization into other passes is also a good > >> sugguestion. > >> > >> - Similar with jump threading, this new transform is replacing jump > >> old destination with new destination. While for this case, the new > >> destination can not be determined at compile time. > >> > >> - And as Andrew said: forwprop/backprop are similar, but this case is in > >> opposite: it is spread common code(phi+gcond) into different preds. > >> > >> - And there is phiopt pass which focus on making 'phi' stmt better. > >> it also do some similar thing: > >> bb0: > >> if (a != b) goto bb2; else goto bb1; > >> bb1: > >> bb2: > >> x = PHI <a (bb1), b (bb0), ...>; > >> > >> tranform to: > >> > >> bb0: > >> bb2: > >> x = PHI <b (bb0), ...>; > >> > >> If I avoid to add new pass, any suggustions about which exiting pass > >> (jumpthreading/forwprop/phiopt/...) would be more suitable to extend to > >> support this new transform? > > > > The main thing the transform does is tail-duplicate the PHI block, > > if we'd just do that followup transforms would do the rest. > One might even claim this is really a transformation for cfg cleanups. > If we ignore the PHI what we have is a unconditional jump to a > conditional jump. The obvious way to optimize that is to replace the > unconditional jump with a copy of the conditional jump.
I though about this too, but then quickly disregarded as too gross ;) > >> Currently this pass handles the case if there is only one PHI and the > >> phi is used by gcond. If there are other suitable cases, we can just > >> extend this tranform. > > > > I was just thinking of a small amount of unrelated stmts in those > > blocks or the case where just one PHI argument is fed by a conditional > > thus only one path would simplify (but maybe the hot one if you look > > at edge probabilities). > Perhaps, but do these happen enough in practice? With the code motions > we do I'd be a bit surprised. It probably depends on what kind of source this typically arises from. Richard.
