[PATCH] D86065: [SVE] Make ElementCount members private

[Christopher Tetreault via Phabricator via llvm-commits]

ctetreau accepted this revision.
ctetreau added a comment.

I think this is good to go as is. Assuming @paulwalker-arm is satisfied with leaving operator/ as is, then LGTM.



================
Comment at: llvm/include/llvm/Support/TypeSize.h:66
+
+  ElementCount &operator/=(unsigned RHS) {
+    Min /= RHS;
----------------
paulwalker-arm wrote:
> If you add an assert that the divide is lossless (i.e. MIN % RHS == 0) then asserts like:
> ```
> assert(EltCnt.isKnownEven() && "Splitting vector, but not in half!");
> ```
> are no longer required.  Plus those places which are not checking for lossless division will be automatically protected.  This feels like a sensible default to me.  If somebody wants a truncated result, they can do the maths using getKnownMinValue().
I would prefer that this not be done. This would make this function non-total in an unrecoverable way, and would force everybody to write a bunch of tedious error handling code, even if the normal integer division behavior would have been fine:

```
ElementCount res = LHS.getKnownMinValue() % RHS.getKnownMinValue() == 0 ? LHS / RHS : SomeOtherThing;
```

Everybody knows how integer division works, so I think the lossy behavior will not surprise anybody. An assert might.


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

https://reviews.llvm.org/D86065