[Lldb-commits] [PATCH] D20357: [LLDB][MIPS] Fix FPU Size Based on Dynamic FR
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Mon Jul 25 11:11:57 PDT 2016
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
A few comments:
- we need the DWARF opcode length in RegisterInfo, but not in the structured data (XML or JSON)
- all macros that initialize RegisterInfo structs must be expended to null and zero out the opcode pointer and size (in all register contexts for all archs)
- remove all virtual and overrides for GetDwarfOpcodeLength()
================
Comment at: include/lldb/Host/common/NativeRegisterContext.h:90-92
@@ -89,2 +89,5 @@
+ virtual size_t
+ GetDwarfOpcodeLength (const uint32_t i) const;
+
virtual uint32_t
----------------
Remove this, the length needs to stay in the RegisterInfo.
================
Comment at: include/lldb/Host/common/NativeRegisterContextRegisterInfo.h:40-42
@@ -39,2 +39,5 @@
+ size_t
+ GetDwarfOpcodeLength (const uint32_t i) const override;
+
const RegisterInfoInterface&
----------------
Remove this, the length needs to stay in the RegisterInfo.
================
Comment at: include/lldb/Target/RegisterContext.h:53
@@ +52,3 @@
+ RegisterInfo* reg_info,
+ size_t opcode_len);
+
----------------
Remove opcode_len, the length needs to stay in the RegisterInfo.
================
Comment at: include/lldb/lldb-private-types.h:57-58
@@ -56,2 +56,4 @@
// ax, ah, and al.
+ const uint8_t *dynamic_size_dwarf_expr_bytes; // A DWARF expression that when evaluated gives
+ // the byte size of this register.
};
----------------
So we either remove both fields from RegisterInfo, or we leave both in. If we remove them both, then getting the register info from the RegisterContext will update the register info automatically if needed as determined by the RegisterContext subclass. I don't mind them being in RegisterInfo and I think both should stay. In this case, we do need the length for the expression in the RegisterInfo struct, we just don't need to *send* the length in the structured data (XML or JSON).
================
Comment at: source/Host/common/NativeRegisterContext.cpp:273-278
@@ -272,2 +272,8 @@
+size_t
+NativeRegisterContext::GetDwarfOpcodeLength (const uint32_t i) const
+{
+ return 0;
+}
+
uint32_t
----------------
Remove this, the length needs to stay in the RegisterInfo.
================
Comment at: source/Host/common/NativeRegisterContextRegisterInfo.cpp:52-58
@@ +51,8 @@
+
+size_t
+NativeRegisterContextRegisterInfo::GetDwarfOpcodeLength (const uint32_t i) const
+{
+ const RegisterInfoInterface& reg_info_interface = GetRegisterInfoInterface ();
+ return reg_info_interface.GetDwarfOpcodeLength (i);
+}
+
----------------
Remove this, the length needs to stay in the RegisterInfo.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:317
@@ +316,3 @@
+ // Update the actual location
+ reg_info.dynamic_size_dwarf_expr_bytes = m_dynamic_reg_size_map[i].data ();
+ }
----------------
set the length in RegisterInfo from the size of the bytes extracted.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:320
@@ +319,3 @@
+ else
+ reg_info.dynamic_size_dwarf_expr_bytes = NULL;
+ }
----------------
set the opcode length to zero here.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:424-428
@@ -396,2 +423,7 @@
+size_t
+DynamicRegisterInfo::GetDwarfOpcodeLength (uint32_t i)
+{
+ return m_dynamic_reg_size_map[i].size();
+}
----------------
remove.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:435
@@ -403,1 +434,3 @@
+ ConstString &set_name,
+ uint32_t dwarf_expr_bytes_len)
{
----------------
remove dwarf_expr_bytes_len.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.h:43
@@ -43,1 +42,3 @@
+ lldb_private::ConstString &set_name,
+ uint32_t dwarf_expr_bytes_len = 0);
----------------
Remove the dwarf_expr_bytes_len, the length needs to stay in the RegisterInfo. If the opcode and length are non-zero, then a copy of the data will need to be made so it can persist. We already do this with the value_regs and invalidate_regs fields in RegisterInfo.
================
Comment at: source/Plugins/Process/Utility/DynamicRegisterInfo.h:57-59
@@ -55,1 +56,5 @@
+ // Get length of the Dwarf Opcode
+ size_t
+ GetDwarfOpcodeLength (uint32_t i);
+
----------------
remove
================
Comment at: source/Plugins/Process/Utility/RegisterContextLinux_mips.cpp:72-84
@@ -71,2 +71,15 @@
+size_t
+RegisterContextLinux_mips::GetDwarfOpcodeLength (const uint32_t i) const
+{
+ if (i < GetRegisterCount ())
+ {
+ const RegisterInfo *reg_info = GetRegisterInfo() + i;
+
+ if (reg_info && reg_info->dynamic_size_dwarf_expr_bytes)
+ return sizeof(dwarf_opcode_mips);
+ }
+ return 0;
+}
+
uint32_t
----------------
remove
================
Comment at: source/Plugins/Process/Utility/RegisterContextLinux_mips.h:34-35
@@ -33,1 +33,4 @@
+ size_t
+ GetDwarfOpcodeLength (const uint32_t i) const override;
+
----------------
remove
================
Comment at: source/Plugins/Process/Utility/RegisterContextLinux_mips64.cpp:124-136
@@ -123,2 +123,15 @@
+size_t
+RegisterContextLinux_mips64::GetDwarfOpcodeLength (const uint32_t i) const
+{
+ if (i < GetRegisterCount ())
+ {
+ const RegisterInfo *reg_info = GetRegisterInfo() + i;
+
+ if (reg_info && reg_info->dynamic_size_dwarf_expr_bytes)
+ return sizeof(dwarf_opcode_mips64);
+ }
+ return 0;
+}
+
uint32_t
----------------
remove
================
Comment at: source/Plugins/Process/Utility/RegisterContextLinux_mips64.h:36-38
@@ -35,2 +35,5 @@
+ size_t
+ GetDwarfOpcodeLength (const uint32_t i) const override;
+
private:
----------------
remove
================
Comment at: source/Plugins/Process/Utility/RegisterInfoInterface.h:51-56
@@ -50,2 +50,8 @@
+ virtual size_t
+ GetDwarfOpcodeLength (const uint32_t i) const
+ {
+ return 0;
+ }
+
const lldb_private::ArchSpec&
----------------
remove
https://reviews.llvm.org/D20357
More information about the lldb-commits
mailing list