From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 398 invoked by alias); 28 Mar 2014 17:58:12 -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 388 invoked by uid 89); 28 Mar 2014 17:58:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 28 Mar 2014 17:58:09 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s2SHw0Y2018982 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 28 Mar 2014 13:58:00 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s2SHvvBI008348; Fri, 28 Mar 2014 13:57:58 -0400 Message-ID: <5335B825.1020706@redhat.com> Date: Fri, 28 Mar 2014 17:58:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Joel Brobecker CC: Anton Blanchard , gdb-patches@sourceware.org, emachado@linux.vnet.ibm.com, luis_gustavo@mentor.com, ulrich.weigand@de.ibm.com Subject: Re: [PATCH 2/4] Support up to 3 conditional branches in an atomic sequence References: <1395978111-30706-1-git-send-email-anton@samba.org> <1395978111-30706-2-git-send-email-anton@samba.org> <20140328131230.GE4030@adacore.com> <5335AD94.4030701@redhat.com> <20140328173251.GJ4030@adacore.com> In-Reply-To: <20140328173251.GJ4030@adacore.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-03/txt/msg00669.txt.bz2 On 03/28/2014 05:32 PM, Joel Brobecker wrote: >>> IIUC, it looks like MAX_SINGLE_STEP_BREAKPOINTS is actually not >>> the max, but MAX - 1. I was a little confused by that. Why not >>> change MAX to 3, and then adjust the array definition and code >>> accordingly? I think things will be slightly simpler to understand. >> >> IMO that would be more confusing. I read MAX_SINGLE_STEP_BREAKPOINTS >> as meaning the "maximum number of of single-step breakpoints we >> can create simultaneously". I think you're reading it as >> "the highest index possible into the single-step breakpoints >> array" ? > > Here is how I read the patch: MAX_SINGLE_STEP_BREAKPOINTS is the size > of the array, and we rely on the last element always being NULL > to determine the number of "live" elements we actually have. But we can fill the whole array, there's no sentinel. E.g.: + for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++) + if (single_step_breakpoints[i] == NULL) + break; + + gdb_assert (i < MAX_SINGLE_STEP_BREAKPOINTS); + This just looks for the first empty slot. > Hence, to me, the maximum number of SS breakpoints we can handle > in practice is not MAX_SINGLE_STEP_BREAKPOINTS but 1 less. Nope. We can handle MAX_SINGLE_STEP_BREAKPOINTS. > For > instance, I think the patch is trying to increase the number of > SS breakpoints to 3, and yet defines MAX_SINGLE_STEP_BREAKPOINTS > to 4. Anton is making the port handle 3 conditional branches + 1 terminating branch, so that's 4. I guess it's either the subject that's confusing you, or this, perhaps: > - if (bc_insn_count >= 1) > - return 0; /* More than one conditional branch found, fallback > + if (last_breakpoint >= MAX_SINGLE_STEP_BREAKPOINTS - 1) > + return 0; /* too many conditional branches found, fallback > to the standard single-step code. */ This is "- 1" here because that's leaving space for the terminating branch. At least, that's what I understood. I blame lack of comments in the patch. :-) > Perhaps it's time to just use a vec? That way, we don't have > a limit at all anymore... Yeah... -- Pedro Alves