[Lldb-commits] [lldb] 52495b9 - [lldb/Utility] Fix float->integral conversions in Scalar APInt getters

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-07-07T16:59:06+02:00
New Revision: 52495b98eecefcbaea2e30edec6816e43653d175

URL: https://github.com/llvm/llvm-project/commit/52495b98eecefcbaea2e30edec6816e43653d175
DIFF: https://github.com/llvm/llvm-project/commit/52495b98eecefcbaea2e30edec6816e43653d175.diff

LOG: [lldb/Utility] Fix float->integral conversions in Scalar APInt getters

These functions were doing a bitcast on the float value, which is not
consistent with the other getters, which were doing a numeric conversion
(47.0 -> 47). Change these to do numeric conversions too.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 7397744fb51c..6e2715130c70 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -557,6 +557,14 @@ bool Scalar::MakeUnsigned() {
   return success;
 }
 
+static llvm::APInt ToAPInt(const llvm::APFloat &f, unsigned bits,
+                           bool is_unsigned) {
+  llvm::APSInt result(bits, is_unsigned);
+  bool isExact;
+  f.convertToInteger(result, llvm::APFloat::rmTowardZero, &isExact);
+  return std::move(result);
+}
+
 template <typename T> T Scalar::GetAs(T fail_value) const {
   switch (GetCategory(m_type)) {
   case Category::Void:
@@ -565,12 +573,9 @@ template <typename T> T Scalar::GetAs(T fail_value) const {
     if (IsSigned(m_type))
       return m_integer.sextOrTrunc(sizeof(T) * 8).getSExtValue();
     return m_integer.zextOrTrunc(sizeof(T) * 8).getZExtValue();
-  case Category::Float: {
-    llvm::APSInt result(sizeof(T) * 8, std::is_unsigned<T>::value);
-    bool isExact;
-    m_float.convertToInteger(result, llvm::APFloat::rmTowardZero, &isExact);
-    return result.getSExtValue();
-  }
+  case Category::Float:
+    return ToAPInt(m_float, sizeof(T) * 8, std::is_unsigned<T>::value)
+        .getSExtValue();
   }
   return fail_value;
 }
@@ -612,51 +617,25 @@ unsigned long long Scalar::ULongLong(unsigned long long fail_value) const {
 }
 
 llvm::APInt Scalar::SInt128(const llvm::APInt &fail_value) const {
-  switch (m_type) {
-  case e_void:
+  switch (GetCategory(m_type)) {
+  case Category::Void:
     break;
-  case e_sint:
-  case e_uint:
-  case e_slong:
-  case e_ulong:
-  case e_slonglong:
-  case e_ulonglong:
-  case e_sint128:
-  case e_uint128:
-  case e_sint256:
-  case e_uint256:
-  case e_sint512:
-  case e_uint512:
+  case Category::Integral:
     return m_integer;
-  case e_float:
-  case e_double:
-  case e_long_double:
-    return m_float.bitcastToAPInt();
+  case Category::Float:
+    return ToAPInt(m_float, 128, /*is_unsigned=*/false);
   }
   return fail_value;
 }
 
 llvm::APInt Scalar::UInt128(const llvm::APInt &fail_value) const {
-  switch (m_type) {
-  case e_void:
+  switch (GetCategory(m_type)) {
+  case Category::Void:
     break;
-  case e_sint:
-  case e_uint:
-  case e_slong:
-  case e_ulong:
-  case e_slonglong:
-  case e_ulonglong:
-  case e_sint128:
-  case e_uint128:
-  case e_sint256:
-  case e_uint256:
-  case e_sint512:
-  case e_uint512:
+  case Category::Integral:
     return m_integer;
-  case e_float:
-  case e_double:
-  case e_long_double:
-    return m_float.bitcastToAPInt();
+  case Category::Float:
+    return ToAPInt(m_float, 128, /*is_unsigned=*/true);
   }
   return fail_value;
 }

diff  --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp
index 910ff173bc92..42a2f2aaebf2 100644
--- a/lldb/unittests/Utility/ScalarTest.cpp
+++ b/lldb/unittests/Utility/ScalarTest.cpp
@@ -66,16 +66,6 @@ TEST(ScalarTest, ComparisonFloat) {
   ASSERT_TRUE(s2 >= s1);
 }
 
-template <typename T1, typename T2>
-static T2 ConvertHost(T1 val, T2 (Scalar::*)(T2) const) {
-  return T2(val);
-}
-
-template <typename T1, typename T2>
-static T2 ConvertScalar(T1 val, T2 (Scalar::*conv)(T2) const) {
-  return (Scalar(val).*conv)(T2());
-}
-
 template <typename T> static void CheckConversion(T val) {
   SCOPED_TRACE("val = " + std::to_string(val));
   EXPECT_EQ((signed char)val, Scalar(val).SChar());
@@ -102,6 +92,19 @@ TEST(ScalarTest, Getters) {
   CheckConversion<unsigned long long>(0x8765432112345678ull);
   CheckConversion<float>(42.25f);
   CheckConversion<double>(42.25);
+
+  EXPECT_EQ(APInt(128, 1) << 70, Scalar(std::pow(2.0f, 70.0f)).SInt128(APInt()));
+  EXPECT_EQ(APInt(128, -1, true) << 70,
+            Scalar(-std::pow(2.0f, 70.0f)).SInt128(APInt()));
+  EXPECT_EQ(APInt(128, 1) << 70,
+            Scalar(std::pow(2.0f, 70.0f)).UInt128(APInt()));
+  EXPECT_EQ(APInt(128, 0), Scalar(-std::pow(2.0f, 70.0f)).UInt128(APInt()));
+
+  EXPECT_EQ(APInt(128, 1) << 70, Scalar(std::pow(2.0, 70.0)).SInt128(APInt()));
+  EXPECT_EQ(APInt(128, -1, true) << 70,
+            Scalar(-std::pow(2.0, 70.0)).SInt128(APInt()));
+  EXPECT_EQ(APInt(128, 1) << 70, Scalar(std::pow(2.0, 70.0)).UInt128(APInt()));
+  EXPECT_EQ(APInt(128, 0), Scalar(-std::pow(2.0, 70.0)).UInt128(APInt()));
 }
 
 TEST(ScalarTest, RightShiftOperator) {