[PATCH] D75145: [PassManager] adjust VectorCombine placement

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D75145#1909694 <https://reviews.llvm.org/D75145#1909694>, @spatel wrote:

> 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
> > >
> >
>


Oh wait, the bottom source is the good one? I misread that completely.

>>>> 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