From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13452 invoked by alias); 1 Mar 2012 05:09:54 -0000 Received: (qmail 13441 invoked by uid 22791); 1 Mar 2012 05:09:52 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 01 Mar 2012 05:09:35 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q2159YTK015031 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 1 Mar 2012 00:09:35 -0500 Received: from mesquite.lan (ovpn-113-100.phx2.redhat.com [10.3.113.100]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q2159XVh010155 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO) for ; Thu, 1 Mar 2012 00:09:33 -0500 Date: Thu, 01 Mar 2012 05:09:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Subject: Re: [RFC] sh-tdep.c: Don't fetch FPSCR register if it doesn't exist Message-ID: <20120229220932.17ad0d5f@mesquite.lan> In-Reply-To: <20120301013318.GJ3118@adacore.com> References: <20120229173408.78699013@mesquite.lan> <20120301013318.GJ3118@adacore.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: 2012-03/txt/msg00007.txt.bz2 On Wed, 29 Feb 2012 17:33:18 -0800 Joel Brobecker wrote: > > Comments? Is there a better way to test for the existence of a register? > > Perhaps an idea... Apparently, this is a property of the arch, so > something relatively static. This is typically the type of information > the sh_gdbarch_init routine would save in the gdbarch_tdep structure. > You can create a new flag in that structure, and then set it at the > same time you register the register_name gdbarch hook. I considered that approach. It'll work, but seemed like more coding than it was worth when I first wrote the patch. But thinking about it now, I don't find this to be a very compelling reason to code it the way that I did. That said, I'm not fond of the tdep flag idea, at least not in this instance... If we were to use a tdep flag, we must ensure that this flag is set correctly for each architecture variant. For sh, there are a bunch of `register_name' and `register_type' implementations. It seems like a shame to have to add redundant knowledge about fpscr to the tdep struct. If we were to keep that knowledge together or somehow derive it from knowledge we already had it'd be different. But in order to determine the correct value of this new flag, the most obvious course of action is (for one of us) to examine the register names to see if fpscr is present. We'll have to transcribe that knowledge to the various cases in sh_gdbarch_init(). It's still doable, but I'd like to consider another approach first. I've seen the register name test used in other instances to check for the existence of a register. One of the places it's used is in various `register_reggroup_p' implementations. It's used in both default_register_reggroup_p() and sh_register_reggroup_p(). This suggests the following approach: if (gdbarch_register_reggroup_p (gdbarch, FPSCR_REGNUM, all_reggroup)) ... else ... We know that gdbarch_register_reggroup_p() called in that fashion will somehow determine whether the given register number is a member of the "all" register group. We don't care how it figures this out, whether it be by examining the register name, or a flag in the tdep struct, or some other method altogether. But we do know that it'll return 1 for registers that exist and 0 for those that don't. What do you think? Here's an updated patch: * sh-tdep.c (sh_frame_cache): Don't fetch the FPSCR register unless it exists for this architecture. Index: sh-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/sh-tdep.c,v retrieving revision 1.239 diff -u -p -r1.239 sh-tdep.c --- sh-tdep.c 27 Feb 2012 16:40:48 -0000 1.239 +++ sh-tdep.c 1 Mar 2012 05:02:47 -0000 @@ -2553,7 +2553,12 @@ sh_frame_cache (struct frame_info *this_ if (cache->pc != 0) { ULONGEST fpscr; - fpscr = get_frame_register_unsigned (this_frame, FPSCR_REGNUM); + + /* Check existence of FPSCR. */ + if (gdbarch_register_reggroup_p (gdbarch, FPSCR_REGNUM, all_reggroup)) + fpscr = get_frame_register_unsigned (this_frame, FPSCR_REGNUM); + else + fpscr = 0; sh_analyze_prologue (gdbarch, cache->pc, current_pc, cache, fpscr); }