Re: [RFA][3/5] New port: Cell BE SPU (the port itself)

["Mark Kettenis" <mark dot kettenis at xs4all dot nl>]

>  Hello,
>
>  this is the bulk of the new SPU port.  It adds a configuration for
>  spu*-*-* as target, as well as a 'pseudo-native' configuration hosted
>  on powerpc that allows debugging SPU code natively on a Cell BE system.
>
>  OK?

Hi Ulrich,

A couple of points:

First, "spu" doesn't occur anywhere in config.guess.  Is this a name the
community agrees on?  I understand it stands for Synergistic Processor Unit,
and it seems a bad idea to me that's a fairly generic term.

>  	* config/spu/spu-cell.mt: New file.
>  	* config/spu/spu.mt: New file.

Two .mt files?  I think spu-cell.mt should be renamed spu.mh.

>  	* spu-nat.c: New file.

This file seems to be Linux-spefic.  Can you rename it to spu-linux-nat.c?

>  +/* Some older glibc versions do not define this.  */
>  +#ifndef __WNOTHREAD
>  +#define __WNOTHREAD     0x20000000      /* Don't wait on children of
>  other
>  +				           threads in this group */
>  +#endif

Is this really needed?  I mean, sometimes stuff like this gets added when
we're working on getting anew platform running, but things get fixed
before there's an "offical" release.

> +      sprintf (mess, "reading PPC register #%d", regno);

Can you use xsnprintf() wherever you use sprintf?  The later is a very
dangerous function, and really should not be used in new code.

>  +    char buf[8];

Lots of places use char where you should use gdb_byte.

>  diff -urN gdb-orig/gdb/spu-tdep.h gdb-head/gdb/spu-tdep.h
>  --- gdb-orig/gdb/spu-tdep.h	1970-01-01 01:00:00.000000000 +0100
>  +++ gdb-head/gdb/spu-tdep.h	2006-11-10 02:10:48.572865680 +0100
>  @@ -0,0 +1,44 @@
>  +/* SPU target-dependent code for GDB, the GNU debugger.
>  +   Copyright (C) 2006 Free Software Foundation, Inc.
>  +
>  +   This file is part of GDB.
>  +
>  +   This program is free software; you can redistribute it and/or modify
>  +   it under the terms of the GNU General Public License as published by
>  +   the Free Software Foundation; either version 2 of the License, or
>  +   (at your option) any later version.
>  +
>  +   This program is distributed in the hope that it will be useful,
>  +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>  +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>  +   GNU General Public License for more details.
>  +
>  +   You should have received a copy of the GNU General Public License
>  +   along with this program; if not, write to the Free Software
>  +   Foundation, Inc., 51 Franklin Street, Fifth Floor,
>  +   Boston, MA 02110-1301, USA.  */
>  +
>  +#ifndef SPU_TDEP_H
>  +#define SPU_TDEP_H
>  +
>  +/* Number of registers.  */
>  +#define SPU_NUM_REGS         130
>  +#define SPU_NUM_PSEUDO_REGS  1
>  +#define SPU_NUM_CORE_REGS    128
>  +
>  +/* SPU calling convention.  */
>  +#define SPU_LR_REGNUM        0
>  +#define SPU_RAW_SP_REGNUM    1
>  +#define SPU_ARG1_REGNUM      3
>  +#define SPU_ARGN_REGNUM      79
>  +#define SPU_FP_REGNUM        127
>  +
>  +/* Special registers.  */
>  +#define SPU_ID_REGNUM        128
>  +#define SPU_PC_REGNUM        129
>  +#define SPU_SP_REGNUM        130
>  +
>  +/* Local store.  */
>  +#define SPU_LS_SIZE          0x40000
>  +
>  +#endif

Many ports use an enum for the register numbers, which makes debugging a
bit easier.  I also think SPU_NUM_CORE_REGS is a bad name.  I first thought
this had something to do with core files.  May I suggest SPU_NUM_VEC_REGS.

Mark