llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-risc-v Author: Michael Buch (Michael137) <details> <summary>Changes</summary> This is part of a patch series to clean up the `TypeSystemClang::IsFloatingPointType` API. Currently the API is a bit of a foot-gun because it returns `true` for both Complex floats and vector types whose element types are floats. The former aligns with the `clang::Type::isFloatingType` API, but the latter doesn't. This specific implementation choice will be addressed in a separate patch. This patch adds a new `CompilerType::IsRealFloatingPointType` API which clients can use to query about non-complex floats. For `TypeSystemClang` this just dispatches to the similarly named `clang::Type::isRealFloatingType`. This allows us to clean up some of the callers which only wanted to handle non-complex floats. On encountering complex/vector floats, some of the ABI plugins would set an error but then immediately overwrite the error to something else. I just removed those branches entirely, since it was essentially dead code. --- Patch is 30.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/178904.diff 16 Files Affected: - (modified) lldb/include/lldb/Symbol/CompilerType.h (+4) - (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 (-7) - (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/PowerPC/ABISysV_ppc.cpp (+23-29) - (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 (+27-34) - (modified) lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp (+27-34) - (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+1-2) - (modified) lldb/source/Symbol/CompilerType.cpp (+5) - (modified) lldb/unittests/Symbol/TestTypeSystemClang.cpp (+21) ``````````diff diff --git a/lldb/include/lldb/Symbol/CompilerType.h b/lldb/include/lldb/Symbol/CompilerType.h index 869c5076ee0a7..4f1662ee6d162 100644 --- a/lldb/include/lldb/Symbol/CompilerType.h +++ b/lldb/include/lldb/Symbol/CompilerType.h @@ -144,8 +144,12 @@ class CompilerType { bool IsDefined() const; + /// Returns \c true for floating point types (including complex floats). bool IsFloatingPointType(bool &is_complex) const; + /// Returns \c true for non-complex float types. + bool IsRealFloatingPointType() const; + bool IsFunctionType() const; uint32_t IsHomogeneousAggregate(CompilerType *base_type_ptr) const; 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..0c53542e72f22 100644 --- a/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp +++ b/lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp @@ -1884,13 +1884,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/PowerPC/ABISysV_ppc.cpp b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp index 0d25faef1c659..944bc66bafeb1 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"); } } 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 (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 size of type"); + 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"); } } diff --git a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp index d209980d65589..dce905f08aa5f 100644 --- a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp +++ b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp @@ -642,18 +642,14 @@ ABISysV_riscv::GetReturnValueObjectSimple(Thread &thread, value, ConstString("")); } // Floating point return type. - else if (type_flags & eTypeIsFloat) { - bool is_complex = false; - - if (compiler_type.IsFloatingPointType(is_complex) && - !(type_flags & eTypeIsVector) && !is_complex) { - const uint32_t arch_fp_flags = - arch.GetFlags() & ArchSpec::eRISCV_float_abi_mask; - return_valobj_sp = GetValObjFromFPRegs( - thread, reg_ctx, machine, arch_fp_flags, type_flags, byte_size); - return return_valobj_sp; - } + else if (compiler_type.IsRealFloatingPointType()) { + const uint32_t arch_fp_flags = + arch.GetFlags() & ArchSpec::eRISCV_float_abi_mask; + return_valobj_sp = GetValObjFromFPRegs( + thread, reg_ctx, machine, arch_fp_flags, type_flags, byte_size); + return return_valobj_sp; } + // Unsupported return type. return return_valobj_sp; } diff --git a/lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp b/lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp index 301c3b309ffd5..594bf27f7e0b7 100644 --- a/lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp +++ b/lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp @@ -422,42 +422,37 @@ Status ABISysV_s390x::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) { + const RegisterInfo *f0_info = reg_ctx->GetRegisterInfoByName("f0", 0); + RegisterValue f0_value; + 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) { - const RegisterInfo *f0_info = reg_ctx->GetRegisterInfoByName("f0", 0); - RegisterValue f0_value; - 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[8]; - ByteOrder byte_order = data.GetByteOrder(); + unsigned char buffer[8]; + ByteOrder byte_order = data.GetByteOrder(); - data.CopyByteOrderedData(0, num_bytes, buffer, 8, byte_order); - f0_value.SetBytes(buffer, 8, byte_order); - reg_ctx->WriteRegister(f0_info, f0_value); - set_it_simple = true; - } else { - // FIXME - don't know how to do long doubles yet. - error = Status::FromErrorString( - "We don't suppo... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/178904 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
