paulwalker-arm added inline comments.
================
Comment at: llvm/include/llvm/Support/TypeSize.h:56
+ friend bool operator>(const ElementCount &LHS, const ElementCount &RHS) {
+ assert(LHS.Scalable == RHS.Scalable &&
----------------
david-arm wrote:
> ctetreau wrote:
> > fpetrogalli wrote:
> > > I think that @ctetreau is right on
> > > https://reviews.llvm.org/D85794#inline-793909. We should not overload a
> > > comparison operator on this class because the set it represent it cannot
> > > be ordered.
> > >
> > > Chris suggests an approach of writing a static function that can be used
> > > as a comparison operator, so that we can make it explicit of what kind
> > > of comparison we are doing.
> > In C++, it's common to overload the comparison operators for the purposes
> > of being able to std::sort and use ordered sets. Normally, I would be OK
> > with such usages. However, since `ElementCount` is basically a numeric
> > type, and they only have a partial ordering, I think this is dangerous. I'm
> > concerned that this will result in more bugs whereby somebody didn't
> > remember that vectors can be scalable.
> >
> > I don't have a strong opinion what the comparator function should be
> > called, but I strongly prefer that it not be a comparison operator.
> Hi @ctetreau, yeah I understand. The reason I chose to use operators was
> simply to be consistent with what we have already in TypeSize. Also, we have
> existing "==" and "!=" operators in ElementCount too, although these are
> essentially testing that two ElementCounts are identically the same or not,
> i.e. for 2 given polynomials (a + bx) and (c + dx) we're essentially asking
> if both a==c and b==d.
>
> If I introduce a new comparison function, I'll probably keep the asserts in
> for now, but in general we can do better than simply asserting if something
> is scalable or not. For example, we know that (vscale * 4) is definitely >= 4
> because vscale is at least 1. I'm just not sure if we have that need yet.
I think we should treat the non-equality comparison functions more like
floating point. What we don't want is somebody writing !GreaterThan when they
actually mean LessThan.
Perhaps we should name the functions accordingly (i.e. ogt for
OrderedAndGreaterThan). We will also need matching less than functions since I
can see those being useful when analysing constant insert/extract element
indices which stand a good chance to be a known comparison (with 0 being the
most common index).
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
