[PATCH] D87700: [SVE] Replace / operator in TypeSize/ElementCount with coefficientDiv

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 22 09:27:40 PDT 2020


sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:64
 
-  ElementCount &operator/=(unsigned RHS) {
-    Min /= RHS;
-    return *this;
+  ElementCount div(unsigned RHS) const {
+    return ElementCount(Min / RHS, Scalable);
----------------
ctetreau wrote:
> I would suggest renaming this to something more self-documenting. Perhaps `coefficientDiv`? This has the added benefit of making it seem less strange that it's a named divide rather than operator/
Agree that the use of `coefficient` is a good addition. Perhaps this is getting into bike-shedding territory, but then I'd probably prefer the more verbose `divideCoefficientBy` as this reads a bit more intuitive to me (and follows a natural way of phrasing similar to `isKnownMultipleOf`). Any thoughts?


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

https://reviews.llvm.org/D87700



More information about the llvm-commits mailing list