[PATCH] D73286: [llvm][VectorUtils] Tweak VFShape for scalable vector functions.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Sorry for my delay. Some comments below.



================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:170
+Optional<VFInfo> tryDemangleForVFABI(StringRef MangledName,
+                                     const Module *M = nullptr);
 
----------------
Any reason one would not pass `M` here? If not, I would remove the default and maybe even make it required (via reference).


================
Comment at: llvm/lib/Analysis/VFABIDemangling.cpp:302
+  using Container = SmallVector<VectorType *, 2>;
+  Container VecTys;
+  if (auto RetTy = dyn_cast<VectorType>(Signature->getReturnType()))
----------------
Nit: `using` is not needed.


================
Comment at: llvm/lib/Analysis/VFABIDemangling.cpp:314
+  const ElementCount EC = VecTys[0]->getElementCount();
+  return std::all_of(VecTys.begin() + 1, VecTys.end(), [&EC](VectorType *VTy) {
+    return (EC == VTy->getElementCount());
----------------
`llvm::all_of`


================
Comment at: llvm/lib/Analysis/VFABIDemangling.cpp:317
+  });
+}
+
----------------
Nit: `auto *` and `auto &` wherever possible. 


================
Comment at: llvm/lib/Analysis/VFABIDemangling.cpp:334
+
+  return {/*Min=*/1, /*Scalable=*/false};
+}
----------------
Let's be even more verbose here: `ElementCount{...}`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73286