-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-lldb Author: None (beetrees) ChangesAdds support to LLDB for displaying 128-bit IEEE 754 floats ( Full diff: https://github.com/llvm/llvm-project/pull/98369.diff 11 Files Affected:
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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
|
@@ -46,6 +46,7 @@ static constexpr FormatInfo g_format_infos[] = { | ||
{eFormatHex, 'x', "hex"}, | ||
{eFormatHexUppercase, 'X', "uppercase hex"}, | ||
{eFormatFloat, 'f', "float"}, | ||
{eFormatFloat128, '\0', "float128"}, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
(I've fixed the CI failure.) |
I've rebased to fix the merge conflict. Friendly ping @JDevlieghere. |
There was a problem hiding this 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); |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
.
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/PPClong double
(see #98179), the type name is used to disambiguate instead.(Note to reviewer: I don't have commit access)