[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:174
+
+  constexpr ElementCount(PolySize<unsigned> V) : PolySize(V) {}
+
----------------
ctetreau wrote:
> Why provide this implicit conversion from base PolySize to ElementCount?
> 
> This could potentially lead to bugs, especially in the future when we introduce true a Polynomial type with exponents and/or multiple variables.
> 
> Is this neccesary to support cases like `ElementCount EC = PolySize::getScalable(4)`?
This is needed for the result from the base class' operators which will be of type `PolySize<unsigned>`. With this conversion, these can be freely converted to ElementCount and avoids having to re-specify all the operators. I've taken a similar approach in D88982 and wasn't entirely sure if this was actually unsafe.

The alternative is redefining the operators and calling the base-class, and making the conversion function private/protected to the deriving class.

I actually tried deriving from some other class, something like:
``` template<typename Derived, typename Base>
class Operators {
  Derived operator+(const Derived &Other) { return (Derived) Base::operator+(Other); }
};

template<typename T> class Derived : public PolyBase<...>,
              public Operators<Derived, PolyBase> { ... };```

but I got C++ template errors that I wasn't able to work around. Not sure if this should even be possible :)


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

https://reviews.llvm.org/D88409