[PATCH] D86928: [SVE][CodeGen] Fix TypeSize/ElementCount related warnings in sve-split-store.ll

[David Sherwood via Phabricator via llvm-commits]

david-arm marked an inline comment as done.
david-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:557
     ReplaceNode(SDValue(Node, 0), Result);
-  } else if (StWidth & (StWidth - 1)) {
+  } else if (!isPowerOf2_64(StWidth.getKnownMinSize())) {
     // If not storing a power-of-2 number of bits, expand as two stores.
----------------
sdesmalen wrote:
> You're using `getKnownMinSize()` here but using `getFixedSize()` below on line 560. Is this missing a condition, e.g.:
> ```!StVT.isScalableVector() && !isPowerOf2_64(StWidth.getFixedSize())```
> or perhaps (because it seems like this only applies to integers):
> ```!StVT.isVector() && !isPowerOf2_64(StWidth.getFixedSize())```
Yes, I used the known min size here because it could be a scalable vector (since we haven't hit the final else case yet). The equivalent load function has the same problem (see https://reviews.llvm.org/D86697 that was checked in recently). So we either:

1. Restructure the code so that the vector case comes first.
2. Use getKnownMinSize.
3. Use !StVT.isVector() && !isPowerOf2_64(StWidth.getFixedSize())

I'm happy to go with either way - whichever you and @efriedma prefer really.


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

https://reviews.llvm.org/D86928