[llvm] r189007 - Teach the SLP vectorizer the correct way to check for consecutive access

[Chandler Carruth]

On Thu, Aug 22, 2013 at 9:32 AM, Nadav Rotem <nrotem at apple.com> wrote:

> Hi Chandler,
>
> Thanks for working on this. Did you measure the difference in compile time
> of this patch on the LLVM test suite?  The original code avoided calling
> accumulateConstantOffset when possible because it is somewhat expensive.  I
> like your pass-wide memoization idea. :)
>

No, because the new version is at least correct. ;] I spot checked and
didn't see any huge regressions. Since you're much more familiar with this
code, I'll leave fine tuning the performance to you, I merely wanted to
correct a miscompile we were seeing in the wild and that blocked our
release process.


> >
> > +  APInt OffsetA(PtrBitWidth, 0), OffsetB(PtrBitWidth, 0);
> > +  PtrA = PtrA->stripAndAccumulateInBoundsConstantOffsets(*DL, OffsetA);
> > +  PtrB = PtrB->stripAndAccumulateInBoundsConstantOffsets(*DL, OffsetB);
> >
> > -  // Check if PtrB is the base and PtrA is a constant offset.
> > -  if (GepA && GepA->getPointerOperand() == PtrB) {
> > -    APInt Offset(BW, 0);
> > -    if (GepA->accumulateConstantOffset(*DL, Offset))
> > -      return Offset.getSExtValue() == -Sz;
> > -    return false;
> > -  }
> > +  APInt OffsetDelta = OffsetB - OffsetA;
> > +
> > +  // Check if they are based on the same pointer. That makes the offsets
> > +  // sufficient.
> > +  if (PtrA == PtrB)
> > +    return OffsetDelta == Size;
> >
>
> What happens if both PtrA and PtrB are zero ?  This means that both
> OffsetA and OffsetB are garbage. You need to check if at least one of the
> calls to stripAndAccumulate succeeded.
>

Instead, the stripAndAccumulate calls should ensure the offset is preserved
when we find a non-constant GEP. That way, a partial walk can still remove
some layers of GEPs before we hand it off to SCEV. I've fixed this in
r189188 so that we should in the worst case just get zero offsets here. If
you have a test case for this, please check it in or send it my way. Good
catch by inspection though.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130825/7a927973/attachment.html>