[PATCH] D102437: [LV] NFCI: Move implementation of isScalarWithPredication to LoopVectorizationLegality

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

I like this change, but some of the widening decisions used to feed back to the result of `isScalarWithPredication` which may alter the result in unexpected ways. I'd suggest you try to remove the VF from that function in a separate patch first. If it doesn't break anything anywhere, then we can move ahead with this patch.



================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:262
+  bool canVectorizeWithPredication(Instruction *I,
+                                   bool VectorizeWithPredication);
+
----------------
could you try to make this function `const`? You'd probably need to propagate it to `blockNeedsPredication` and a few other places, but it'd be worth it.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5350
-    const Align Alignment = getLoadStoreAlignment(I);
-    return isa<LoadInst>(I) ? !(isLegalMaskedLoad(Ty, Ptr, Alignment) ||
-                                isLegalMaskedGather(Ty, Alignment))
----------------
CM's `isLegalMaskedLoad`, `isLegalMaskedStore`, `isLegalMaskedGather`, `isLegalMaskedScatter` can also be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102437