On 5/9/19 12:07 AM, Richard Biener wrote: > 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 ;) Yea, the changes to the CFG (and dominator tree and SSA graph) are probably significant enough that trying to do it into cleanup_cfg is just madness. One of those few cases where doing something on RTL is probably easier than gimple.
Jeff
