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

[David Sherwood via Phabricator via llvm-commits]

david-arm marked 3 inline comments as done.
david-arm added inline comments.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:229
 
+  bool isPowerOf2() const { return (MinSize & (MinSize - 1)) == 0; }
+
----------------
efriedma wrote:
> This name is possibly misleading; the runtime size might not be a power of two even if isPowerOf2() returns true.
> 
> In the one place you're using it, the type can't be a vector type, anyway.
Good point and I've removed this function so it's calculated explicitly in the code as before


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1663
+      MPI = MLD->getPointerInfo().getWithOffset(
+          LoMemVT.getStoreSize().getFixedSize());
 
----------------
efriedma wrote:
> I thought we added some form of pointer increment that took care of the MachinePointerInfo?  Or am I misremembering?
> 
> Semantically, this looks correct.
We do have IncrementPointer, but it sadly doesn't do the same thing. The code above increments the pointer using IncrementMemoryAddress, which attempts to deal with compressed memory based upon the mask.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:9646
   EVT LoVT, HiVT;
-  if (VTNumElts > EnvNumElts) {
+  if (VTNumElts.Min > EnvNumElts.Min) {
     LoVT = EnvVT;
----------------
efriedma wrote:
> Should we add a comparison operator to ElementCount, like the one we have for TypeSize?
Based on discussions in the parent patch (https://reviews.llvm.org/D86065) I think the consensus is to avoid the use of operators for comparisons. Perhaps we can revisit this at some point when we have some kind of agreed set of interfaces?


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

https://reviews.llvm.org/D86697