Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[lldb] Add support for displaying __float128 variables #98369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
Loading
from

Conversation

beetrees
Copy link
Contributor

@beetrees beetrees commented Jul 10, 2024

Adds support to LLDB for displaying 128-bit IEEE 754 floats (__float128 in Clang, _Float128 in GCC, f128 in Rust). As DWARF does not currently provide a systemic way to distinguish between a 128-bit IEEE 754 float and a 128-bit x87/PPC long double (see #98179), the type name is used to disambiguate instead.

(Note to reviewer: I don't have commit access)

@beetrees beetrees requested a review from JDevlieghere as a code owner July 10, 2024 19:00
@llvmbot llvmbot added the lldb label Jul 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 10, 2024

@llvm/pr-subscribers-lldb

Author: None (beetrees)

Changes

Adds support to LLDB for displaying 128-bit IEEE 754 floats (__float128 in Clang, _Float128 in GCC, f128 in Rust). As DWARF does not currently provide a systemic way to distinguish between a 128-bit IEEE 754 float and a 128-bit x87/PPC long double (see #98179), the type name is used to disambiguate instead.


Full diff: https://github.com/llvm/llvm-project/pull/98369.diff

11 Files Affected:

  • (modified) lldb/bindings/python/python-extensions.swig (+1)
  • (modified) lldb/docs/python_api_enums.rst (+2)
  • (modified) lldb/include/lldb/lldb-enumerations.h (+9-4)
  • (modified) lldb/source/Commands/CommandObjectMemory.cpp (+2)
  • (modified) lldb/source/Core/DumpDataExtractor.cpp (+6-1)
  • (modified) lldb/source/Core/ValueObject.cpp (+3-2)
  • (modified) lldb/source/DataFormatters/FormatManager.cpp (+1)
  • (modified) lldb/source/DataFormatters/VectorType.cpp (+2)
  • (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+21)
  • (modified) lldb/unittests/Core/DumpDataExtractorTest.cpp (+6)
  • (modified) lldb/unittests/Symbol/TestTypeSystemClang.cpp (+2)
diff --git a/lldb/bindings/python/python-extensions.swig b/lldb/bindings/python/python-extensions.swig
index 4ba1607c70909..40fa76872ee96 100644
--- a/lldb/bindings/python/python-extensions.swig
+++ b/lldb/bindings/python/python-extensions.swig
@@ -594,6 +594,7 @@ def is_numeric_type(basic_type):
     if basic_type == eBasicTypeFloat: return (True,True)
     if basic_type == eBasicTypeDouble: return (True,True)
     if basic_type == eBasicTypeLongDouble: return (True,True)
+    if basic_type == eBasicTypeFloat128: return (True,True)
     if basic_type == eBasicTypeFloatComplex: return (True,True)
     if basic_type == eBasicTypeDoubleComplex: return (True,True)
     if basic_type == eBasicTypeLongDoubleComplex: return (True,True)
diff --git a/lldb/docs/python_api_enums.rst b/lldb/docs/python_api_enums.rst
index b6a2497ea878e..92ddfaf7b110a 100644
--- a/lldb/docs/python_api_enums.rst
+++ b/lldb/docs/python_api_enums.rst
@@ -295,6 +295,7 @@ Format
 .. py:data:: eFormatHex
 .. py:data:: eFormatHexUppercase
 .. py:data:: eFormatFloat
+.. py:data:: eFormatFloat128
 .. py:data:: eFormatOctal
 .. py:data:: eFormatOSType
 .. py:data:: eFormatUnicode16
@@ -1037,6 +1038,7 @@ BasicType
 .. py:data:: eBasicTypeFloat
 .. py:data:: eBasicTypeDouble
 .. py:data:: eBasicTypeLongDouble
+.. py:data:: eBasicTypeFloat128
 .. py:data:: eBasicTypeFloatComplex
 .. py:data:: eBasicTypeDoubleComplex
 .. py:data:: eBasicTypeLongDoubleComplex
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 74ff458bf87de..d90a64bb652e6 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -170,6 +170,10 @@ enum Format {
   eFormatHex,
   eFormatHexUppercase,
   eFormatFloat,
+  eFormatFloat128, //< Disambiguate between 128-bit `long double` (which uses
+                   //< `eFormatFloat`) and `__float128` (which uses
+                   //< `eFormatFloat128`). If the value being formatted is not
+                   //< 128 bits, then this is identical to `eFormatFloat`.
   eFormatOctal,
   eFormatOSType, ///< OS character codes encoded into an integer 'PICT' 'text'
                  ///< etc...
@@ -195,10 +199,10 @@ enum Format {
                          ///< character arrays that can contain non printable
                          ///< characters
   eFormatAddressInfo,    ///< Describe what an address points to (func + offset
-                      ///< with file/line, symbol + offset, data, etc)
-  eFormatHexFloat,    ///< ISO C99 hex float string
-  eFormatInstruction, ///< Disassemble an opcode
-  eFormatVoid,        ///< Do not print this
+                         ///< with file/line, symbol + offset, data, etc)
+  eFormatHexFloat,       ///< ISO C99 hex float string
+  eFormatInstruction,    ///< Disassemble an opcode
+  eFormatVoid,           ///< Do not print this
   eFormatUnicode8,
   kNumFormats
 };
@@ -819,6 +823,7 @@ enum BasicType {
   eBasicTypeFloat,
   eBasicTypeDouble,
   eBasicTypeLongDouble,
+  eBasicTypeFloat128,
   eBasicTypeFloatComplex,
   eBasicTypeDoubleComplex,
   eBasicTypeLongDoubleComplex,
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index 137b1ad981073..dde27d2df0fe5 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -156,6 +156,7 @@ class OptionGroupReadMemory : public OptionGroup {
 
     case eFormatBinary:
     case eFormatFloat:
+    case eFormatFloat128:
     case eFormatOctal:
     case eFormatDecimal:
     case eFormatEnum:
@@ -1329,6 +1330,7 @@ class CommandObjectMemoryWrite : public CommandObjectParsed {
       switch (m_format_options.GetFormat()) {
       case kNumFormats:
       case eFormatFloat: // TODO: add support for floats soon
+      case eFormatFloat128:
       case eFormatCharPrintable:
       case eFormatBytesWithASCII:
       case eFormatComplex:
diff --git a/lldb/source/Core/DumpDataExtractor.cpp b/lldb/source/Core/DumpDataExtractor.cpp
index 826edd7bab046..faaeb0dacc6a8 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -334,6 +334,8 @@ static const llvm::fltSemantics &GetFloatSemantics(const TargetSP &target_sp,
       return llvm::APFloat::IEEEsingle();
     case 8:
       return llvm::APFloat::IEEEdouble();
+    case 16:
+      return llvm::APFloat::IEEEquad();
   }
   return llvm::APFloat::Bogus();
 }
@@ -652,6 +654,7 @@ lldb::offset_t lldb_private::DumpDataExtractor(
       }
     } break;
 
+    case eFormatFloat128:
     case eFormatFloat: {
       TargetSP target_sp;
       if (exe_scope)
@@ -665,7 +668,9 @@ lldb::offset_t lldb_private::DumpDataExtractor(
       const unsigned format_precision = 0;
 
       const llvm::fltSemantics &semantics =
-          GetFloatSemantics(target_sp, item_byte_size);
+          item_format == eFormatFloat128 && item_byte_size == 16
+              ? llvm::APFloat::IEEEquad()
+              : GetFloatSemantics(target_sp, item_byte_size);
 
       // Recalculate the byte size in case of a difference. This is possible
       // when item_byte_size is 16 (128-bit), because you could get back the
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4..0d925ab5f144a 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1438,8 +1438,9 @@ bool ValueObject::DumpPrintableRepresentation(
           (custom_format == eFormatComplexFloat) ||
           (custom_format == eFormatDecimal) || (custom_format == eFormatHex) ||
           (custom_format == eFormatHexUppercase) ||
-          (custom_format == eFormatFloat) || (custom_format == eFormatOctal) ||
-          (custom_format == eFormatOSType) ||
+          (custom_format == eFormatFloat) ||
+          (custom_format == eFormatFloat128) ||
+          (custom_format == eFormatOctal) || (custom_format == eFormatOSType) ||
           (custom_format == eFormatUnicode16) ||
           (custom_format == eFormatUnicode32) ||
           (custom_format == eFormatUnsigned) ||
diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp
index 7e19989a8264a..4f50c0df5e7b6 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -46,6 +46,7 @@ static constexpr FormatInfo g_format_infos[] = {
     {eFormatHex, 'x', "hex"},
     {eFormatHexUppercase, 'X', "uppercase hex"},
     {eFormatFloat, 'f', "float"},
+    {eFormatFloat128, '\0', "float128"},
     {eFormatOctal, 'o', "octal"},
     {eFormatOSType, 'O', "OSType"},
     {eFormatUnicode16, 'U', "unicode16"},
diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp
index 19de204c24353..36af7a1286f77 100644
--- a/lldb/source/DataFormatters/VectorType.cpp
+++ b/lldb/source/DataFormatters/VectorType.cpp
@@ -55,6 +55,8 @@ static CompilerType GetCompilerTypeForFormat(lldb::Format format,
 
   case lldb::eFormatFloat:
     return type_system->GetBasicTypeFromAST(lldb::eBasicTypeFloat);
+  case lldb::eFormatFloat128:
+    return type_system->GetBasicTypeFromAST(lldb::eBasicTypeFloat128);
 
   case lldb::eFormatHex:
   case lldb::eFormatHexUppercase:
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index f70efe5ed57e4..fcb0cb9ae62a0 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -790,6 +790,8 @@ TypeSystemClang::GetBuiltinTypeForEncodingAndBitSize(Encoding encoding,
       return GetType(ast.LongDoubleTy);
     if (QualTypeMatchesBitSize(bit_size, ast, ast.HalfTy))
       return GetType(ast.HalfTy);
+    if (QualTypeMatchesBitSize(bit_size, ast, ast.Float128Ty))
+      return GetType(ast.Float128Ty);
     break;
 
   case eEncodingVector:
@@ -948,6 +950,13 @@ CompilerType TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
     if (type_name == "long double" &&
         QualTypeMatchesBitSize(bit_size, ast, ast.LongDoubleTy))
       return GetType(ast.LongDoubleTy);
+    // As Rust currently uses `TypeSystemClang`, match `f128` here as well so it
+    // doesn't get misinterpreted as `long double` on targets where they are
+    // the same size but different formats.
+    if ((type_name == "__float128" || type_name == "_Float128" ||
+         type_name == "f128") &&
+        QualTypeMatchesBitSize(bit_size, ast, ast.Float128Ty))
+      return GetType(ast.Float128Ty);
     // Fall back to not requiring a name match
     if (QualTypeMatchesBitSize(bit_size, ast, ast.FloatTy))
       return GetType(ast.FloatTy);
@@ -957,6 +966,8 @@ CompilerType TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
       return GetType(ast.LongDoubleTy);
     if (QualTypeMatchesBitSize(bit_size, ast, ast.HalfTy))
       return GetType(ast.HalfTy);
+    if (QualTypeMatchesBitSize(bit_size, ast, ast.Float128Ty))
+      return GetType(ast.Float128Ty);
     break;
 
   case DW_ATE_signed:
@@ -2050,6 +2061,8 @@ TypeSystemClang::GetOpaqueCompilerType(clang::ASTContext *ast,
     return ast->DoubleTy.getAsOpaquePtr();
   case eBasicTypeLongDouble:
     return ast->LongDoubleTy.getAsOpaquePtr();
+  case eBasicTypeFloat128:
+    return ast->Float128Ty.getAsOpaquePtr();
   case eBasicTypeFloatComplex:
     return ast->getComplexType(ast->FloatTy).getAsOpaquePtr();
   case eBasicTypeDoubleComplex:
@@ -4722,6 +4735,8 @@ TypeSystemClang::GetFloatTypeSemantics(size_t byte_size) {
     return ast.getFloatTypeSemantics(ast.LongDoubleTy);
   else if (bit_size == ast.getTypeSize(ast.HalfTy))
     return ast.getFloatTypeSemantics(ast.HalfTy);
+  else if (bit_size == ast.getTypeSize(ast.Float128Ty))
+    return ast.getFloatTypeSemantics(ast.Float128Ty);
   return llvm::APFloatBase::Bogus();
 }
 
@@ -5232,6 +5247,8 @@ lldb::Format TypeSystemClang::GetFormat(lldb::opaque_compiler_type_t type) {
     case clang::BuiltinType::Double:
     case clang::BuiltinType::LongDouble:
       return lldb::eFormatFloat;
+    case clang::BuiltinType::Float128:
+      return lldb::eFormatFloat128;
     default:
       return lldb::eFormatHex;
     }
@@ -5551,6 +5568,8 @@ TypeSystemClang::GetBasicTypeEnumeration(lldb::opaque_compiler_type_t type) {
         return eBasicTypeDouble;
       case clang::BuiltinType::LongDouble:
         return eBasicTypeLongDouble;
+      case clang::BuiltinType::Float128:
+        return eBasicTypeFloat128;
 
       case clang::BuiltinType::NullPtr:
         return eBasicTypeNullPtr;
@@ -6111,6 +6130,7 @@ uint32_t TypeSystemClang::GetNumPointeeChildren(clang::QualType type) {
     case clang::BuiltinType::Float:
     case clang::BuiltinType::Double:
     case clang::BuiltinType::LongDouble:
+    case clang::BuiltinType::Float128:
     case clang::BuiltinType::Dependent:
     case clang::BuiltinType::Overload:
     case clang::BuiltinType::ObjCId:
@@ -8807,6 +8827,7 @@ bool TypeSystemClang::DumpTypeValue(
         case eFormatHex:
         case eFormatHexUppercase:
         case eFormatFloat:
+        case eFormatFloat128:
         case eFormatOctal:
         case eFormatOSType:
         case eFormatUnsigned:
diff --git a/lldb/unittests/Core/DumpDataExtractorTest.cpp b/lldb/unittests/Core/DumpDataExtractorTest.cpp
index 3d1e8bc5e4623..6302f1e1d31a6 100644
--- a/lldb/unittests/Core/DumpDataExtractorTest.cpp
+++ b/lldb/unittests/Core/DumpDataExtractorTest.cpp
@@ -163,6 +163,9 @@ TEST_F(DumpDataExtractorTest, Formats) {
   TestDump(0xcafef00d, lldb::Format::eFormatHex, "0xcafef00d");
   TestDump(0xcafef00d, lldb::Format::eFormatHexUppercase, "0xCAFEF00D");
   TestDump(0.456, lldb::Format::eFormatFloat, "0.45600000000000002");
+  TestDump(std::vector<uint64_t>{0x47ae147ae147ae14, 0x40011147ae147ae1},
+           lldb::Format::eFormatFloat128,
+           "4.26999999999999999999999999999999963");
   TestDump(9, lldb::Format::eFormatOctal, "011");
   // Chars packed into an integer.
   TestDump<uint32_t>(0x4C4C4442, lldb::Format::eFormatOSType, "'LLDB'");
@@ -388,6 +391,9 @@ TEST_F(DumpDataExtractorTest, ItemByteSizeErrors) {
   TestDumpWithItemByteSize(
       18, lldb::Format::eFormatFloat,
       "error: unsupported byte size (18) for float format");
+  TestDumpWithItemByteSize(
+      17, lldb::Format::eFormatFloat128,
+      "error: unsupported byte size (17) for float format");
 
   // We want sizes to exactly match one of float/double.
   TestDumpWithItemByteSize(
diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
index 30d20b9587f91..e08131deea953 100644
--- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -75,6 +75,8 @@ TEST_F(TestTypeSystemClang, TestGetBasicTypeFromEnum) {
                                   context.getComplexType(context.FloatTy)));
   EXPECT_TRUE(
       context.hasSameType(GetBasicQualType(eBasicTypeHalf), context.HalfTy));
+  EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeFloat128),
+                                  context.Float128Ty));
   EXPECT_TRUE(
       context.hasSameType(GetBasicQualType(eBasicTypeInt), context.IntTy));
   EXPECT_TRUE(context.hasSameType(GetBasicQualType(eBasicTypeInt128),

///< with file/line, symbol + offset, data, etc)
eFormatHexFloat, ///< ISO C99 hex float string
eFormatInstruction, ///< Disassemble an opcode
eFormatVoid, ///< Do not print this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the formatting changes to this part?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately git clang-format made those changes, and CI will complain if I remove them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the codebase isn't 100% formatted, it's fine to ignore the formatter sometimes. Generally I'd push a reformat NFC commit directly before or after the PR changes, but here I think it'd need to be after. This means folks can ignore the NFC change without skipping over functional changes like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved the formatting-only changes to a separate commit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI that llvm does squash and merge only, so the commits will become one. So you can either push each part of this PR manually or push the formatting change separately after the PR goes in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to remove the formatting change from this PR, but that will cause the PR CI to fail. Will that be an issue? (I don't have commit access so I can't push anything directly.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the format check doesn't block merging.

Copy link
Contributor Author

@beetrees beetrees Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've dropped the formatting change from this PR.

Copy link

github-actions bot commented Jul 10, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- lldb/include/lldb/Symbol/TypeSystem.h lldb/include/lldb/lldb-enumerations.h lldb/source/Commands/CommandObjectMemory.cpp lldb/source/Core/DumpDataExtractor.cpp lldb/source/DataFormatters/FormatManager.cpp lldb/source/DataFormatters/VectorType.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/source/ValueObject/ValueObject.cpp lldb/unittests/Core/DumpDataExtractorTest.cpp lldb/unittests/Symbol/TestTypeSystemClang.cpp
View the diff from clang-format here.
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 311a18b85..6a9a09fdb 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -198,10 +198,10 @@ enum Format {
                          ///< character arrays that can contain non printable
                          ///< characters
   eFormatAddressInfo,    ///< Describe what an address points to (func + offset
-                      ///< with file/line, symbol + offset, data, etc)
-  eFormatHexFloat,    ///< ISO C99 hex float string
-  eFormatInstruction, ///< Disassemble an opcode
-  eFormatVoid,        ///< Do not print this
+                         ///< with file/line, symbol + offset, data, etc)
+  eFormatHexFloat,       ///< ISO C99 hex float string
+  eFormatInstruction,    ///< Disassemble an opcode
+  eFormatVoid,           ///< Do not print this
   eFormatUnicode8,
   eFormatFloat128, ///< Disambiguate between 128-bit `long double` (which uses
                    ///< `eFormatFloat`) and `__float128` (which uses

lldb/source/Core/DumpDataExtractor.cpp Outdated Show resolved Hide resolved
lldb/include/lldb/lldb-enumerations.h Outdated Show resolved Hide resolved
lldb/include/lldb/lldb-enumerations.h Outdated Show resolved Hide resolved
@@ -46,6 +46,7 @@ static constexpr FormatInfo g_format_infos[] = {
{eFormatHex, 'x', "hex"},
{eFormatHexUppercase, 'X', "uppercase hex"},
{eFormatFloat, 'f', "float"},
{eFormatFloat128, '\0', "float128"},
Copy link
Collaborator

@jasonmolenda jasonmolenda Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this part of lldb, but do we need an entry with a character code that can't be entered? I don't think there are commands which take the "long name" of these entries, do they? Vaguely relatedly, I wondered about OptionGroupFormat::ParserGDBFormatLetter which recognizes the gdb formatters (v. https://sourceware.org/gdb/current/onlinedocs/gdb.html/Output-Formats.html ) but I think just "f" for "float of undetermined size" is correct as-is? This is seen in commands like x/2gx $fp etc, which command aliases rewrite to a longer memory read.

To be clear the way these gdb type specifiers work is one character for the size ("g" above) and one character for the type ("x" above), so I don't think any changes are needed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a static assert just below the array which states "All formats must have a corresponding info entry.", and will cause compilation to fail otherwise. Commands such as type format add -f <format> <type> can take either the single letter code or the long name to identify the format ('\0' is used to represent the format not having a single letter code).

On platforms with a 16-byte long double, the f format code will continue to map to eFormatFloat and therefore use long double for 16-byte values, just like in GDB.

@beetrees
Copy link
Contributor Author

(I've fixed the CI failure.)

@beetrees
Copy link
Contributor Author

beetrees commented May 6, 2025

I've rebased to fix the merge conflict. Friendly ping @JDevlieghere.

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Do the other reviewers have any other concerns? If not I'm happy to merge this on your behalf.

GetFloatSemantics(target_sp, item_byte_size);
item_format == eFormatFloat128 && item_byte_size == 16
? llvm::APFloat::IEEEquad()
: GetFloatSemantics(target_sp, item_byte_size);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't this byte_size check be part of GetFloatSemantics?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved the it inside GetFloatSemantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.