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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen 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.
----------------
david-arm wrote:
> 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.
3 would have my preference.


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

https://reviews.llvm.org/D86928