paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

There's probably a few .Min to .getKnownMinValue() conversions where the .Min 
could be dropped (calls to Builder.CreateVectorSplat for example) but they can 
be tidied up as part of a proper activity to reduce the places where 
getKnownMinValue is called.  So other than my suggested updated to 
EC::operator/ the patch looks good to my eye.  Please give other reviewers a 
little more time to provide other insights.



================
Comment at: llvm/include/llvm/Support/TypeSize.h:66
+
+  ElementCount &operator/=(unsigned RHS) {
+    Min /= RHS;
----------------
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().


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

https://reviews.llvm.org/D86065

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to