[PATCH] D71919: [LoopVectorize] Disable single stride access predicates when gather loads are available.

[Dave Green via Phabricator via llvm-commits]

dmgreen marked an inline comment as done.
dmgreen added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:2287
+  if (LoadInst *LI = dyn_cast<LoadInst>(MemAccess)) {
     Ptr = LI->getPointerOperand();
+    if (TTI && TTI->isLegalMaskedGather(LI->getType(),
----------------
Ayal wrote:
> Separate the existing `Value *Ptr = getLoadStorePointerOperand(MemAccess);  if (!Ptr) return;`  part from the new gather/scatter consideration?
> 
> Would have been nice to reuse LV's `isLegalGatherOrScatter(Value *V)`, or perhaps refactor it into `if (TTI && TTI->isLegalGatherOrScatter(MemAccess)) return;`?
> 
> Worth informing of filtered strides with LLVM_DEBUG messages.
> 
> (Can check if Ptr is already in SymbolicStrides and exit early; unrelated change.)
Yeah, my original version of this patch had isLegalGatherOrScatter passed as a functor from the LoopVectoriser through to LAA. It felt quite ugly though, this seemed a lot simpler to just pass TTI (plus I was looking as the SVE patch set and they had also added TTI to LAA for different reasons).

But adding isLegalGatherOrScatter to TTI sounds like a better idea. That will make this cleaner.


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

https://reviews.llvm.org/D71919