[PATCH] D98967: [Analysis]Add getPointersDiff function to improve compile time.

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3155
         }
-        const SCEV *Scev0 = SE->getSCEV(Ptr0);
-        const SCEV *ScevN = SE->getSCEV(PtrN);
-        const auto *Diff =
-            dyn_cast<SCEVConstant>(SE->getMinusSCEV(ScevN, Scev0));
-        uint64_t Size = DL->getTypeAllocSize(ScalarTy);
+        Optional<int> Dist = getPointersDiff(Ptr0, PtrN, *DL, *SE);
         // Check that the sorted pointer operands are consecutive.
----------------
ABataev wrote:
> uabelho wrote:
> > Hi,
> > I'm seeing a miscompile with this patch for my out-of-tree target and I wonder if we perhaps should pass StrictCheck=true for this getPointerDiff call.
> > 
> > In the case I see I have two loads where Dist is calculated as 1, but actually it's 3/2, and the resulting vectorized load then doesn't load the correct words.
> > 
> > I'm having some troubles reproducing this on an in-tree target but I don't see why it shouldn't be possible.
> > 
> > I see that if I do pass StrictCheck=true here the miscopile goes away.
> > 
> > @ABataev , what do you think about this? Is it possible that the code broke here?
> It should not be broken here. `sortPtrAccesses` should call `getPointersDiff` with `StrictCheck` already after 040c60d9b69e2ad570556f255a746929a4b10e82 from Richard Smith and we can get here only if `getPointerDiff` with `StrictCheck==true` returned defined result already. Try to update your repo. Plus, this code is for stores, not for loads. Anyway, your problem should be fixed already, just update
Aha, yep  040c60d9b69e2 seems to help. Great!
(And yes, the problem I saw was with the load case at line 2880 and not here in store.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98967