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

[Richard Sandiford via Phabricator via llvm-commits]

rsandifo-arm added a comment.

In D86065#2241146 <https://reviews.llvm.org/D86065#2241146>, @david-arm wrote:

> Hi @ctetreau, ok for now I'm going to completely remove the operators and revert the code using those operators to how it was before. I'm not sure what you mean about the predicate functions so I've left those for now, since they aren't needed for this patch. The purpose of this patch was originally supposed to be mechanical anyway - just making members private. I only added the operators as an after-thought really, just to be consistent with how TypeSize dealt with the identical problem. For what it's worth, I believe that GCC solved this exact same problem by adding two types of comparison functions - one set that absolutely wanted an answer to ">,<,>=,<=" and asserted if it wasn't known at compile time, and another set of comparison functions that returned an additional boolean value indicating whether the answer was known or not. Perhaps my knowledge is out of date, but I believe this was the accepted solution and seemed to work well.

FWIW, the GCC scheme is to have one set of functions `maybe_<cond>` that are true if a condition *might* hold (i.e. would hold for one possible value of the runtime indeterminates), and another set of functions `known_<cond>` (originally `must_<cond>`) that are true if a condition *always* holds (i.e. would hold for all possible values of the runtime indeterminates).  `known_le` is a partial order but `maybe_le` is not (because it isn't antisymmetric).  Having both is redundant with `!`, since e.g. `known_le` is the opposite of `maybe_gt`, but it seemed more readable to allow every condition to be expressed positively.

Like you say, there is also a test for whether two values are ordered by `known_le`, and there are also some operations like `ordered_min` and `ordered_max` that assert if the values aren't ordered by `known_le`.

[Sorry for the post-commit comment, but it's related to something that wasn't part of the commit.]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86065