[llvm] [llvm][TypeSize] Consider TypeSize of '0' to be fixed/scalable-agnostic. (PR #72994)

Sander de Smalen via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 21 06:01:28 PST 2023


https://github.com/sdesmalen-arm created https://github.com/llvm/llvm-project/pull/72994

This patch allows adding any quantity to a zero-initialized TypeSize, such
that e.g.:

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

This makes it easier to implement add-reductions using TypeSize where
the 'scalable' flag is not yet known before starting the reduction.

(this PR follows on from #72979)

>From 089811535a4f824340166466b0ed51951c0be0d3 Mon Sep 17 00:00:00 2001
From: Sander de Smalen <sander.desmalen at arm.com>
Date: Tue, 21 Nov 2023 13:50:33 +0000
Subject: [PATCH 1/2] [llvm][TypeSize] Fix addition/subtraction in TypeSize.

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.
---
 llvm/include/llvm/Support/TypeSize.h    | 34 ++++++++++++-------------
 llvm/unittests/Support/TypeSizeTest.cpp |  5 ++++
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/llvm/include/llvm/Support/TypeSize.h b/llvm/include/llvm/Support/TypeSize.h
index 8e638f8278e828b..7b8825e9c29b891 100644
--- a/llvm/include/llvm/Support/TypeSize.h
+++ b/llvm/include/llvm/Support/TypeSize.h
@@ -93,20 +93,20 @@ 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.IsScalable == RHS.IsScalable && "Incompatible types");
     LHS.Quantity += RHS.Quantity;
     return LHS;
   }
 
   friend constexpr LeafTy &operator-=(LeafTy &LHS, const LeafTy &RHS) {
-    assert(LHS.Scalable == RHS.Scalable && "Incompatible types");
+    assert(LHS.IsScalable == RHS.IsScalable && "Incompatible types");
     LHS.Quantity -= RHS.Quantity;
     return LHS;
   }
@@ -140,11 +140,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 +155,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 +277,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 +293,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 +315,11 @@ class TypeSize : public details::FixedOrScalableQuantity<TypeSize, uint64_t> {
       : FixedOrScalableQuantity(V) {}
 
 public:
-  constexpr TypeSize(ScalarTy Quantity, bool Scalable)
-      : FixedOrScalableQuantity(Quantity, Scalable) {}
+  constexpr TypeSize(ScalarTy Quantity, bool IsScalable)
+      : FixedOrScalableQuantity(Quantity, IsScalable) {}
 
-  static constexpr TypeSize get(ScalarTy Quantity, bool Scalable) {
-    return TypeSize(Quantity, Scalable);
+  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..eeb6f2424a21b0f 100644
--- a/llvm/unittests/Support/TypeSizeTest.cpp
+++ b/llvm/unittests/Support/TypeSizeTest.cpp
@@ -81,4 +81,9 @@ 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));
 
+TEST(TypeSize, FailIncompatibleTypes) {
+  EXPECT_DEBUG_DEATH(TypeSize::Fixed(8) + TypeSize::Scalable(8),
+                     "Incompatible types");
+}
+
 } // namespace

>From 8f60f1fc5ae8eccea7d875305212c511bf6c71eb Mon Sep 17 00:00:00 2001
From: Sander de Smalen <sander.desmalen at arm.com>
Date: Tue, 21 Nov 2023 13:51:09 +0000
Subject: [PATCH 2/2] [llvm][TypeSize] Consider TypeSize of '0' to be
 fixed/scalable-agnostic.

This patch allows adding any quantity to a zero-initialized TypeSize, such
that e.g.:

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

This makes it easier to implement add-reductions using TypeSize where
the 'scalable' flag is not yet known before starting the reduction.
---
 llvm/include/llvm/Support/TypeSize.h    | 12 ++++++++++--
 llvm/unittests/Support/TypeSizeTest.cpp | 10 ++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/Support/TypeSize.h b/llvm/include/llvm/Support/TypeSize.h
index 7b8825e9c29b891..34c6c0ebfef25de 100644
--- a/llvm/include/llvm/Support/TypeSize.h
+++ b/llvm/include/llvm/Support/TypeSize.h
@@ -100,13 +100,19 @@ template <typename LeafTy, typename ValueTy> class FixedOrScalableQuantity {
       : Quantity(Quantity), IsScalable(IsScalable) {}
 
   friend constexpr LeafTy &operator+=(LeafTy &LHS, const LeafTy &RHS) {
-    assert(LHS.IsScalable == RHS.IsScalable && "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.IsScalable == RHS.IsScalable && "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;
   }
@@ -315,6 +321,8 @@ class TypeSize : public details::FixedOrScalableQuantity<TypeSize, uint64_t> {
       : FixedOrScalableQuantity(V) {}
 
 public:
+  constexpr TypeSize() : FixedOrScalableQuantity(0, false) {}
+
   constexpr TypeSize(ScalarTy Quantity, bool IsScalable)
       : FixedOrScalableQuantity(Quantity, IsScalable) {}
 
diff --git a/llvm/unittests/Support/TypeSizeTest.cpp b/llvm/unittests/Support/TypeSizeTest.cpp
index eeb6f2424a21b0f..0331b4c00d5f1b1 100644
--- a/llvm/unittests/Support/TypeSizeTest.cpp
+++ b/llvm/unittests/Support/TypeSizeTest.cpp
@@ -81,6 +81,16 @@ 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");



More information about the llvm-commits mailing list