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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev 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))) {
----------------
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.


https://reviews.llvm.org/D51428