[PATCH] D149842: Scalarizer: limit scalarization for small element types

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle marked 2 inline comments as done.
nhaehnle added a comment.

Thank you all for the reviews. I've addressed the remaining small comments as part of the commit.

In D149842#4407192 <https://reviews.llvm.org/D149842#4407192>, @arsenm wrote:

> Also a TTI control and / or pass parameter would be better than cl::opt

All of the existing cl::opts as well as this new one can be set as a pass parameter.



================
Comment at: llvm/lib/Transforms/Scalar/Scalarizer.cpp:220
+/// described in @p VS.
+static Value *concatenate(IRBuilder<> &Builder, ArrayRef<Value *> Fragments,
+                          const VectorSplit &VS, Twine Name) {
----------------
arsenm wrote:
> I swear this function exists somewhere else but I can't seem to find it
Yeah. I was looking for it but couldn't find one. A version exists for SelectionDAG and I believe for G-MIR


================
Comment at: llvm/lib/Transforms/Scalar/Scalarizer.cpp:608
+  } else {
+    Split.NumPacked = ScalarizeMinBits / ElemTy->getScalarSizeInBits();
+    if (Split.NumPacked >= NumElems)
----------------
foad wrote:
> Do you have any test cases for when this division is not exact?
I have now :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149842