[PATCH] D134032: [SROA] Check typeSizeEqualsStoreSize in isVectorPromotionViable

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:1936
       continue;
+    if (!DL.typeSizeEqualsStoreSize(Ty))
+      continue;
----------------
I should probably add some explanatory code comment here (given that this solution is accepted).

Background to why I used the typeSizeEqualsStoreSize check:

The idea is to make a similar check to what is done at line 1859 ("While the definition of LLVM vectors is bitpacked, we don't support sizes that aren't byte sized."), although that isn't exactly what testing for typeSizeEqualsStoreSize is doing.

Not sure if it would be better to check for "byte sized". It might work as well (but it would complicate stuff for our out-of-target with 16 bit bytes if hard-coding another "8" here). 

Anyhow, for the "VectorSize / ElementSize" calculation below to result in a vector with the same size as the original vector I think typeSizeEqualsStoreSize needs to be fulfilled (or we could change the ElementSize calculation to just use getTypeSizeInBits instead of  getTypeStoreSizeInBits, but then I suspect that the transform might be unsafe in case typeSizeEqualsStoreSize isn't fulfilled). So checking for typeSizeEqualsStoreSize seemed like a defensive approach here to avoid the corner cases without sacrificing the normal cases that we want to handle (and that is regression tested).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134032