Re: [patch v6 00/12] branch tracing support for Atom

Jan Kratochvil
2012-12-18T13:55:22Z

On Tue, 18 Dec 2012 11:12:49 +0100, Metzger, Markus T wrote:
> I like the idea of adding BTS as a configuration option to "record" to
> provide a control-flow only trace for the reverse- commands. This will be
> quite limited since the only thing you can really do is iterate over
> disassembly and source lines

Yes. But without teaching users new commands, one of the issues of GDB.
I find myself to prefer the already used reverse-stepi even with BTS.

> - plus some fake back trace.

I would exclude that feature from the current discussion as it can be
implemented as an add-on patch without changing much other code.

> We should not remove the existing btrace commands, though.

In general I agree. Just they should be moved under the "record" prefix.
Command "btrace" then should be renamed to some "record disassemble".

And for example the "btrace" current position should be shared with
"reverse-*" current position and the btrace args parser/syntax should be
unified with "record goto" syntax. That means only supporting start/begin/end
as otherwise btrace syntax is more rich than the record one - but the goal is
to make the syntax compatible for users.

> Using the remote- commands, you would need a sequence of reverse-stepi or
> reverse-step commands. And even then, the output would be clobbered with
> intermixed prompts and would be in reverse control-flow order, unless you
> navigated back and forth again.

I agree. Just "reverse-step" is a more general command - even for record
- while still usable for the basic use case of BTS.

> Depending on your use case, either the btrace or the remote- commands are
> more effective.

Yes.

> As far as I understand, the btrace commands could also be implemented for
> "record" to provide a more compact view of the recorded control-flow. They
> could thus also help improve the "record" feature.

Exactly. That both record navigation commands are useful (or at least usable)
for BTS and that BTS view commands are definitely useful for record proves to
me they should share the same backend/data.

> Given the different use cases, it would make sense to establish two sets of
> commands for all kinds of control-flow tracing/replay: one for viewing and
> one for navigating. In a first step, BTS tracing will only implement the
> former and "record" will only implement the latter. In a second step,
> "record" could implement the viewing commands, and in third step, BTS
> tracing can implement the navigation commands.
>
> Would that be OK with you?

I believe you a heading for an approval of this patchset as is, so that the
unification can be implemented in an add-on patch series.

I do not think the unification patch series will get checked-in soon enough
for gdb-7.6. That means the currently defined commands will need to remain
backward comparible indefinitely in the future. So one should make the
commands syntax final already (there is deprecate_cmd but it is not good to
plan its use).

I believe the internal API will change a lot for the unification with record
backend/data so it seems to me a large part of the patch will need to get
replaced afterwards.

But I am OK with that if we at least make the commands unified with record
with some hope the backend functionality unification will come later.

Thanks,
Jan