[PATCH] D66871: [SVE] MVT scalable size queries

[Graham Hunter via Phabricator via llvm-commits]

huntergr added inline comments.


================
Comment at: llvm/utils/TableGen/CodeGenDAGPatterns.cpp:520
     // to a non-vector, it should return false (to avoid removal).
     if (A.isVector() != B.isVector())
       return false;
----------------
huntergr wrote:
> huntergr wrote:
> > rovka wrote:
> > > Why does LE care about isVector and LT doesn't?
> > > 
> > > In any case, I think both functions would benefit from being rewritten using std::tie and the corresponding operators.
> > LT seems to be used with filtered min/max functions below and will only operate on either only vector or only scalar MVTs, so never compares between them.
> > 
> > LE is then explicitly used to remove MVTs from Typesets that may have mixed scalar and vector MVTs, so needs to avoid comparing them.
> > 
> > I'll try reworking them with std::tie.
> I've reworked them to use `std::tie`, which meant I had to make the size query methods return `const TypeSize`. Everything still seems to work (make check-all, plus an LNT run).
> 
> I don't think the `std::tie` comparisons are strictly equivalent to what was there before -- if `A.getScalarSizeInBits()` is greater than `B.getScalarSizeInBits()` in `LT` then I think the std::tie comparison would return false immediately instead of progressing to the second comparison as the previous code would.
...actually, I think it works out the same anyway, since you would only get a `true` result from `LT` in the second case if the scalar sizes were equal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66871