[PATCH] D29717: [LoopVectorize] Added address space check when analysing interleaved accesses

[Karl-Johan Karlsson via Phabricator via llvm-commits]

Ka-Ka added a subscriber: llvm-commits.
Ka-Ka removed a reviewer: llvm-commits.
Ka-Ka added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5856
 
+      // Ignore A if the memory object of A and B don't belong to the same
+      // address space
----------------
efriedma wrote:
> mkuper wrote:
> > mkuper wrote:
> > > Hold on.
> > > Shouldn't this fail the distance check? How can we have pointers in different address spaces where we succeed in computing the distance?
> > Err, sorry, actually went ahead and read the bug. That was the wrong question to ask.
> > 
> > The right question:
> > Do getMinusSCEV/getAddExpr require pointers to be in the same address space, as an API requirement? I mean, are they supposed to assert, or should they return something sane? Just trying to figure out whether this should be fixed on the caller side or the callee side.
> > (This code is a good idea anyway, no point in wasting SCEV's time when we know we won't get a constant distance...)
> SCEV getAddExpr doesn't actually check the address space explicitly, but the operation doesn't make sense unless all the operands have the same size.
Do you want me to try to add address space checks in getMinusSCEV/getAddExpr as well?
Is sounds like it is a larger task to add address space checks in ScalarEvolution. Is is better to do this in a separate review/commit?



================
Comment at: test/Transforms/LoopVectorize/AArch64/pr31900.ll:1
+; RUN: opt -mtriple=aarch64-apple-ios -loop-vectorize -enable-interleaved-mem-accesses -o /dev/null %s
+
----------------
mssimpso wrote:
> I would prefer that we actually check the resulting IR rather than checking for "no crash". Maybe force vectorization (-force-vector-width) and then check that the loads are scalarized.
Sure, I will update the testcase in the next patch.


https://reviews.llvm.org/D29717