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

Sander de Smalen via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 21 05:58:56 PST 2023


https://github.com/sdesmalen-arm updated https://github.com/llvm/llvm-project/pull/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] [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



More information about the llvm-commits mailing list