Re: [RFC] Add support for Renesas RX architecture

[Kevin Buettner <kevinb at redhat dot com>]

On Fri, 4 Dec 2009 09:38:37 -0800
Joel Brobecker <brobecker@adacore.com> 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