From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9435 invoked by alias); 13 Jul 2004 00:01:37 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 9427 invoked from network); 13 Jul 2004 00:01:36 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 13 Jul 2004 00:01:36 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i6D01ae1007936 for ; Mon, 12 Jul 2004 20:01:36 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i6D01a016952 for ; Mon, 12 Jul 2004 20:01:36 -0400 Received: from localhost.localdomain (vpn50-57.rdu.redhat.com [172.16.50.57]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id i6D01ZFO007284; Mon, 12 Jul 2004 20:01:35 -0400 Received: from saguaro (saguaro.lan [192.168.64.2]) by localhost.localdomain (8.12.11/8.12.10) with SMTP id i6D01T4n022784; Mon, 12 Jul 2004 17:01:29 -0700 Date: Tue, 13 Jul 2004 00:01:00 -0000 From: Kevin Buettner To: Jim Blandy Cc: gdb-patches@sources.redhat.com Subject: Re: RFA: PowerPC: add segment register numbers Message-Id: <20040712170129.47f4bc06@saguaro> In-Reply-To: References: Organization: Red Hat Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SW-Source: 2004-07/txt/msg00137.txt.bz2 On 12 Jul 2004 17:08:11 -0500 Jim Blandy wrote: > * ppc-tdep.h (struct gdbarch_tdep): New member: ppc_sr0_regnum. > * rs6000-tdep.c (rs6000_gdbarch_init): Initialize it. I have a question and a comment... First the question: Regarding this portion of the patch... *************** rs6000_gdbarch_init (struct gdbarch_info *** 2879,2884 **** --- 2879,2885 ---- tdep->ppc_mq_regnum = -1; tdep->ppc_fp0_regnum = 32; tdep->ppc_fpscr_regnum = power ? 71 : 70; + tdep->ppc_sr0_regnum = 71; ...what about the case when ``power'' is non-zero? Do we want ppc_fpscr_regnum and ppc_sr0_regnum to both be equal to 71? Now the comment: Regarding the following portion of the patch... > *************** rs6000_gdbarch_init (struct gdbarch_info > *** 2939,2945 **** > --- 2943,2961 ---- > set_gdbarch_pseudo_register_read (gdbarch, e500_pseudo_register_read); > set_gdbarch_pseudo_register_write (gdbarch, e500_pseudo_register_write); > break; > + > + case bfd_mach_ppc64: > + case bfd_mach_ppc_620: > + case bfd_mach_ppc_630: > + case bfd_mach_ppc_a35: > + case bfd_mach_ppc_rs64ii: > + case bfd_mach_ppc_rs64iii: > + /* These processor's register sets don't have segment registers. */ > + tdep->ppc_sr0_regnum = -1; > + break; > } > + else > + gdb_assert (0); ...I don't really like the gdb_assert (0). With this patch, the code is structured as follows: if (v->arch == bfd_arch_powerpc) switch (v->mach) { ... } else gdb_assert (0); Could you revise your patch so that the above is instead structured like this...? gdb_assert(v->arch == bfd_arch_powerpc); switch (v->mach) { ... } BTW, the reason I don't like the assert (0) is that I like seeing a somewhat meaningful condition when an assert is triggered. Seeing 0 as the failed condition doesn't give much of a clue about what's going on. Kevin