[llvm] [llvm][TypeSize] Fix addition/subtraction in TypeSize. (PR #72979)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Sander de Smalen (sdesmalen-arm)

<details>
<summary>Changes</summary>

It seems TypeSize is currently broken in the sense that:

  TypeSize::Fixed(4) + TypeSize::Scalable(4) => TypeSize::Fixed(8)

without failing its assert that explicitly tests for this case:

  assert(LHS.Scalable == RHS.Scalable && ...);

The reason this fails is that `Scalable` is a static method of class TypeSize, and LHS and RHS are both objects of class TypeSize. So this is evaluating if the pointer to the function Scalable == the pointer to the function Scalable, which is always true because LHS and RHS have the same class.

This patch fixes the issue by renaming `FixedOrScalableQuantity::Scalable` -> `FixedOrScalableQuantity::IsScalable` so that it no longer clashes with the static method in TypeSize.

This patch also adds some new functionality, where it allows adding any quantity to a zero-initialized TypeSize, e.g.

  TypeSize::Scalable(0) + TypeSize::Fixed(4) == TypeSize::Fixed(4)

This makes it easier to implement add-reductions using TypeSize.

---
Full diff: https://github.com/llvm/llvm-project/pull/72979.diff


2 Files Affected:

- (modified) llvm/include/llvm/Support/TypeSize.h (+25-17) 
- (modified) llvm/unittests/Support/TypeSizeTest.cpp (+13) 


``````````diff
diff --git a/llvm/include/llvm/Support/TypeSize.h b/llvm/include/llvm/Support/TypeSize.h
index 8e638f8278e828b..34c6c0ebfef25de 100644
--- a/llvm/include/llvm/Support/TypeSize.h
+++ b/llvm/include/llvm/Support/TypeSize.h
@@ -93,20 +93,26 @@ template <typename LeafTy, typename ValueTy> class FixedOrScalableQuantity {
 
 protected:
   ScalarTy Quantity = 0;
-  bool Scalable = false;
+  bool IsScalable = false;
 
   constexpr FixedOrScalableQuantity() = default;
-  constexpr FixedOrScalableQuantity(ScalarTy Quantity, bool Scalable)
-      : Quantity(Quantity), Scalable(Scalable) {}
+  constexpr FixedOrScalableQuantity(ScalarTy Quantity, bool IsScalable)
+      : Quantity(Quantity), IsScalable(IsScalable) {}
 
   friend constexpr LeafTy &operator+=(LeafTy &LHS, const LeafTy &RHS) {
-    assert(LHS.Scalable == RHS.Scalable && "Incompatible types");
+    assert((LHS.Quantity == 0 || RHS.Quantity == 0 ||
+            LHS.IsScalable == RHS.IsScalable) &&
+           "Incompatible types");
+    LHS.IsScalable = LHS.Quantity ? LHS.IsScalable : RHS.IsScalable;
     LHS.Quantity += RHS.Quantity;
     return LHS;
   }
 
   friend constexpr LeafTy &operator-=(LeafTy &LHS, const LeafTy &RHS) {
-    assert(LHS.Scalable == RHS.Scalable && "Incompatible types");
+    assert((LHS.Quantity == 0 || RHS.Quantity == 0 ||
+            LHS.IsScalable == RHS.IsScalable) &&
+           "Incompatible types");
+    LHS.IsScalable = LHS.Quantity ? LHS.IsScalable : RHS.IsScalable;
     LHS.Quantity -= RHS.Quantity;
     return LHS;
   }
@@ -140,11 +146,11 @@ template <typename LeafTy, typename ValueTy> class FixedOrScalableQuantity {
 
 public:
   constexpr bool operator==(const FixedOrScalableQuantity &RHS) const {
-    return Quantity == RHS.Quantity && Scalable == RHS.Scalable;
+    return Quantity == RHS.Quantity && IsScalable == RHS.IsScalable;
   }
 
   constexpr bool operator!=(const FixedOrScalableQuantity &RHS) const {
-    return Quantity != RHS.Quantity || Scalable != RHS.Scalable;
+    return Quantity != RHS.Quantity || IsScalable != RHS.IsScalable;
   }
 
   constexpr bool isZero() const { return Quantity == 0; }
@@ -155,14 +161,14 @@ template <typename LeafTy, typename ValueTy> class FixedOrScalableQuantity {
 
   /// Add \p RHS to the underlying quantity.
   constexpr LeafTy getWithIncrement(ScalarTy RHS) const {
-    return LeafTy::get(Quantity + RHS, Scalable);
+    return LeafTy::get(Quantity + RHS, IsScalable);
   }
 
   /// Returns the minimum value this quantity can represent.
   constexpr ScalarTy getKnownMinValue() const { return Quantity; }
 
   /// Returns whether the quantity is scaled by a runtime quantity (vscale).
-  constexpr bool isScalable() const { return Scalable; }
+  constexpr bool isScalable() const { return IsScalable; }
 
   /// A return value of true indicates we know at compile time that the number
   /// of elements (vscale * Min) is definitely even. However, returning false
@@ -277,8 +283,8 @@ template <typename LeafTy, typename ValueTy> class FixedOrScalableQuantity {
 //  - ElementCount::getScalable(4) : A scalable vector type holding 4 values.
 class ElementCount
     : public details::FixedOrScalableQuantity<ElementCount, unsigned> {
-  constexpr ElementCount(ScalarTy MinVal, bool Scalable)
-      : FixedOrScalableQuantity(MinVal, Scalable) {}
+  constexpr ElementCount(ScalarTy MinVal, bool IsScalable)
+      : FixedOrScalableQuantity(MinVal, IsScalable) {}
 
   constexpr ElementCount(
       const FixedOrScalableQuantity<ElementCount, unsigned> &V)
@@ -293,8 +299,8 @@ class ElementCount
   static constexpr ElementCount getScalable(ScalarTy MinVal) {
     return ElementCount(MinVal, true);
   }
-  static constexpr ElementCount get(ScalarTy MinVal, bool Scalable) {
-    return ElementCount(MinVal, Scalable);
+  static constexpr ElementCount get(ScalarTy MinVal, bool IsScalable) {
+    return ElementCount(MinVal, IsScalable);
   }
 
   /// Exactly one element.
@@ -315,11 +321,13 @@ class TypeSize : public details::FixedOrScalableQuantity<TypeSize, uint64_t> {
       : FixedOrScalableQuantity(V) {}
 
 public:
-  constexpr TypeSize(ScalarTy Quantity, bool Scalable)
-      : FixedOrScalableQuantity(Quantity, Scalable) {}
+  constexpr TypeSize() : FixedOrScalableQuantity(0, false) {}
 
-  static constexpr TypeSize get(ScalarTy Quantity, bool Scalable) {
-    return TypeSize(Quantity, Scalable);
+  constexpr TypeSize(ScalarTy Quantity, bool IsScalable)
+      : FixedOrScalableQuantity(Quantity, IsScalable) {}
+
+  static constexpr TypeSize get(ScalarTy Quantity, bool IsScalable) {
+    return TypeSize(Quantity, IsScalable);
   }
   static constexpr TypeSize Fixed(ScalarTy ExactSize) {
     return TypeSize(ExactSize, false);
diff --git a/llvm/unittests/Support/TypeSizeTest.cpp b/llvm/unittests/Support/TypeSizeTest.cpp
index 2850705d39f69f5..8ba68159e5f82ac 100644
--- a/llvm/unittests/Support/TypeSizeTest.cpp
+++ b/llvm/unittests/Support/TypeSizeTest.cpp
@@ -81,4 +81,17 @@ static_assert(INT64_C(2) * TSFixed32 == TypeSize::Fixed(64));
 static_assert(UINT64_C(2) * TSFixed32 == TypeSize::Fixed(64));
 static_assert(alignTo(TypeSize::Fixed(7), 8) == TypeSize::Fixed(8));
 
+static_assert(TypeSize() == TypeSize::Fixed(0));
+static_assert(TypeSize() != TypeSize::Scalable(0));
+static_assert(!TypeSize().isScalable());
+static_assert(TypeSize::Fixed(0) + TypeSize::Scalable(8) == TypeSize::Scalable(8));
+static_assert(TypeSize::Scalable(8) + TypeSize::Fixed(0) == TypeSize::Scalable(8));
+static_assert(TypeSize::Fixed(8) + TypeSize::Scalable(0) == TypeSize::Fixed(8));
+static_assert(TypeSize::Scalable(0) + TypeSize::Fixed(8) == TypeSize::Fixed(8));
+
+TEST(TypeSize, FailIncompatibleTypes) {
+  EXPECT_DEBUG_DEATH(TypeSize::Fixed(8) + TypeSize::Scalable(8),
+                     "Incompatible types");
+}
+
 } // namespace

``````````

</details>


https://github.com/llvm/llvm-project/pull/72979