llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) <details> <summary>Changes</summary> Depends on: * https://github.com/llvm/llvm-project/pull/178904 (only last commit is relevant for the review) This is part of a patch series to clean up the TypeSystemClang::IsFloatingPointType API. The `is_complex` parameter is rarely checked. This patch introduces a `CompilerType::IsComplexType` API which callers that previously checked `is_complex` can use instead. --- Patch is 44.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/178906.diff 20 Files Affected: - (modified) lldb/include/lldb/Symbol/CompilerType.h (+7-1) - (modified) lldb/include/lldb/Symbol/TypeSystem.h (+1-2) - (modified) lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp (+8-13) - (modified) lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp (-8) - (modified) lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp (+10-19) - (modified) lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp (+5-9) - (modified) lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp (+40-56) - (modified) lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp (+2-3) - (modified) lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp (+24-31) - (modified) lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp (+23-29) - (modified) lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp (+7-11) - (modified) lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp (+27-32) - (modified) lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp (-8) - (modified) lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp (+29-38) - (modified) lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp (+29-38) - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+1-2) - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+5-14) - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+1-2) - (modified) lldb/source/Symbol/CompilerType.cpp (+15-9) - (modified) lldb/unittests/Symbol/TestTypeSystemClang.cpp (+36) ``````````diff diff --git a/lldb/include/lldb/Symbol/CompilerType.h b/lldb/include/lldb/Symbol/CompilerType.h index 869c5076ee0a7..b65a75041c387 100644 --- a/lldb/include/lldb/Symbol/CompilerType.h +++ b/lldb/include/lldb/Symbol/CompilerType.h @@ -144,7 +144,13 @@ class CompilerType { bool IsDefined() const; - bool IsFloatingPointType(bool &is_complex) const; + bool IsComplexType() const; + + /// Returns \c true for floating point types (including complex floats). + bool IsFloatingPointType() const; + + /// Returns \c true for non-complex float types. + bool IsRealFloatingPointType() const; bool IsFunctionType() const; diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h index 99ea0585e5370..d7f4cfcf0ffc7 100644 --- a/lldb/include/lldb/Symbol/TypeSystem.h +++ b/lldb/include/lldb/Symbol/TypeSystem.h @@ -162,8 +162,7 @@ class TypeSystem : public PluginInterface, virtual bool IsDefined(lldb::opaque_compiler_type_t type) = 0; - virtual bool IsFloatingPointType(lldb::opaque_compiler_type_t type, - bool &is_complex) = 0; + virtual bool IsFloatingPointType(lldb::opaque_compiler_type_t type) = 0; virtual bool IsFunctionType(lldb::opaque_compiler_type_t type) = 0; diff --git a/lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp b/lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp index e41a28bd21c36..95f4057b1d3e8 100644 --- a/lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp +++ b/lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp @@ -479,19 +479,14 @@ ABISysV_arc::GetReturnValueObjectSimple(Thread &thread, value.SetValueType(Value::ValueType::Scalar); } // Floating point return type. - else if (type_flags & eTypeIsFloat) { - bool is_complex = false; - - if (compiler_type.IsFloatingPointType(is_complex) && - !compiler_type.IsVectorType() && !is_complex) { - const size_t byte_size = - llvm::expectedToOptional(compiler_type.GetByteSize(&thread)) - .value_or(0); - auto raw_value = ReadRawValue(reg_ctx, byte_size); - - if (!SetSizedFloat(value.GetScalar(), raw_value, byte_size)) - return ValueObjectSP(); - } + else if (compiler_type.IsRealFloatingPointType()) { + const size_t byte_size = + llvm::expectedToOptional(compiler_type.GetByteSize(&thread)) + .value_or(0); + auto raw_value = ReadRawValue(reg_ctx, byte_size); + + if (!SetSizedFloat(value.GetScalar(), raw_value, byte_size)) + return ValueObjectSP(); } // Unsupported return type. else diff --git a/lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp b/lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp index 8e690218843fa..84791a90a450d 100644 --- a/lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp +++ b/lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp @@ -1695,7 +1695,6 @@ Status ABIMacOSX_arm::SetReturnValueObject(lldb::StackFrameSP &frame_sp, Thread *thread = frame_sp->GetThread().get(); bool is_signed; - bool is_complex; RegisterContext *reg_ctx = thread->GetRegisterContext().get(); @@ -1766,13 +1765,6 @@ Status ABIMacOSX_arm::SetReturnValueObject(lldb::StackFrameSP &frame_sp, "We don't support returning longer than 64 bit " "integer values at present."); } - } else if (compiler_type.IsFloatingPointType(is_complex)) { - if (is_complex) - error = Status::FromErrorString( - "We don't support returning complex values at present"); - else - error = Status::FromErrorString( - "We don't support returning float values at present"); } if (!set_it_simple) diff --git a/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp b/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp index 7258f5cc9acb5..019ad9b794905 100644 --- a/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp +++ b/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp @@ -1549,7 +1549,6 @@ ValueObjectSP ABISysV_arm::GetReturnValueObjectImpl( return return_valobj_sp; bool is_signed; - bool is_complex; bool is_vfp_candidate = false; uint8_t vfp_count = 0; uint8_t vfp_byte_size = 0; @@ -1633,9 +1632,9 @@ ValueObjectSP ABISysV_arm::GetReturnValueObjectImpl( if (!GetReturnValuePassedInMemory(thread, reg_ctx, *byte_size, value)) return return_valobj_sp; } - } else if (compiler_type.IsFloatingPointType(is_complex)) { + } else if (compiler_type.IsFloatingPointType()) { // Vector types are handled above. - if (!is_complex) { + if (!compiler_type.IsCompleteType()) { switch (*bit_width) { default: return return_valobj_sp; @@ -1681,7 +1680,7 @@ ValueObjectSP ABISysV_arm::GetReturnValueObjectImpl( break; } } - } else if (is_complex) { + } else { if (IsArmHardFloat(thread)) { is_vfp_candidate = true; vfp_byte_size = *byte_size / 2; @@ -1689,9 +1688,7 @@ ValueObjectSP ABISysV_arm::GetReturnValueObjectImpl( } else if (!GetReturnValuePassedInMemory(thread, reg_ctx, *bit_width / 8, value)) return return_valobj_sp; - } else - // not handled yet - return return_valobj_sp; + } } else if (compiler_type.IsAggregateType()) { if (IsArmHardFloat(thread)) { CompilerType base_type; @@ -1709,9 +1706,10 @@ ValueObjectSP ABISysV_arm::GetReturnValueObjectImpl( vfp_count = (*base_byte_size == 8 ? homogeneous_count : homogeneous_count * 2); } - } else if (base_type.IsFloatingPointType(is_complex)) { - // Vector types are handled above. - if (!is_complex) { + } else if (base_type.IsFloatingPointType()) { + assert(!base_type.IsVectorType() && + "Vector types should've been handled above."); + if (!base_type.IsComplexType()) { is_vfp_candidate = true; if (base_byte_size) vfp_byte_size = *base_byte_size; @@ -1728,10 +1726,10 @@ ValueObjectSP ABISysV_arm::GetReturnValueObjectImpl( base_type = compiler_type.GetFieldAtIndex(index, name, nullptr, nullptr, nullptr); - if (base_type.IsFloatingPointType(is_complex)) { + if (base_type.IsFloatingPointType()) { std::optional<uint64_t> base_byte_size = llvm::expectedToOptional(base_type.GetByteSize(&thread)); - if (is_complex) { + if (base_type.IsComplexType()) { if (index != 0 && base_byte_size && vfp_byte_size != *base_byte_size) break; @@ -1884,13 +1882,6 @@ Status ABISysV_arm::SetReturnValueObject(lldb::StackFrameSP &frame_sp, "We don't support returning longer than 64 bit " "integer values at present."); } - } else if (compiler_type.IsFloatingPointType(is_complex)) { - if (is_complex) - error = Status::FromErrorString( - "We don't support returning complex values at present"); - else - error = Status::FromErrorString( - "We don't support returning float values at present"); } if (!set_it_simple) diff --git a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp index 91b965d3b5715..d2a2cbe0643ac 100644 --- a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp +++ b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp @@ -509,16 +509,12 @@ ValueObjectSP ABISysV_loongarch::GetReturnValueObjectSimple( return ValueObjectConstResult::Create(thread.GetStackFrameAtIndex(0).get(), value, ConstString("")); } - if (type_flags & eTypeIsFloat) { - bool is_complex = false; - - if (compiler_type.IsFloatingPointType(is_complex) && - !(type_flags & eTypeIsVector) && !is_complex) { - return_valobj_sp = - GetValObjFromFPRegs(thread, reg_ctx, machine, type_flags, byte_size); - return return_valobj_sp; - } + if (compiler_type.IsRealFloatingPointType()) { + return_valobj_sp = + GetValObjFromFPRegs(thread, reg_ctx, machine, type_flags, byte_size); + return return_valobj_sp; } + return return_valobj_sp; } diff --git a/lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp b/lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp index e03604467ceec..338d5c152f612 100644 --- a/lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp +++ b/lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp @@ -708,7 +708,6 @@ Status ABISysV_mips::SetReturnValueObject(lldb::StackFrameSP &frame_sp, Thread *thread = frame_sp->GetThread().get(); bool is_signed; - bool is_complex; RegisterContext *reg_ctx = thread->GetRegisterContext().get(); @@ -749,13 +748,6 @@ Status ABISysV_mips::SetReturnValueObject(lldb::StackFrameSP &frame_sp, "We don't support returning longer than 64 bit " "integer values at present."); } - } else if (compiler_type.IsFloatingPointType(is_complex)) { - if (is_complex) - error = Status::FromErrorString( - "We don't support returning complex values at present"); - else - error = Status::FromErrorString( - "We don't support returning float values at present"); } if (!set_it_simple) @@ -795,7 +787,6 @@ ValueObjectSP ABISysV_mips::GetReturnValueObjectImpl( return return_valobj_sp; bool is_signed = false; - bool is_complex = false; // In MIPS register "r2" (v0) holds the integer function return values const RegisterInfo *r2_reg_info = reg_ctx->GetRegisterInfoByName("r2", 0); @@ -858,11 +849,9 @@ ValueObjectSP ABISysV_mips::GetReturnValueObjectImpl( return_valobj_sp = ValueObjectMemory::Create( &thread, "", Address(mem_address, nullptr), return_compiler_type); return return_valobj_sp; - } else if (return_compiler_type.IsFloatingPointType(is_complex)) { + } else if (return_compiler_type.IsRealFloatingPointType()) { if (IsSoftFloat(fp_flag)) { uint64_t raw_value = reg_ctx->ReadRegisterAsUnsigned(r2_reg_info, 0); - if (is_complex) - return return_valobj_sp; switch (*bit_width) { default: return return_valobj_sp; @@ -894,51 +883,46 @@ ValueObjectSP ABISysV_mips::GetReturnValueObjectImpl( f0_value.GetData(f0_data); lldb::offset_t offset = 0; - if (!return_compiler_type.IsVectorType() && !is_complex) { - switch (*bit_width) { - default: - return return_valobj_sp; - case 64: { - static_assert(sizeof(double) == sizeof(uint64_t)); - const RegisterInfo *f1_info = reg_ctx->GetRegisterInfoByName("f1", 0); - RegisterValue f1_value; - DataExtractor f1_data; - reg_ctx->ReadRegister(f1_info, f1_value); - DataExtractor *copy_from_extractor = nullptr; - WritableDataBufferSP data_sp(new DataBufferHeap(8, 0)); - DataExtractor return_ext( - data_sp, target_byte_order, - target->GetArchitecture().GetAddressByteSize()); - - if (target_byte_order == eByteOrderLittle) { - copy_from_extractor = &f0_data; - copy_from_extractor->CopyByteOrderedData( - offset, 4, data_sp->GetBytes(), 4, target_byte_order); - f1_value.GetData(f1_data); - copy_from_extractor = &f1_data; - copy_from_extractor->CopyByteOrderedData( - offset, 4, data_sp->GetBytes() + 4, 4, target_byte_order); - } else { - copy_from_extractor = &f0_data; - copy_from_extractor->CopyByteOrderedData( - offset, 4, data_sp->GetBytes() + 4, 4, target_byte_order); - f1_value.GetData(f1_data); - copy_from_extractor = &f1_data; - copy_from_extractor->CopyByteOrderedData( - offset, 4, data_sp->GetBytes(), 4, target_byte_order); - } - value.GetScalar() = (double)return_ext.GetDouble(&offset); - break; - } - case 32: { - static_assert(sizeof(float) == sizeof(uint32_t)); - value.GetScalar() = (float)f0_data.GetFloat(&offset); - break; - } - } - } else { - // not handled yet + switch (*bit_width) { + default: return return_valobj_sp; + case 64: { + static_assert(sizeof(double) == sizeof(uint64_t)); + const RegisterInfo *f1_info = reg_ctx->GetRegisterInfoByName("f1", 0); + RegisterValue f1_value; + DataExtractor f1_data; + reg_ctx->ReadRegister(f1_info, f1_value); + DataExtractor *copy_from_extractor = nullptr; + WritableDataBufferSP data_sp(new DataBufferHeap(8, 0)); + DataExtractor return_ext( + data_sp, target_byte_order, + target->GetArchitecture().GetAddressByteSize()); + + if (target_byte_order == eByteOrderLittle) { + copy_from_extractor = &f0_data; + copy_from_extractor->CopyByteOrderedData( + offset, 4, data_sp->GetBytes(), 4, target_byte_order); + f1_value.GetData(f1_data); + copy_from_extractor = &f1_data; + copy_from_extractor->CopyByteOrderedData( + offset, 4, data_sp->GetBytes() + 4, 4, target_byte_order); + } else { + copy_from_extractor = &f0_data; + copy_from_extractor->CopyByteOrderedData( + offset, 4, data_sp->GetBytes() + 4, 4, target_byte_order); + f1_value.GetData(f1_data); + copy_from_extractor = &f1_data; + copy_from_extractor->CopyByteOrderedData( + offset, 4, data_sp->GetBytes(), 4, target_byte_order); + } + value.GetScalar() = (double)return_ext.GetDouble(&offset); + break; + } + case 32: { + static_assert(sizeof(float) == sizeof(uint32_t)); + value.GetScalar() = (float)f0_data.GetFloat(&offset); + break; + } } } } else { diff --git a/lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp b/lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp index 0dd9db0948220..af71f4ad08342 100644 --- a/lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp +++ b/lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp @@ -922,7 +922,6 @@ ValueObjectSP ABISysV_mips64::GetReturnValueObjectImpl( // True if the result is copied into our data buffer bool sucess = false; std::string name; - bool is_complex; const uint32_t num_children = return_compiler_type.GetNumFields(); // A structure consisting of one or two FP values (and nothing else) will @@ -936,7 +935,7 @@ ValueObjectSP ABISysV_mips64::GetReturnValueObjectImpl( return_compiler_type.GetFieldAtIndex(idx, name, &field_bit_offset, nullptr, nullptr); - if (field_compiler_type.IsFloatingPointType(is_complex)) + if (field_compiler_type.IsFloatingPointType()) use_fp_regs = true; else found_non_fp_field = true; @@ -1043,7 +1042,7 @@ ValueObjectSP ABISysV_mips64::GetReturnValueObjectImpl( if (field_compiler_type.IsIntegerOrEnumerationType(is_signed) || field_compiler_type.IsPointerType() || - field_compiler_type.IsFloatingPointType(is_complex)) { + field_compiler_type.IsFloatingPointType()) { padding = field_byte_offset - integer_bytes; if (integer_bytes < 8) { diff --git a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp index 0d25faef1c659..604a6d6ee9c16 100644 --- a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp +++ b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp @@ -426,7 +426,6 @@ Status ABISysV_ppc::SetReturnValueObject(lldb::StackFrameSP &frame_sp, Thread *thread = frame_sp->GetThread().get(); bool is_signed; - bool is_complex; RegisterContext *reg_ctx = thread->GetRegisterContext().get(); @@ -453,38 +452,33 @@ Status ABISysV_ppc::SetReturnValueObject(lldb::StackFrameSP &frame_sp, "We don't support returning longer than 64 bit " "integer values at present."); } - } else if (compiler_type.IsFloatingPointType(is_complex)) { - if (is_complex) - error = Status::FromErrorString( - "We don't support returning complex values at present"); - else { - std::optional<uint64_t> bit_width = - llvm::expectedToOptional(compiler_type.GetBitSize(frame_sp.get())); - if (!bit_width) { - error = Status::FromErrorString("can't get type size"); + } else if (compiler_type.IsRealFloatingPointType()) { + std::optional<uint64_t> bit_width = + llvm::expectedToOptional(compiler_type.GetBitSize(frame_sp.get())); + if (!bit_width) { + error = Status::FromErrorString("can't get type size"); + return error; + } + if (*bit_width <= 64) { + DataExtractor data; + Status data_error; + size_t num_bytes = new_value_sp->GetData(data, data_error); + if (data_error.Fail()) { + error = Status::FromErrorStringWithFormat( + "Couldn't convert return value to raw data: %s", + data_error.AsCString()); return error; } - if (*bit_width <= 64) { - DataExtractor data; - Status data_error; - size_t num_bytes = new_value_sp->GetData(data, data_error); - if (data_error.Fail()) { - error = Status::FromErrorStringWithFormat( - "Couldn't convert return value to raw data: %s", - data_error.AsCString()); - return error; - } - unsigned char buffer[16]; - ByteOrder byte_order = data.GetByteOrder(); + unsigned char buffer[16]; + ByteOrder byte_order = data.GetByteOrder(); - data.CopyByteOrderedData(0, num_bytes, buffer, 16, byte_order); - set_it_simple = true; - } else { - // FIXME - don't know how to do 80 bit long doubles yet. - error = Status::FromErrorString( - "We don't support returning float values > 64 bits at present"); - } + data.CopyByteOrderedData(0, num_bytes, buffer, 16, byte_order); + set_it_simple = true; + } else { + // FIXME - don't know how to do 80 bit long doubles yet. + error = Status::FromErrorString( + "We don't support returning float values > 64 bits at present"); } } @@ -693,7 +687,6 @@ ValueObjectSP ABISysV_ppc::GetReturnValueObjectImpl( std::string name; uint64_t field_bit_offset = 0; bool is_signed; - bool is_complex; CompilerType field_compiler_type = return_compiler_type.GetFieldAtIndex( idx, name, &field_bit_offset, nullptr, nullptr); @@ -739,7 +732,7 @@ ValueObjectSP ABISysV_ppc::GetReturnValueObjectImpl( // return a nullptr return value object. return return_valobj_sp; } - } else if (field_compiler_type.IsFloatingPointType(is_complex)) { + } else if (field_compiler_type.IsFloatingPointType()) { // Structs with long doubles are always passed in memory. if (*field_bit_width == 128) { is_memory = true; diff --git a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp index 63357618774d4..0895cd3d75df6 100644 --- a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp +++ b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp @@ -309,7 +309,6 @@ Status ABISysV_ppc64::SetReturnValueObject(lldb::StackFrameSP &frame_sp, Thread *thread = frame_sp->GetThread().get(); bool is_signed; - bool is_complex; RegisterContext *reg_ctx = thread->GetRegisterContext().get(); @@ -338,38 +337,33 @@ Status ABISysV_ppc64::SetReturnValueObject(lldb::StackFrameSP &frame_sp, "We don't support returning longer than 64 bit " "integer values at present."); } - } else if (compiler_type.IsFloatingPointType(is_complex)) { - if (is_complex) - error = Status::FromErrorString( - "We don't support returning complex values at present"); - else { - std::optional<uint64_t> bit_width = - llvm::expectedToOptional(compiler_type.GetBitSize(frame_sp.get())); - if (!bit_width) { - error = Status::FromErrorString("can't get size of type"); + } else if (compil... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/178906 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
