From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10227 invoked by alias); 28 Dec 2010 15:29:28 -0000 Received: (qmail 10216 invoked by uid 22791); 28 Dec 2010 15:29:27 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 28 Dec 2010 15:29:22 +0000 Received: (qmail 23621 invoked from network); 28 Dec 2010 15:29:20 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 28 Dec 2010 15:29:20 -0000 From: Pedro Alves To: Thiago Jung Bauermann Subject: Re: [patch 2/3] Implement support for PowerPC BookE ranged watchpoints Date: Tue, 28 Dec 2010 16:10:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.33-29-realtime; KDE/4.4.5; x86_64; ; ) Cc: gdb-patches@sourceware.org, Jan Kratochvil , Joel Brobecker , Eli Zaretskii References: <1290549100.3164.47.camel@hactar> <201012232017.11120.pedro@codesourcery.com> <1293484743.1544.78.camel@hactar> In-Reply-To: <1293484743.1544.78.camel@hactar> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201012281529.18162.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2010-12/txt/msg00512.txt.bz2 On Monday 27 December 2010 21:19:03, Thiago Jung Bauermann wrote: > @@ -1394,25 +1397,33 @@ update_watchpoint (struct breakpoint *b, int reparse) > if ((b->type == bp_watchpoint || b->type == bp_hardware_watchpoint) > && reparse) > { > - int i, mem_cnt, other_type_used; > - > - /* We need to determine how many resources are already used > - for all other hardware watchpoints to see if we still have > - enough resources to also fit this watchpoint in as well. > - To avoid the hw_watchpoint_used_count call below from counting > - this watchpoint, make sure that it is marked as a software > - watchpoint. */ > - b->type = bp_watchpoint; > - i = hw_watchpoint_used_count (bp_hardware_watchpoint, > - &other_type_used); > - mem_cnt = can_use_hardware_watchpoint (val_chain); > - > - if (!mem_cnt) > + int reg_cnt; > + > + reg_cnt = can_use_hardware_watchpoint (val_chain, b->exact); > + > + if (!reg_cnt) > b->type = bp_watchpoint; > else > { > - int target_resources_ok = target_can_use_hardware_watchpoint > - (bp_hardware_watchpoint, i + mem_cnt, other_type_used); > + int i, other_type_used, target_resources_ok; > + enum bptype orig_type; > + > + if (b->ops && b->ops->extra_resources_needed) > + reg_cnt += b->ops->extra_resources_needed (b, NULL); I'm taking a look at the resource accounting changes, which I mostly ignored before, and I'm confused. can_use_hardware_watchpoint (above) calls target_region_ok_for_hw_watchpoint, which has been changed to return the number of resources normally necessary. Yet, extra_resources_needed_watchpoint calls target_region_ok_for_hw_watchpoint as well (discounting 1, it seems). Aren't we ending up with gdb thinking it needs more resources than are really necessary? > + > + /* We need to determine how many resources are already used > + for all other hardware watchpoints to see if we still have > + enough resources to also fit this watchpoint in as well. > + To avoid the hw_watchpoint_used_count call below from > + counting this watchpoint, make sure that it is marked as a > + software watchpoint. */ > + orig_type = b->type; > + b->type = bp_watchpoint; Something I notice: It sounds like if you remove this hack above, > + i = hw_watchpoint_used_count (bp_hardware_watchpoint, > + &other_type_used); then this above accounts for the new watchpoint too, so... > + > + target_resources_ok = target_can_use_hardware_watchpoint > + (bp_hardware_watchpoint, i + reg_cnt, other_type_used) ... this would just be ", i,", instead of ", i + reg_cnt,". > if (target_resources_ok <= 0) > b->type = bp_watchpoint; > else > > What does the "TYPE_NFIELDS (t) == 1" do ? > > TYPE_NFIELDS (t) == 1 && TYPE_CODE (TYPE_INDEX_TYPE (t)) == TYPE_CODE_RANGE > is a way to determine that the given array type has known bounds. Didn't know about that. I thought TYPE_NFIELDS only made sense for structs/classes/unions. My grep foo isn't finding other similar uses. I tried: compunit1.c: extern int array[]; void foo () { ... } compunit2.c: int array[]; and in foo, ptype array gave me array[0], TYPE_NFIELDS==1. > > Should be "Use of exact watchpoints is ...", I think? Thus getting rid > > of the small lie that not all watchpoints (in the user perspective) will > > use just one debug register, leaving the just one debug register > > issue explanation to the help string (as you already have). > > Good point. Changed to: (...) Thanks. -- Pedro Alves