[RFA, 1 of 3] save/restore process record, part 1 (exec_entry)

[Michael Snyder]

Joel Brobecker wrote:
>> 2009-10-16  Hui Zhu  <teawater@gmail.com>
>> 	    Michael Snyder  <msnyder@vmware.com>
>>
>> 	* record.c (record_exec_entry): New function.  Emulate one
>> 	instruction, forward or backward.  Abstracted from record_wait.
>> 	(record_wait) Call record_exec_entry.
> 
> I can personnally only comment on details, since I don't know much
> about process record. Not sure who from the Global Maintainers actually
> know much about it except you, Michael :).

I know -- do you think, if I add more comments and documentation,
it will help encourage a few of the other GMs to take an interest?
;-)

In the meantime, Hui and I depend on each other's review.


>> +static inline void
>> +record_exec_entry (struct regcache *regcache, struct gdbarch *gdbarch,
>> +                   struct record_entry *entry)
> 
> We're really pushing for having all functions properly documented.
> Can you add a comment explaining that this function does? The function
> name makes it more or less obvious, I guess, but I'd personally rather
> have a consistent (mindless) approach of documenting everything rather
> than having to judge on a case-by-case basis.

OK, see attached revision.  Thanks for the request.

> Also, I can't help but wonder about the use of "inline" in this case.
> I'm always reluctant to use this sort of feature until I can be proven
> that this helps performance. Since this is a static function, I would
> imagine that the compiler would have more knowledge of whether the
> function should be inlined or not? Or is that too naive?

"Inline", like "register", is just a suggestion to the compiler.
The compiler will do as it pleases, based on other factors.

This function gets called once per instruction in playback mode.
Hence it seemed like it would benefit from inlining.  It's probably
too big to be inlined anyway, but there again, the compiler will
make up its own mind.

I don't know whether we have a policy about using "inline" in
gdb code.  Maybe we should.  I'll be willing to take it out,
but until I hear a positive request, I'll leave it in.  I doubt
if I can measure the performance difference, if any.

Revised diff attached.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: tmp1b.txt
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20091017/e9f95aa8/attachment.txt>