From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25450 invoked by alias); 7 Dec 2009 20:39:23 -0000 Received: (qmail 25437 invoked by uid 22791); 7 Dec 2009 20:39:20 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS 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; Mon, 07 Dec 2009 20:39:15 +0000 Received: from int-mx04.intmail.prod.int.phx2.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.17]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nB7KdDhs012907 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 7 Dec 2009 15:39:13 -0500 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx04.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id nB7KdC3w000827 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO) for ; Mon, 7 Dec 2009 15:39:13 -0500 Date: Mon, 07 Dec 2009 20:39:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Subject: Re: [RFC] Add support for Renesas RX architecture Message-ID: <20091207133912.3e97099d@redhat.com> In-Reply-To: <20091204173837.GD2891@adacore.com> References: <20091203161906.7f7a6c06@redhat.com> <20091204173837.GD2891@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: 2009-12/txt/msg00097.txt.bz2 On Fri, 4 Dec 2009 09:38:37 -0800 Joel Brobecker wrote: > > I'll note up front that there's one #if 0 in rx-tdep.c. I'd prefer to > > leave it there until Nick and I get all of the kinks worked out of > > the debug info. I'm willing to remove it though if there are objections > > to having it in new code. > > I usually prefer to avoid #if 0 code, but the comment is definitely > useful. In this particular case, I don't think it's so bad, although, > IMO, the same effect could have been achieved without the #if 0, just > the comment (enabling the dwarf2 unwinders is standard, so it's not > like we need the reminder). Anyway, I'm personally OK, since I presume > that this in the TODO list to get fixed as some point in the near/medium > future. In my recent testing, I've been flipping back and forth between #if 0 and #if 1 as I test the code. And, you're right, it is our plan to get this fixed in short order. > > Comments? > > > > * configure.tgt: Add rx-*-elf target. > > * rx-tdep.c: New target. > > Yes. Great code, very clean. Please go ahead and commit. I think this > also deserves a NEWS entry (you can send it as a separate patch for Eli). As I mentioned in my commit message, thanks for the review! I've just posted a patch for NEWS. > Nice to see someone using the prologue-value infrastructure too, btw. It's a very nice bit of infrastructure, making it much easier to write prologue analyzers, IMO. The other cool thing about the prologue analyzer for this architecture is that it shares its instruction decoder with the simulator and various utilities in binutils. The decoder returns symbolic values for the instruction and operand types, making the code a lot cleaner than the usual mask against a hex value with a comparison to another hex value. Here's a brief example from rx-tdep.c: else if (opc.id == RXO_mov /* mov.l rsrc, [-SP] */ && opc.op[0].type == RX_Operand_Predec && opc.op[0].reg == RX_SP_REGNUM && opc.op[1].type == RX_Operand_Register && opc.size == RX_Long) It reads well enough that I could have almost dispensed with the comment - in fact, as I look over the code, it appears that in some cases, I did dispense with the comment. The one gotcha which might make it worth keeping the comments, and would perhaps make it worth it for me to go back and add the missing ones, is that in some cases, the operand order specified by opc.op[N] is not always the same as that implied by the syntax. However, even with this gotcha, this approach is still preferable to the alternative of shifting and masking a portion of the raw instruction stream to determine the operand. Kevin