-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -795,6 +795,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: | ||
|
@@ -956,6 +958,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); | ||
|
@@ -965,6 +974,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: | ||
|
@@ -2054,6 +2065,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: | ||
|
@@ -4742,19 +4755,24 @@ CompilerType TypeSystemClang::CreateGenericFunctionPrototype() { | |
// Exploring the type | ||
|
||
const llvm::fltSemantics & | ||
TypeSystemClang::GetFloatTypeSemantics(size_t byte_size) { | ||
TypeSystemClang::GetFloatTypeSemantics(size_t byte_size, lldb::Format format) { | ||
clang::ASTContext &ast = getASTContext(); | ||
const size_t bit_size = byte_size * 8; | ||
if (bit_size == ast.getTypeSize(ast.FloatTy)) | ||
return ast.getFloatTypeSemantics(ast.FloatTy); | ||
else if (bit_size == ast.getTypeSize(ast.DoubleTy)) | ||
return ast.getFloatTypeSemantics(ast.DoubleTy); | ||
else if (format == eFormatFloat128 && | ||
bit_size == ast.getTypeSize(ast.Float128Ty)) | ||
return ast.getFloatTypeSemantics(ast.Float128Ty); | ||
else if (bit_size == ast.getTypeSize(ast.LongDoubleTy) || | ||
bit_size == llvm::APFloat::semanticsSizeInBits( | ||
ast.getFloatTypeSemantics(ast.LongDoubleTy))) | ||
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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this fallback branch? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the user formats a 128-bit value using |
||
return ast.getFloatTypeSemantics(ast.Float128Ty); | ||
return llvm::APFloatBase::Bogus(); | ||
} | ||
|
||
|
@@ -5232,6 +5250,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; | ||
} | ||
|
@@ -5529,6 +5549,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; | ||
|
@@ -6090,6 +6112,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: | ||
|
@@ -8733,6 +8756,7 @@ bool TypeSystemClang::DumpTypeValue( | |
case eFormatHex: | ||
case eFormatHexUppercase: | ||
case eFormatFloat: | ||
case eFormatFloat128: | ||
case eFormatOctal: | ||
case eFormatOSType: | ||
case eFormatUnsigned: | ||
|
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.
Uh oh!
There was an error while loading. Please reload this page.
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.