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

[Sanne Wouda via Phabricator via llvm-commits]

sanwou01 added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4545
+      } else {
+        Module *M = F->getParent();
+        Type *Tys[] = {FixedVectorType::get(CI->getType(), E->Scalars.size())};
----------------
fpetrogalli wrote:
> `M` is used only inside `getDeclaration`, no need to declare a variable for it.
I'm just moving some pre-existing code into an else clause here. I can still tidy this up if you prefer, but perhaps in a follow-up NFC?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5110
+
+  return true;
+}
----------------
fpetrogalli wrote:
> I suspect there are many things that may fail here, other than not having a mapping in `VFDatabase` or having pointer arguments. I think it would be safer to reverse the logic, and have the function return false by default, and return true if VFDatabase is not empty and there is no pointer arguments.
As discussed below, we can rely on the existing readonly attribute here, so this helper function is no longer needed, and removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82550