[PATCH] D24271: [LV] Don't mark pointers used by scalarized memory accesses uniform

[Matthew Simpson via llvm-commits]

mssimpso added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5290
@@ +5289,3 @@
+  if (auto *Ptr = getPointerOperand(I))
+    return isConsecutivePtr(Ptr);
+  return false;
----------------
mkuper wrote:
> I think this will also return true for "reverse consecutive".
> Is that true? If so, is it intentional?
Good point. I'll check this out and add a test case, regardless.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5309
@@ +5308,3 @@
+  // scalarized.
+  if (SI && blockNeedsPredication(SI->getParent()) && !isMaskRequired(SI))
+    return true;
----------------
mkuper wrote:
> Any chance this and vectorizeMemoryInstruction can share code?
> Having this duplicated scares me a bit.
Sure, we can pull this out into a shared helper function.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5315-5317
@@ +5314,5 @@
+  auto &DL = I->getModule()->getDataLayout();
+  auto *ScalarType = LI ? LI->getType() : SI->getValueOperand()->getType();
+  if (DL.getTypeAllocSize(ScalarType) != DL.getTypeStoreSize(ScalarType))
+    return true;
+
----------------
Ideally, we could share this check with vectorizeMemoryInstruction as well. But the version in vectorizeMemoryInstruction uses the actual VF. We don't have the VF available to us yet in Legality. And I think the VF selection is to some extent affected by the uniforms. So the check here is essentially the same as the one in vectorizeMemoryInstruction, but with VF = 1. I'm wondering if it makes sense to replace the check in vectorizeMemoryInstruction with this one. Do you know of any cases that we would miss vectorizing?


https://reviews.llvm.org/D24271