[PATCH] D75145: [PassManager] adjust VectorCombine placement

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D75145#1909597 <https://reviews.llvm.org/D75145#1909597>, @lebedev.ri wrote:

> In D75145#1909542 <https://reviews.llvm.org/D75145#1909542>, @lebedev.ri wrote:
>
> > In D75145#1909503 <https://reviews.llvm.org/D75145#1909503>, @dmgreen wrote:
> >
> > > > just curious since we got regressions downstream after this patch... haven't looked deeper at that
> > >
> > > Same here. It looks like running cse before instcombine is altering a fair amount, at least in a way that our Low Overhead loop pass does not like. I'm not sure if there are other problems or if it's just that.
> > >
> > > Looking at it, the way the iteration count is calculated is done differently now. This code:
> > >  https://godbolt.org/z/2gBwF2
> > >  Has changed the way that the vector preheader calculated the loop iteration values. This is after (top) and before (bottom):
> > >  https://godbolt.org/z/tocy_x
> > >  Notice the differences in `%n.mod.vf = and i32 %blockSize, 7` vs `%n.vec = and i32 %blockSize, -8`. The SCEV of the BETC for the vector body is then unknown in the new case. I think that's what's causing the low overhead loop pass to go wrong, probably the unrolling too.
> > >
> > > Any thoughts?
> >
> >
> > I don't know if that helps the problem overall, but i see yet another seemingly-bogus one-use check restriction there.
> >  https://godbolt.org/z/z8oyUC
> >  I'll see if i can post a patch..
>
>
> Hm, no, doesn't help //much//  https://godbolt.org/z/G24anE
>  Though from SCEV side i'd say that was overall helpful, less `<<Unknown>>`s.


Just to make sure I'm seeing it correctly:

1. The problem(s) we're discussing are independent of VectorCombine.
2. The extra run of EarlyCSE is making InstCombine more effective (that was the intent of this patch).
3. The differences in IR after InstCombine are causing problems for passes later in the pipeline.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75145/new/

https://reviews.llvm.org/D75145