[Lldb-commits] [lldb] 8a8a2dd - [lldb/Utility] Simplify Scalar handling of float types

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Aug 20 07:26:14 PDT 2020


Author: Pavel Labath
Date: 2020-08-20T16:26:02+02:00
New Revision: 8a8a2dd3165e63b29e725526745427c6434f0654

URL: https://github.com/llvm/llvm-project/commit/8a8a2dd3165e63b29e725526745427c6434f0654
DIFF: https://github.com/llvm/llvm-project/commit/8a8a2dd3165e63b29e725526745427c6434f0654.diff

LOG: [lldb/Utility] Simplify Scalar handling of float types

Similarly to D85836, collapse all Scalar float types to a single enum
value, and use APFloat semantics to differentiate between. This
simplifies the code, and opens to door to supporting other floating
point semantics (which would be needed for fully supporting
architectures with more interesting float types such as PPC).

Differential Revision: https://reviews.llvm.org/D86220

Added: 
    

Modified: 
    lldb/include/lldb/Utility/Scalar.h
    lldb/source/Utility/Scalar.cpp
    lldb/unittests/Utility/ScalarTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Utility/Scalar.h b/lldb/include/lldb/Utility/Scalar.h
index d1e0fdd888df..4e0505e669dd 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -45,8 +45,6 @@ class Scalar {
     e_sint,
     e_uint,
     e_float,
-    e_double,
-    e_long_double
   };
 
   // Constructors and Destructors
@@ -70,8 +68,8 @@ class Scalar {
       : m_type(e_uint), m_integer(sizeof(v) * 8, uint64_t(v), false),
         m_float(0.0f) {}
   Scalar(float v) : m_type(e_float), m_float(v) {}
-  Scalar(double v) : m_type(e_double), m_float(v) {}
-  Scalar(long double v) : m_type(e_long_double), m_float(double(v)) {
+  Scalar(double v) : m_type(e_float), m_float(v) {}
+  Scalar(long double v) : m_type(e_float), m_float(double(v)) {
     bool ignore;
     m_float.convert(llvm::APFloat::x87DoubleExtended(),
                     llvm::APFloat::rmNearestTiesToEven, &ignore);
@@ -114,15 +112,13 @@ class Scalar {
 
   void GetValue(Stream *s, bool show_type) const;
 
-  bool IsValid() const {
-    return (m_type >= e_sint) && (m_type <= e_long_double);
-  }
+  bool IsValid() const { return (m_type >= e_sint) && (m_type <= e_float); }
 
   /// Convert to an integer with \p bits and the given signedness.
   void TruncOrExtendTo(uint16_t bits, bool sign);
 
   bool IntegralPromote(uint16_t bits, bool sign);
-  bool FloatPromote(Scalar::Type type);
+  bool FloatPromote(const llvm::fltSemantics &semantics);
 
   bool MakeSigned();
 
@@ -136,8 +132,6 @@ class Scalar {
   static Scalar::Type
   GetValueTypeForUnsignedIntegerWithByteSize(size_t byte_size);
 
-  static Scalar::Type GetValueTypeForFloatWithByteSize(size_t byte_size);
-
   // All operators can benefits from the implicit conversions that will happen
   // automagically by the compiler, so no temporary objects will need to be
   // created. As a result, we currently don't need a variety of overloaded set
@@ -257,8 +251,13 @@ class Scalar {
 
   static Type PromoteToMaxType(Scalar &lhs, Scalar &rhs);
 
-  using IntPromotionKey = std::pair<unsigned, bool>;
-  IntPromotionKey GetIntKey() const;
+  enum class Category { Void, Integral, Float };
+  static Category GetCategory(Scalar::Type type);
+
+  using PromotionKey = std::tuple<Category, unsigned, bool>;
+  PromotionKey GetPromoKey() const;
+
+  static PromotionKey GetFloatPromoKey(const llvm::fltSemantics &semantics);
 
 private:
   friend const Scalar operator+(const Scalar &lhs, const Scalar &rhs);

diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 32082b50eaa8..1a27808068d6 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -26,17 +26,11 @@ using namespace lldb_private;
 using llvm::APFloat;
 using llvm::APInt;
 
-namespace {
-enum class Category { Void, Integral, Float };
-}
-
-static Category GetCategory(Scalar::Type type) {
+Scalar::Category Scalar::GetCategory(Scalar::Type type) {
   switch (type) {
   case Scalar::e_void:
     return Category::Void;
   case Scalar::e_float:
-  case Scalar::e_double:
-  case Scalar::e_long_double:
     return Category::Float;
   case Scalar::e_sint:
   case Scalar::e_uint:
@@ -52,49 +46,62 @@ static bool IsSigned(Scalar::Type type) {
     return false;
   case Scalar::e_sint:
   case Scalar::e_float:
-  case Scalar::e_double:
-  case Scalar::e_long_double:
     return true;
   }
   llvm_unreachable("Unhandled type!");
 }
 
-Scalar::IntPromotionKey Scalar::GetIntKey() const {
-  assert(GetCategory(GetType()) == Category::Integral);
-  return {m_integer.getBitWidth(), !IsSigned(m_type)};
+Scalar::PromotionKey Scalar::GetPromoKey() const {
+  Category cat = GetCategory(m_type);
+  switch (cat) {
+  case Category::Void:
+    return {cat, 0, false};
+  case Category::Integral:
+    return {cat, m_integer.getBitWidth(), !IsSigned(m_type)};
+  case Category::Float:
+    return GetFloatPromoKey(m_float.getSemantics());
+  }
+  llvm_unreachable("Unhandled category!");
+}
+
+Scalar::PromotionKey Scalar::GetFloatPromoKey(const llvm::fltSemantics &sem) {
+  static const llvm::fltSemantics *const order[] = {
+      &APFloat::IEEEsingle(), &APFloat::IEEEdouble(),
+      &APFloat::x87DoubleExtended()};
+  for (const auto &entry : llvm::enumerate(order)) {
+    if (entry.value() == &sem)
+      return {Category::Float, entry.index(), false};
+  }
+  llvm_unreachable("Unsupported semantics!");
 }
 
 // Promote to max type currently follows the ANSI C rule for type promotion in
 // expressions.
 Scalar::Type Scalar::PromoteToMaxType(Scalar &lhs, Scalar &rhs) {
   const auto &Promote = [](Scalar &a, const Scalar &b) {
-    if (GetCategory(b.GetType()) == Category::Integral)
+    switch (GetCategory(b.GetType())) {
+    case Category::Void:
+      break;
+    case Category::Integral:
       a.IntegralPromote(b.UInt128(APInt()).getBitWidth(),
                         IsSigned(b.GetType()));
-    else
-      a.FloatPromote(b.GetType());
+      break;
+    case Category::Float:
+      a.FloatPromote(b.m_float.getSemantics());
+    }
   };
 
-  // Extract the types of both the right and left hand side values
-  Scalar::Type lhs_type = lhs.GetType();
-  Scalar::Type rhs_type = rhs.GetType();
-
-  if (GetCategory(lhs_type) == Category::Integral &&
-      GetCategory(rhs_type) == Category::Integral) {
-    IntPromotionKey lhs_key = lhs.GetIntKey();
-    IntPromotionKey rhs_key = rhs.GetIntKey();
-    if (lhs_key > rhs_key)
-      Promote(rhs, lhs);
-    else if (rhs_key > lhs_key)
-      Promote(lhs, rhs);
-  } else if (lhs_type > rhs_type)
+  PromotionKey lhs_key = lhs.GetPromoKey();
+  PromotionKey rhs_key = rhs.GetPromoKey();
+
+  if (lhs_key > rhs_key)
     Promote(rhs, lhs);
-  else if (lhs_type < rhs_type)
+  else if (rhs_key > lhs_key)
     Promote(lhs, rhs);
 
   // Make sure our type promotion worked as expected
-  if (lhs.GetType() == rhs.GetType())
-    return lhs.GetType(); // Return the resulting max type
+  if (lhs.GetPromoKey() == rhs.GetPromoKey())
+    return lhs.GetType(); // Return the resulting type
 
   // Return the void type (zero) if we fail to promote either of the values.
   return Scalar::e_void;
@@ -155,11 +162,7 @@ size_t Scalar::GetByteSize() const {
   case e_uint:
     return (m_integer.getBitWidth() / 8);
   case e_float:
-    return sizeof(float_t);
-  case e_double:
-    return sizeof(double_t);
-  case e_long_double:
-    return sizeof(long_double_t);
+    return m_float.bitcastToAPInt().getBitWidth() / 8;
   }
   return 0;
 }
@@ -203,29 +206,13 @@ void Scalar::TruncOrExtendTo(uint16_t bits, bool sign) {
   m_type = GetBestTypeForBitSize(bits, sign);
 }
 
-static const llvm::fltSemantics &GetFltSemantics(Scalar::Type type) {
-  switch (type) {
-  case Scalar::e_void:
-  case Scalar::e_sint:
-  case Scalar::e_uint:
-    llvm_unreachable("Only floating point types supported!");
-  case Scalar::e_float:
-    return llvm::APFloat::IEEEsingle();
-  case Scalar::e_double:
-    return llvm::APFloat::IEEEdouble();
-  case Scalar::e_long_double:
-    return llvm::APFloat::x87DoubleExtended();
-  }
-  llvm_unreachable("Unhandled type!");
-}
-
 bool Scalar::IntegralPromote(uint16_t bits, bool sign) {
   switch (GetCategory(m_type)) {
   case Category::Void:
   case Category::Float:
     break;
   case Category::Integral:
-    if (GetIntKey() > IntPromotionKey(bits, !sign))
+    if (GetPromoKey() > PromotionKey(Category::Integral, bits, !sign))
       break;
     if (IsSigned(m_type))
       m_integer = m_integer.sextOrTrunc(bits);
@@ -237,29 +224,27 @@ bool Scalar::IntegralPromote(uint16_t bits, bool sign) {
   return false;
 }
 
-bool Scalar::FloatPromote(Scalar::Type type) {
-  assert(GetCategory(type) == Category::Float);
+bool Scalar::FloatPromote(const llvm::fltSemantics &semantics) {
   bool success = false;
   switch (GetCategory(m_type)) {
   case Category::Void:
     break;
   case Category::Integral:
-    m_float = llvm::APFloat(GetFltSemantics(type));
+    m_float = llvm::APFloat(semantics);
     m_float.convertFromAPInt(m_integer, IsSigned(m_type),
                              llvm::APFloat::rmNearestTiesToEven);
     success = true;
     break;
   case Category::Float:
-    if (type < m_type)
+    if (GetFloatPromoKey(semantics) < GetFloatPromoKey(m_float.getSemantics()))
       break;
     bool ignore;
     success = true;
-    m_float.convert(GetFltSemantics(type), llvm::APFloat::rmNearestTiesToEven,
-                    &ignore);
+    m_float.convert(semantics, llvm::APFloat::rmNearestTiesToEven, &ignore);
   }
 
   if (success)
-    m_type = type;
+    m_type = e_float;
   return success;
 }
 
@@ -273,10 +258,6 @@ const char *Scalar::GetValueTypeAsCString(Scalar::Type type) {
     return "unsigned int";
   case e_float:
     return "float";
-  case e_double:
-    return "double";
-  case e_long_double:
-    return "long double";
   }
   return "???";
 }
@@ -291,16 +272,6 @@ Scalar::GetValueTypeForUnsignedIntegerWithByteSize(size_t byte_size) {
   return e_uint;
 }
 
-Scalar::Type Scalar::GetValueTypeForFloatWithByteSize(size_t byte_size) {
-  if (byte_size == sizeof(float_t))
-    return e_float;
-  if (byte_size == sizeof(double_t))
-    return e_double;
-  if (byte_size == sizeof(long_double_t))
-    return e_long_double;
-  return e_void;
-}
-
 bool Scalar::MakeSigned() {
   bool success = false;
 
@@ -317,12 +288,6 @@ bool Scalar::MakeSigned() {
   case e_float:
     success = true;
     break;
-  case e_double:
-    success = true;
-    break;
-  case e_long_double:
-    success = true;
-    break;
   }
 
   return success;
@@ -344,12 +309,6 @@ bool Scalar::MakeUnsigned() {
   case e_float:
     success = true;
     break;
-  case e_double:
-    success = true;
-    break;
-  case e_long_double:
-    success = true;
-    break;
   }
 
   return success;
@@ -524,8 +483,6 @@ Scalar &Scalar::operator>>=(const Scalar &rhs) {
   switch (m_type) {
   case e_void:
   case e_float:
-  case e_double:
-  case e_long_double:
     m_type = e_void;
     break;
 
@@ -534,8 +491,6 @@ Scalar &Scalar::operator>>=(const Scalar &rhs) {
     switch (rhs.m_type) {
     case e_void:
     case e_float:
-    case e_double:
-    case e_long_double:
       m_type = e_void;
       break;
     case e_sint:
@@ -570,8 +525,6 @@ bool Scalar::AbsoluteValue() {
   case e_uint:
     return true;
   case e_float:
-  case e_double:
-  case e_long_double:
     m_float.clearSign();
     return true;
   }
@@ -610,13 +563,13 @@ const Scalar lldb_private::operator+(const Scalar &lhs, const Scalar &rhs) {
 const Scalar lldb_private::operator-(Scalar lhs, Scalar rhs) {
   Scalar result;
   if ((result.m_type = Scalar::PromoteToMaxType(lhs, rhs)) != Scalar::e_void) {
-    switch (GetCategory(result.m_type)) {
-    case Category::Void:
+    switch (Scalar::GetCategory(result.m_type)) {
+    case Scalar::Category::Void:
       break;
-    case Category::Integral:
+    case Scalar::Category::Integral:
       result.m_integer = lhs.m_integer - rhs.m_integer;
       break;
-    case Category::Float:
+    case Scalar::Category::Float:
       result.m_float = lhs.m_float - rhs.m_float;
       break;
     }
@@ -628,16 +581,16 @@ const Scalar lldb_private::operator/(Scalar lhs, Scalar rhs) {
   Scalar result;
   if ((result.m_type = Scalar::PromoteToMaxType(lhs, rhs)) != Scalar::e_void &&
       !rhs.IsZero()) {
-    switch (GetCategory(result.m_type)) {
-    case Category::Void:
+    switch (Scalar::GetCategory(result.m_type)) {
+    case Scalar::Category::Void:
       break;
-    case Category::Integral:
+    case Scalar::Category::Integral:
       if (IsSigned(result.m_type))
         result.m_integer = lhs.m_integer.sdiv(rhs.m_integer);
       else
         result.m_integer = lhs.m_integer.udiv(rhs.m_integer);
       return result;
-    case Category::Float:
+    case Scalar::Category::Float:
       result.m_float = lhs.m_float / rhs.m_float;
       return result;
     }
@@ -651,13 +604,13 @@ const Scalar lldb_private::operator/(Scalar lhs, Scalar rhs) {
 const Scalar lldb_private::operator*(Scalar lhs, Scalar rhs) {
   Scalar result;
   if ((result.m_type = Scalar::PromoteToMaxType(lhs, rhs)) != Scalar::e_void) {
-    switch (GetCategory(result.m_type)) {
-    case Category::Void:
+    switch (Scalar::GetCategory(result.m_type)) {
+    case Scalar::Category::Void:
       break;
-    case Category::Integral:
+    case Scalar::Category::Integral:
       result.m_integer = lhs.m_integer * rhs.m_integer;
       break;
-    case Category::Float:
+    case Scalar::Category::Float:
       result.m_float = lhs.m_float * rhs.m_float;
       break;
     }
@@ -668,7 +621,7 @@ const Scalar lldb_private::operator*(Scalar lhs, Scalar rhs) {
 const Scalar lldb_private::operator&(Scalar lhs, Scalar rhs) {
   Scalar result;
   if ((result.m_type = Scalar::PromoteToMaxType(lhs, rhs)) != Scalar::e_void) {
-    if (GetCategory(result.m_type) == Category::Integral)
+    if (Scalar::GetCategory(result.m_type) == Scalar::Category::Integral)
       result.m_integer = lhs.m_integer & rhs.m_integer;
     else
       result.m_type = Scalar::e_void;
@@ -679,7 +632,7 @@ const Scalar lldb_private::operator&(Scalar lhs, Scalar rhs) {
 const Scalar lldb_private::operator|(Scalar lhs, Scalar rhs) {
   Scalar result;
   if ((result.m_type = Scalar::PromoteToMaxType(lhs, rhs)) != Scalar::e_void) {
-    if (GetCategory(result.m_type) == Category::Integral)
+    if (Scalar::GetCategory(result.m_type) == Scalar::Category::Integral)
       result.m_integer = lhs.m_integer | rhs.m_integer;
     else
       result.m_type = Scalar::e_void;
@@ -690,7 +643,8 @@ const Scalar lldb_private::operator|(Scalar lhs, Scalar rhs) {
 const Scalar lldb_private::operator%(Scalar lhs, Scalar rhs) {
   Scalar result;
   if ((result.m_type = Scalar::PromoteToMaxType(lhs, rhs)) != Scalar::e_void) {
-    if (!rhs.IsZero() && GetCategory(result.m_type) == Category::Integral) {
+    if (!rhs.IsZero() &&
+        Scalar::GetCategory(result.m_type) == Scalar::Category::Integral) {
       if (IsSigned(result.m_type))
         result.m_integer = lhs.m_integer.srem(rhs.m_integer);
       else
@@ -705,7 +659,7 @@ const Scalar lldb_private::operator%(Scalar lhs, Scalar rhs) {
 const Scalar lldb_private::operator^(Scalar lhs, Scalar rhs) {
   Scalar result;
   if ((result.m_type = Scalar::PromoteToMaxType(lhs, rhs)) != Scalar::e_void) {
-    if (GetCategory(result.m_type) == Category::Integral)
+    if (Scalar::GetCategory(result.m_type) == Scalar::Category::Integral)
       result.m_integer = lhs.m_integer ^ rhs.m_integer;
     else
       result.m_type = Scalar::e_void;
@@ -773,16 +727,17 @@ Status Scalar::SetValueFromCString(const char *value_str, Encoding encoding,
   }
 
   case eEncodingIEEE754: {
-    Type type = GetValueTypeForFloatWithByteSize(byte_size);
-    if (type == e_void) {
-      error.SetErrorStringWithFormatv("unsupported float byte size: {0}",
-                                      byte_size);
-      break;
-    }
-    APFloat f(GetFltSemantics(type));
+    // FIXME: It's not possible to unambiguously map a byte size to a floating
+    // point type. This function should be refactored to take an explicit
+    // semantics argument.
+    const llvm::fltSemantics &sem =
+        byte_size <= 4 ? APFloat::IEEEsingle()
+                       : byte_size <= 8 ? APFloat::IEEEdouble()
+                                        : APFloat::x87DoubleExtended();
+    APFloat f(sem);
     if (llvm::Expected<APFloat::opStatus> op =
             f.convertFromString(value_str, APFloat::rmNearestTiesToEven)) {
-      m_type = type;
+      m_type = e_float;
       m_float = std::move(f);
     } else
       error = op.takeError();
@@ -854,8 +809,6 @@ bool Scalar::SignExtend(uint32_t sign_bit_pos) {
     switch (m_type) {
     case Scalar::e_void:
     case Scalar::e_float:
-    case Scalar::e_double:
-    case Scalar::e_long_double:
       return false;
 
     case Scalar::e_sint:
@@ -910,8 +863,6 @@ bool Scalar::ExtractBitfield(uint32_t bit_size, uint32_t bit_offset) {
   switch (m_type) {
   case Scalar::e_void:
   case Scalar::e_float:
-  case Scalar::e_double:
-  case Scalar::e_long_double:
     break;
 
   case Scalar::e_sint:
@@ -942,8 +893,6 @@ bool lldb_private::operator==(Scalar lhs, Scalar rhs) {
   case Scalar::e_uint:
     return lhs.m_integer == rhs.m_integer;
   case Scalar::e_float:
-  case Scalar::e_double:
-  case Scalar::e_long_double:
     result = lhs.m_float.compare(rhs.m_float);
     if (result == llvm::APFloat::cmpEqual)
       return true;
@@ -968,8 +917,6 @@ bool lldb_private::operator<(Scalar lhs, Scalar rhs) {
   case Scalar::e_uint:
     return lhs.m_integer.ult(rhs.m_integer);
   case Scalar::e_float:
-  case Scalar::e_double:
-  case Scalar::e_long_double:
     result = lhs.m_float.compare(rhs.m_float);
     if (result == llvm::APFloat::cmpLessThan)
       return true;
@@ -998,8 +945,6 @@ bool Scalar::ClearBit(uint32_t bit) {
     m_integer.clearBit(bit);
     return true;
   case e_float:
-  case e_double:
-  case e_long_double:
     break;
   }
   return false;
@@ -1014,8 +959,6 @@ bool Scalar::SetBit(uint32_t bit) {
     m_integer.setBit(bit);
     return true;
   case e_float:
-  case e_double:
-  case e_long_double:
     break;
   }
   return false;

diff  --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp
index 544a32baa640..48cadfce466b 100644
--- a/lldb/unittests/Utility/ScalarTest.cpp
+++ b/lldb/unittests/Utility/ScalarTest.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Testing/Support/Error.h"
 
 using namespace lldb_private;
+using llvm::APFloat;
 using llvm::APInt;
 using llvm::Failed;
 using llvm::Succeeded;
@@ -304,12 +305,12 @@ TEST(ScalarTest, Promotion) {
 
   EXPECT_FALSE(a.IntegralPromote(64, true));
 
-  EXPECT_TRUE(a.FloatPromote(Scalar::e_double));
-  EXPECT_EQ(Scalar::e_double, a.GetType());
+  EXPECT_TRUE(a.FloatPromote(APFloat::IEEEdouble()));
+  EXPECT_EQ(Scalar::e_float, a.GetType());
   EXPECT_EQ(47.0, a.Double());
 
-  EXPECT_FALSE(a.FloatPromote(Scalar::e_float));
-  EXPECT_TRUE(a.FloatPromote(Scalar::e_long_double));
+  EXPECT_FALSE(a.FloatPromote(APFloat::IEEEsingle()));
+  EXPECT_TRUE(a.FloatPromote(APFloat::x87DoubleExtended()));
   EXPECT_EQ(47.0L, a.LongDouble());
 }
 


        


More information about the lldb-commits mailing list