From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 118389 invoked by alias); 10 May 2017 02:20:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 118164 invoked by uid 89); 10 May 2017 02:20:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 10 May 2017 02:20:20 +0000 Received: by simark.ca (Postfix, from userid 33) id E39B51E4A4; Tue, 9 May 2017 22:20:21 -0400 (EDT) To: Tim Wiederhake Subject: Re: [PATCH v3 05/12] btrace: Use function segment index in insn iterator. X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 10 May 2017 02:20:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org, markus.t.metzger@intel.com In-Reply-To: <1494312929-22749-6-git-send-email-tim.wiederhake@intel.com> References: <1494312929-22749-1-git-send-email-tim.wiederhake@intel.com> <1494312929-22749-6-git-send-email-tim.wiederhake@intel.com> Message-ID: <509c21522fecdf146903a231e97d531c@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00235.txt.bz2 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 > > 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