labath created this revision.
labath added reviewers: JDevlieghere, teemperor.
Herald added a project: LLDB.
labath requested review of this revision.

Similarly to D85836 <https://reviews.llvm.org/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).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86220

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

Index: lldb/unittests/Utility/ScalarTest.cpp
===================================================================
--- lldb/unittests/Utility/ScalarTest.cpp
+++ 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 @@
 
   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());
 }
 
Index: lldb/source/Utility/Scalar.cpp
===================================================================
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -26,17 +26,11 @@
 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 @@
     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 *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 @@
   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 @@
   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 @@
   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 @@
     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 @@
   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 @@
   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 @@
   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 @@
   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 @@
     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 @@
   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-(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 @@
   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) {
   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) {
   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) {
   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) {
   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) {
   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 @@
   }
 
   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 @@
     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 @@
   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 @@
   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 @@
   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 @@
     m_integer.clearBit(bit);
     return true;
   case e_float:
-  case e_double:
-  case e_long_double:
     break;
   }
   return false;
@@ -1014,8 +959,6 @@
     m_integer.setBit(bit);
     return true;
   case e_float:
-  case e_double:
-  case e_long_double:
     break;
   }
   return false;
Index: lldb/include/lldb/Utility/Scalar.h
===================================================================
--- lldb/include/lldb/Utility/Scalar.h
+++ lldb/include/lldb/Utility/Scalar.h
@@ -45,8 +45,6 @@
     e_sint,
     e_uint,
     e_float,
-    e_double,
-    e_long_double
   };
 
   // Constructors and Destructors
@@ -70,8 +68,8 @@
       : 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 @@
 
   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 @@
   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 @@
 
   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);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to