[PATCH] D80919: [CodeGen] Fix warnings in getCopyFromPartsVector

[Diana Picus via Phabricator via llvm-commits]

rovka added inline comments.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:56
+  // answer at compile time.
+  bool isGT(const ElementCount &RHS, bool &Known) {
+    bool Result;
----------------
Is it ever safe to ignore the value of Known? If not, I'd suggest making this method private so people are forced to use knownGT. Alternatively, you could define a 'compare' function and have it return an enum value saying Unknown, GT, LE etc.


================
Comment at: llvm/include/llvm/Support/TypeSize.h:64
+    } else
+      Known = Result = false;
+    return Result;
----------------
To be honest I find this whole sequence a bit difficult to follow. Shouldn't Known only be true if LHS and RHS are either both scalable or both fixed? If so, then you can just say Known = (Scalable == RHS.Scalable). Similar to how it's handled for TypeSize. This looks like another argument in favor of the alternative design with an enum, which just separates these concerns and makes you decide up front whether or not it's possible to make the comparison.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:443
     if (PartEVT.getVectorElementType() == ValueVT.getVectorElementType()) {
-      assert(PartEVT.getVectorNumElements() > ValueVT.getVectorNumElements() &&
+      assert(PartEC.knownGT(ValueEC) &&
              "Cannot narrow, it would be a lossy transformation");
----------------
If we know that the vector element types are the same, can't we just compare the type sizes here and piggy-back on the comparison operators we already have instead of adding new ones?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80919