[PATCH] D90713: [TypeSize] Extend UnivariateLinearPolyBase with add/sub methods

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 4 04:57:48 PST 2020


sdesmalen added a comment.

In D90713#2373445 <https://reviews.llvm.org/D90713#2373445>, @dmgreen wrote:

> Can you just overload the + operator? Would that make the calling code more confusing?

In our SVE/SVE2 sync-up calls we settled on the approach of not using overloaded operators if the operation is not mathematically correct, and instead use named methods. This was the reason for removing `operator/` from TypeSize and replacing it with `divideCoefficientBy`, because it only divides the (compile-time known) minimum value/coefficient and not the full runtime value in `(coefficient * vscale) / RHS`.

> I don't know this class very well so feel free to ignore me but I feel like it should be called "getWithIncrement" or something like it. That would be more like Type methods I've seen in the past like getWithNewBitwidth. Like I said if you are going for something else feel free to ignore me though.

I like `getWithIncrement`, it seems a better name than `add` which indeed feels too similar to `operator+`. I wasn't able to think of a better name for it and therefore went with `add`, but I like this better. Thanks for the suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90713



More information about the llvm-commits mailing list