This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]