[PATCH] D82550: [SLPVectorizer] handle vectorized lib functions

[Sanne Wouda via Phabricator via llvm-commits]

sanwou01 added inline comments.


================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:227
                                SmallVectorImpl<VFInfo> &Mappings) {
+    if (CI.isIndirectCall())
+      return;
----------------
fhahn wrote:
> unrelated to the change? If so, better to split it off (and perhaps add a test independent of SLP?)
The underlying problem here is that if getVFABIMappings is called with an indirect call instruction, it'll call getName() on a nullptr. I suspect other callers take care not to do that, making it a latent bug without the rest of this patch.

Agreed though that it deserves its own patch and test; not sure if I can write a test independent of SLP though. 


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5098
 
+static bool isSideeffectIntrinsic(Instruction *I) {
+  auto *II = dyn_cast<IntrinsicInst>(I);
----------------
fhahn wrote:
> This is a NFC change, right? Would be good to split off and commit separately. Looks like an independent improvement.
I've split the NFC bit off and will commit that first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82550