[PATCH] D88409: [SVE] Make ElementCount and TypeSize use a new PolySize class

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:31
+  T MinVal;        // The minimum value that it could be.
+  bool IsScalable; // If true, the total value is determined by multiplying
+                   // 'MinVal' by a runtime determinded quantity, 'vscale'.
----------------
You've implemented PolySize more like the current TypeSize than an actual polynomial. That's fine I guess, but I'm currently looking at some interfaces that could use a general StackOffset. Since that currently lives in the AArch64 target, I might create a follow-up patch that generalises this further using a PolyBase class.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:60
   /// Exactly one element.
-  bool isScalar() const { return !Scalable && Min == 1; }
+  bool isScalar() const { return !IsScalable && MinVal == 1; }
   /// One or more elements.
----------------
`isScalar()` only makes sense in the context of ElementCount and doesn't make sense in the context of TypeSize, so I believe this shouldn't live here.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88409