This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 05/12] btrace: Use function segment index in insn iterator.
On 2017-05-09 02:55, Tim Wiederhake wrote:
Remove FUNCTION pointer in struct btrace_insn_iterator and use an index
into
the list of function segments instead.
2017-05-09 Tim Wiederhake <tim.wiederhake@intel.com>
gdb/ChangeLog:
* btrace.c: (btrace_insn_get, btrace_insn_get_error,
btrace_insn_number,
btrace_insn_begin, btrace_insn_end, btrace_insn_next,
btrace_insn_prev,
btrace_find_insn_by_number): Replace function segment pointer with
index.
(btrace_insn_cmp): Simplify.
* btrace.h: (struct btrace_insn_iterator) Rename index to
insn_index. Replace function segment pointer with index into function
segment vector.
* record-btrace.c (record_btrace_call_history): Replace function
segment pointer use with index.
(record_btrace_frame_sniffer): Retrieve function call segment through
vector.
(record_btrace_set_replay): Remove defunc't safety check.
Looks good, just a few comments below.
@@ -2468,12 +2474,21 @@ int
btrace_insn_cmp (const struct btrace_insn_iterator *lhs,
const struct btrace_insn_iterator *rhs)
{
- unsigned int lnum, rnum;
+ gdb_assert (lhs->btinfo == rhs->btinfo);
- lnum = btrace_insn_number (lhs);
- rnum = btrace_insn_number (rhs);
+ if (lhs->call_index > rhs->call_index)
+ return 1;
+
+ if (lhs->call_index < rhs->call_index)
+ return -1;
+
+ if (lhs->insn_index > rhs->insn_index)
+ return 1;
+
+ if (lhs->insn_index < rhs->insn_index)
+ return -1;
- return (int) (lnum - rnum);
+ return 0;
}
I the number of comparisons could be reduced by doing:
int
btrace_insn_cmp (const struct btrace_insn_iterator *lhs,
const struct btrace_insn_iterator *rhs)
{
gdb_assert (lhs->btinfo == rhs->btinfo);
if (lhs->call_index != rhs->call_index)
return lhs->call_index - rhs->call_index;
return lhs->insn_index - rhs->insn_index;
}
/* See btrace.h. */
@@ -2522,8 +2537,8 @@ btrace_find_insn_by_number (struct
btrace_insn_iterator *it,
}
it->btinfo = btinfo;
- it->function = bfun;
- it->index = number - bfun->insn_offset;
+ it->call_index = bfun->number - 1;
+ it->insn_index = number - bfun->insn_offset;
return 1;
}
diff --git a/gdb/btrace.h b/gdb/btrace.h
index ef2c781..ca79667 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -195,12 +195,11 @@ struct btrace_insn_iterator
/* The branch trace information for this thread. Will never be
NULL. */
const struct btrace_thread_info *btinfo;
- /* The branch trace function segment containing the instruction.
- Will never be NULL. */
- const struct btrace_function *function;
Just an idea, you could factor out those
it->btinfo->functions[it->call_index]
in a small helper method in btrace_insn_iterator:
btrace_function *function ()
{
return this->btinfo->functions[this->call_index];
}
@@ -2691,7 +2691,7 @@ record_btrace_set_replay (struct thread_info *tp,
btinfo = &tp->btrace;
- if (it == NULL || it->function == NULL)
+ if (it == NULL)
Not sure, but wouldn't the equivalent check be that call_index <
btinfo->functions.size () ?
Thanks,
Simon