> m_register_infos is allocated only once per platform. >> Well, a single instance of RegisterContext will allocate a single copy of >> g_register_infos. However, we have one instance of RegisterContext per >> POSIXThread. This is required to have a register set per thread, but not to >> have static metadata like g_register_infos,
Good point, I lost track of where trunk was. The attached plugin removes the requirement for static methods like GetRegisterIndexFromOffset() in the base class. There is also a minor cleanup of the platform-specific plugins. I removed m_register_infos from the base class so that there is no stale copy of this variable. I also modified the test as I don't see any evidence that "log enable Darwin registers" is supported. In summary: Fixed "log enable linux registers" and added a test. - Eliminated the use of static for methods that read m_register_infos, so that these routines can be implemented in the base class. - Eliminated m_register_infos in the base class because this is not used when derived classes call UpdateRegisterInfo. - Also moved the namespace using declarations from headers to source files. - Ashok -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Thirumurthi, Ashok Sent: Tuesday, May 07, 2013 4:55 PM To: Samuel Jacob Cc: lldb-dev Subject: Re: [lldb-dev] Patch to add two new classes - RegisterContextLinux_x86_64 and RegisterContextFreeBSD_x86_64 > m_register_infos is allocated only once per platform. Well, a single instance of RegisterContext will allocate a single copy of g_register_infos. However, we have one instance of RegisterContext per POSIXThread. This is required to have a register set per thread, but not to have static metadata like g_register_infos, - Ashok -----Original Message----- From: Samuel Jacob [mailto:[email protected]] Sent: Tuesday, May 07, 2013 3:55 PM To: Thirumurthi, Ashok Cc: lldb-dev Subject: Re: [lldb-dev] Patch to add two new classes - RegisterContextLinux_x86_64 and RegisterContextFreeBSD_x86_64 On Tue, May 7, 2013 at 12:36 PM, Thirumurthi, Ashok <[email protected]> wrote: > I see. Note that there are a few downsides to the current implementation. > First, there is a need for about 6Kb of data for m_register_infos that is > currently allocated per thread. Secondly, it's not possible to have static > methods that use m_register_infos, and we have a regression in logging > because of static methods that use the stale m_register_infos like > GetRegisterIndexFromOffset(). Third, the base class is brittle because it > has stale m_register_infos. > Agree with the downsides. Slight correction m_register_infos is allocated only once per platform. > One possibility is for POSIXThread to maintain a new static class instance > that wraps m_register_infos for each platform (let's call this > RegisterLayout). This would allow the plugins to be recoded without static > methods while keeping the leaf classes lightweight. Currently, we couple the > register set with the layout in RegisterContext, whereas we need one register > set per thread and one register layout per platform. Good idea it will make RegisterContext classes neat. Thanks Samuel _______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
log-regs-4.diff
Description: log-regs-4.diff
_______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
