[lldb-dev] Patch to add two new classes - RegisterContextLinux_x86_64 and RegisterContextFreeBSD_x86_64

[Thirumurthi, Ashok]

> 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: lldb-dev-bounces at cs.uiuc.edu [mailto:lldb-dev-bounces at cs.uiuc.edu] 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:samueldotj at gmail.com] 
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 <ashok.thirumurthi at intel.com> 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
lldb-dev at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: log-regs-4.diff
Type: application/octet-stream
Size: 24055 bytes
Desc: log-regs-4.diff
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20130508/05cf53ec/attachment.obj>