[PATCH] D51428: SLPVectorizer: Fix assert with different sized address spaces

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:6402
           auto *SCEVJ = SE->getSCEV(GEPList[J]);
-          if (isa<SCEVConstant>(SE->getMinusSCEV(SCEVI, SCEVJ))) {
+          if (SCEVJ->getType() == SCEVI->getType() &&
+              isa<SCEVConstant>(SE->getMinusSCEV(SCEVI, SCEVJ))) {
----------------
ABataev wrote:
> arsenm wrote:
> > ABataev wrote:
> > > I don't think this is correct. Instead, you should try to collect GEPs with the different address spaces into different nodes
> > That's what I thought at first, but his isn't vectorizing the GEP itself as far as I can see. It only cares about the index operation, which could be the same size between different address spaces
> Yes, I see now. But I still think this not quite correct, just like the original code. As I understand, it tries to vectorize indices with the same base. So, when we gather GEPs, we just don't need to use `GetUnderlyingObject(GEP->getPointerOperand(), *DL)` as the key, just `GEP->getPointerOperand()`. I think, we should not dig deep into the base of the GEP.
This isn't considering the bases at all, which makes sense based on this comment:
    // Vectorize the index computations of getelementptr instructions. This
    // is primarily intended to catch gather-like idioms ending at
    // non-consecutive loads.


https://reviews.llvm.org/D51428