[PATCH] D17442: [LPM] Remove the last worrisome split in the primary loop pass pipeline, allowing LICM and friends to always run over the outer loop after unrolling has a chance to remove the inner loop.

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

In http://reviews.llvm.org/D17442#357429, @mzolotukhin wrote:

> > LoopPassManager [ loop passes A -> LoopSimplifyCFG -> loop passes B ]
>
> > SimplifyCFG
>
> > 
>
> > does worse than
>
> > 
>
> > LoopPassManager [ loop passes A ]
>
> > SimplifyCFG
>
> >  LoopPassManager [ loop passes B ]
>
>
> Yeah, in my experiments we did have a lot of cases like you described. Basically, `LoopSimplifyCFG` in the first option wasn't able to cleanup the code enough for `loop passes B` to be effective. I also tried adding loop deletion/etc. but with no luck either.
>  That said, I didn't spend much time on it, so I don't have deep details. I think it's worth investigating further, especially since this is an active are now.


The question is, what should we do *now*.

Note that today, the simplify-cfg run comes before the thing that I think needs the most cleanup: unroll. So this change won't regress *any* cleanup taking place prior to loop unrolling. So if "unroll" is the "loop passes A" in the above example, it already doesn't work, and this patch doesn't make it worse.

Today, loop unrolling definitely creates invariant instructions that we then fail to hoist with LICM. This patch will fix that. Similarly, this patch will fix cases where we need to re-rotate an outer loop after unrolling an inner loop in order to unroll the outer loop.

So that's why I suspect the best path is to make this change, and then incrementally improve the loop simplification passes until they are sufficient to handle the other cases.

Thoughts?


http://reviews.llvm.org/D17442