Re: [patch v9 01/23] thread, btrace: add generic branch trace support

Doug Evans
2013-03-07T22:57:03Z

On Wed, Mar 6, 2013 at 11:49 PM, Metzger, Markus T
<markus.t.metzger@intel.com> wrote:
>> -----Original Message-----
>> From: Doug Evans [mailto:dje@google.com]
>> Sent: Wednesday, March 06, 2013 10:12 PM
>
> Thanks for your review.
>
>
>> > + /* The function index range [begin; end[ that has been covered last time.
>>
>> Typo: end]
>
> That's intended to indicate that begin is included in the range whereas end isn't.
> I see that this has not been clear enough. I'll reword it.

For that use [begin,end).
ref: http://en.wikipedia.org/wiki/Interval_(mathematics)

>> > +/* A branch trace block.
>> > +
>> > + This represents a block of sequential control-flow. Adjacent blocks will be
>> > + connected via calls, returns, or jumps. The latter can be direct or
>> > + indirect, conditional or unconditional. Branches can further be
>> > + asynchronous, e.g. interrupts. */
>> > +struct btrace_block
>> > +{
>> > + /* The address of the first instruction in the block. */
>> > + CORE_ADDR begin;
>> > +
>> > + /* The address of the last instruction in the block. */
>> > + CORE_ADDR end;
>> > +};
>>
>> Can you elaborate in the docs for "end" what it is?
>> E.g., on an ISA with only 4 byte instructions, and the block contains
>> two instructions, is end == begin+4 or begin+7 or begin+8?
>> I'd guess that it's begin+4, but IWBN if the comment
>> removed all doubt.
>
> I changed both comments to say "the first byte of ...".

Thanks.