[lldb][TypeSystem] Remove count parameter from TypeSystem::IsFloatingPointType #165707
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…PointType Similar motivation to llvm#165702. It was unused in all callsites and inconsistent with other APIs like `IsIntegerType` (which doesn't take a `count` parameter). If we ever need a "how many elements does this type represent", we can implement one with a new TypeSystem API that does exactly that.
|
@llvm/pr-subscribers-backend-mips @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesSimilar motivation to #165702. It was unused in all callsites and inconsistent with other APIs like If we ever need a "how many elements does this type represent", we can implement one with a new TypeSystem API that does exactly that. Full diff: https://github.com/llvm/llvm-project/pull/165707.diff 9 Files Affected:
diff --git a/lldb/include/lldb/Symbol/CompilerType.h b/lldb/include/lldb/Symbol/CompilerType.h
index df8489a7fe582..43b2505fdab09 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -144,7 +144,7 @@ class CompilerType {
bool IsDefined() const;
- bool IsFloatingPointType(uint32_t &count, bool &is_complex) const;
+ bool IsFloatingPointType(bool &is_complex) const;
bool IsFunctionType() const;
diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h
index 0ec3a28898329..6f5292bcae4e5 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -163,7 +163,7 @@ class TypeSystem : public PluginInterface,
virtual bool IsDefined(lldb::opaque_compiler_type_t type) = 0;
virtual bool IsFloatingPointType(lldb::opaque_compiler_type_t type,
- uint32_t &count, bool &is_complex) = 0;
+ bool &is_complex) = 0;
virtual bool IsFunctionType(lldb::opaque_compiler_type_t type) = 0;
diff --git a/lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp b/lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp
index eaeed6c04590c..ee79abe55ead0 100644
--- a/lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp
+++ b/lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp
@@ -198,7 +198,6 @@ Status ABIMacOSX_i386::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
Thread *thread = frame_sp->GetThread().get();
bool is_signed;
- uint32_t count;
bool is_complex;
RegisterContext *reg_ctx = thread->GetRegisterContext().get();
@@ -240,7 +239,7 @@ Status ABIMacOSX_i386::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
"We don't support returning longer than 64 bit "
"integer values at present.");
}
- } else if (compiler_type.IsFloatingPointType(count, is_complex)) {
+ } else if (compiler_type.IsFloatingPointType(is_complex)) {
if (is_complex)
error = Status::FromErrorString(
"We don't support returning complex values at present");
diff --git a/lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp b/lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
index effb3de8215d6..29fd9f0eceb93 100644
--- a/lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
+++ b/lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
@@ -307,7 +307,6 @@ Status ABISysV_x86_64::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
Thread *thread = frame_sp->GetThread().get();
bool is_signed;
- uint32_t count;
bool is_complex;
RegisterContext *reg_ctx = thread->GetRegisterContext().get();
@@ -337,7 +336,7 @@ Status ABISysV_x86_64::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
"We don't support returning longer than 64 bit "
"integer values at present.");
}
- } else if (compiler_type.IsFloatingPointType(count, is_complex)) {
+ } else if (compiler_type.IsFloatingPointType(is_complex)) {
if (is_complex)
error = Status::FromErrorString(
"We don't support returning complex values at present");
@@ -587,7 +586,6 @@ static bool FlattenAggregateType(
for (uint32_t idx = 0; idx < num_children; ++idx) {
std::string name;
bool is_signed;
- uint32_t count;
bool is_complex;
uint64_t field_bit_offset = 0;
@@ -606,7 +604,7 @@ static bool FlattenAggregateType(
const uint32_t field_type_flags = field_compiler_type.GetTypeInfo();
if (field_compiler_type.IsIntegerOrEnumerationType(is_signed) ||
field_compiler_type.IsPointerType() ||
- field_compiler_type.IsFloatingPointType(count, is_complex)) {
+ field_compiler_type.IsFloatingPointType(is_complex)) {
aggregate_field_offsets.push_back(field_byte_offset);
aggregate_compiler_types.push_back(field_compiler_type);
} else if (field_type_flags & eTypeHasChildren) {
@@ -696,7 +694,6 @@ ValueObjectSP ABISysV_x86_64::GetReturnValueObjectImpl(
is_memory = false;
for (uint32_t idx = 0; idx < num_children; idx++) {
bool is_signed;
- uint32_t count;
bool is_complex;
CompilerType field_compiler_type = aggregate_compiler_types[idx];
@@ -736,7 +733,7 @@ ValueObjectSP ABISysV_x86_64::GetReturnValueObjectImpl(
// return a nullptr return value object.
return return_valobj_sp;
}
- } else if (field_compiler_type.IsFloatingPointType(count, is_complex)) {
+ } else if (field_compiler_type.IsFloatingPointType(is_complex)) {
// Structs with long doubles are always passed in memory.
if (field_bit_width == 128) {
is_memory = true;
diff --git a/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp b/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
index 339012cffb688..6520af2f643ee 100644
--- a/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
+++ b/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
@@ -312,7 +312,6 @@ Status ABIWindows_x86_64::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
Thread *thread = frame_sp->GetThread().get();
bool is_signed;
- uint32_t count;
bool is_complex;
RegisterContext *reg_ctx = thread->GetRegisterContext().get();
@@ -342,7 +341,7 @@ Status ABIWindows_x86_64::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
"We don't support returning longer than 64 bit "
"integer values at present.");
}
- } else if (compiler_type.IsFloatingPointType(count, is_complex)) {
+ } else if (compiler_type.IsFloatingPointType(is_complex)) {
if (is_complex)
error = Status::FromErrorString(
"We don't support returning complex values at present");
@@ -558,7 +557,6 @@ static bool FlattenAggregateType(
for (uint32_t idx = 0; idx < num_children; ++idx) {
std::string name;
bool is_signed;
- uint32_t count;
bool is_complex;
uint64_t field_bit_offset = 0;
@@ -582,7 +580,7 @@ static bool FlattenAggregateType(
const uint32_t field_type_flags = field_compiler_type.GetTypeInfo();
if (field_compiler_type.IsIntegerOrEnumerationType(is_signed) ||
field_compiler_type.IsPointerType() ||
- field_compiler_type.IsFloatingPointType(count, is_complex)) {
+ field_compiler_type.IsFloatingPointType(is_complex)) {
aggregate_field_offsets.push_back(field_byte_offset);
aggregate_compiler_types.push_back(field_compiler_type);
} else if (field_type_flags & eTypeHasChildren) {
@@ -672,7 +670,6 @@ ValueObjectSP ABIWindows_x86_64::GetReturnValueObjectImpl(
for (uint32_t idx = 0; idx < num_children; idx++) {
bool is_signed;
bool is_complex;
- uint32_t count;
CompilerType field_compiler_type = aggregate_compiler_types[idx];
uint32_t field_byte_width =
@@ -691,7 +688,7 @@ ValueObjectSP ABIWindows_x86_64::GetReturnValueObjectImpl(
uint32_t copy_from_offset = 0;
if (field_compiler_type.IsIntegerOrEnumerationType(is_signed) ||
field_compiler_type.IsPointerType() ||
- field_compiler_type.IsFloatingPointType(count, is_complex)) {
+ field_compiler_type.IsFloatingPointType(is_complex)) {
copy_from_extractor = &rax_data;
copy_from_offset = used_bytes;
used_bytes += field_byte_width;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 82e9d867c3ac0..82d7b5c6d8063 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2032,11 +2032,10 @@ static std::optional<clang::APValue> MakeAPValue(const clang::ASTContext &ast,
if (is_integral)
return clang::APValue(apint);
- uint32_t count;
bool is_complex;
// FIXME: we currently support a limited set of floating point types.
// E.g., 16-bit floats are not supported.
- if (!clang_type.IsFloatingPointType(count, is_complex))
+ if (!clang_type.IsFloatingPointType(is_complex))
return std::nullopt;
return clang::APValue(llvm::APFloat(
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 82dfe7e540717..24cd3741110bb 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -3483,7 +3483,7 @@ bool TypeSystemClang::IsReferenceType(lldb::opaque_compiler_type_t type,
}
bool TypeSystemClang::IsFloatingPointType(lldb::opaque_compiler_type_t type,
- uint32_t &count, bool &is_complex) {
+ bool &is_complex) {
if (type) {
clang::QualType qual_type(GetCanonicalQualType(type));
@@ -3492,30 +3492,26 @@ bool TypeSystemClang::IsFloatingPointType(lldb::opaque_compiler_type_t type,
clang::BuiltinType::Kind kind = BT->getKind();
if (kind >= clang::BuiltinType::Float &&
kind <= clang::BuiltinType::LongDouble) {
- count = 1;
is_complex = false;
return true;
}
} else if (const clang::ComplexType *CT =
llvm::dyn_cast<clang::ComplexType>(
qual_type->getCanonicalTypeInternal())) {
- if (IsFloatingPointType(CT->getElementType().getAsOpaquePtr(), count,
+ if (IsFloatingPointType(CT->getElementType().getAsOpaquePtr(),
is_complex)) {
- count = 2;
is_complex = true;
return true;
}
} else if (const clang::VectorType *VT = llvm::dyn_cast<clang::VectorType>(
qual_type->getCanonicalTypeInternal())) {
- if (IsFloatingPointType(VT->getElementType().getAsOpaquePtr(), count,
+ if (IsFloatingPointType(VT->getElementType().getAsOpaquePtr(),
is_complex)) {
- count = VT->getNumElements();
is_complex = false;
return true;
}
}
}
- count = 0;
is_complex = false;
return false;
}
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 9e0a54209345d..46ed80eb32239 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -651,7 +651,7 @@ class TypeSystemClang : public TypeSystem {
bool IsDefined(lldb::opaque_compiler_type_t type) override;
- bool IsFloatingPointType(lldb::opaque_compiler_type_t type, uint32_t &count,
+ bool IsFloatingPointType(lldb::opaque_compiler_type_t type,
bool &is_complex) override;
unsigned GetPtrAuthKey(lldb::opaque_compiler_type_t type) override;
diff --git a/lldb/source/Symbol/CompilerType.cpp b/lldb/source/Symbol/CompilerType.cpp
index 62c0ddf51c012..8e3d52d6e5f39 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -240,13 +240,11 @@ bool CompilerType::ShouldTreatScalarValueAsAddress() const {
return false;
}
-bool CompilerType::IsFloatingPointType(uint32_t &count,
- bool &is_complex) const {
+bool CompilerType::IsFloatingPointType(bool &is_complex) const {
if (IsValid()) {
if (auto type_system_sp = GetTypeSystem())
- return type_system_sp->IsFloatingPointType(m_type, count, is_complex);
+ return type_system_sp->IsFloatingPointType(m_type, is_complex);
}
- count = 0;
is_complex = false;
return false;
}
@@ -331,9 +329,8 @@ bool CompilerType::IsInteger() const {
}
bool CompilerType::IsFloat() const {
- uint32_t count = 0;
bool is_complex = false;
- return IsFloatingPointType(count, is_complex);
+ return IsFloatingPointType(is_complex);
}
bool CompilerType::IsEnumerationType() const {
|
adrian-prantl
approved these changes
Oct 30, 2025
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Oct 31, 2025
We were setting these bits inverted. Not sure how this bug actually manifests, I just noticed when working on llvm#165707. I suspect these types just aren't very frequently used.
DavidSpickett
approved these changes
Oct 31, 2025
Michael137
added a commit
that referenced
this pull request
Oct 31, 2025
) We were setting these bits inverted. Not sure how this bug actually manifests, I just noticed when working on #165707. I suspect these types just aren't very frequently used.
Michael137
commented
Oct 31, 2025
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 31, 2025
…types (#165837) We were setting these bits inverted. Not sure how this bug actually manifests, I just noticed when working on llvm/llvm-project#165707. I suspect these types just aren't very frequently used.
google-yfyang
added a commit
to google-yfyang/llvm-project
that referenced
this pull request
Oct 31, 2025
google-yfyang
added a commit
to google-yfyang/llvm-project
that referenced
this pull request
Oct 31, 2025
google-yfyang
added a commit
to google-yfyang/llvm-project
that referenced
this pull request
Oct 31, 2025
rupprecht
pushed a commit
that referenced
this pull request
Oct 31, 2025
DEBADRIBASAK
pushed a commit
to DEBADRIBASAK/llvm-project
that referenced
this pull request
Nov 3, 2025
…#165837) We were setting these bits inverted. Not sure how this bug actually manifests, I just noticed when working on llvm#165707. I suspect these types just aren't very frequently used.
DEBADRIBASAK
pushed a commit
to DEBADRIBASAK/llvm-project
that referenced
this pull request
Nov 3, 2025
…PointType (llvm#165707) Similar motivation to llvm#165702. It was unused in all callsites and inconsistent with other APIs like `IsIntegerType` (which doesn't take a `count` parameter). If we ever need a "how many elements does this type represent", we can implement one with a new TypeSystem API that does exactly that. Some callsites checked for `count == 1` previously, but I suspect what they intended to do is check for whether it's a vector type or complex type, before reading the FP register. I'm somewhat confident that's the case because the `TypeSystemClang::GetTypeInfo` currently incorrectly sets the integer and floating point bits for complex and vector types (will fix separately). But some architectures might choose to pass single-element vectors in scalar registers. I should probably changes these to check the vector element size. All the `count == 2 && is_complex` were redundant because `count == 2` iff `is_complex == true`. So I just removed the count check there.
DEBADRIBASAK
pushed a commit
to DEBADRIBASAK/llvm-project
that referenced
this pull request
Nov 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similar motivation to #165702. It was unused in all callsites and inconsistent with other APIs like
IsIntegerType(which doesn't take acountparameter).If we ever need a "how many elements does this type represent", we can implement one with a new TypeSystem API that does exactly that.
Some callsites checked for
count == 1previously, but I suspect what they intended to do is check for whether it's a vector type or complex type, before reading the FP register. I'm somewhat confident that's the case because theTypeSystemClang::GetTypeInfocurrently incorrectly sets the integer and floating point bits for complex and vector types (will fix separately). But some architectures might choose to pass single-element vectors in scalar registers. I should probably changes these to check the vector element size.All the
count == 2 && is_complexwere redundant becausecount == 2iffis_complex == true. So I just removed the count check there.