From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31595 invoked by alias); 10 Jun 2005 23:13:10 -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 31572 invoked by uid 22791); 10 Jun 2005 23:13:06 -0000 Received: from sibelius.xs4all.nl (HELO sibelius.xs4all.nl) (82.92.89.47) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Fri, 10 Jun 2005 23:13:06 +0000 Received: from elgar.sibelius.xs4all.nl (root@elgar.sibelius.xs4all.nl [192.168.0.2]) by sibelius.xs4all.nl (8.13.0/8.13.0) with ESMTP id j5ANCtMf008710; Sat, 11 Jun 2005 01:12:55 +0200 (CEST) Received: from elgar.sibelius.xs4all.nl (kettenis@localhost.sibelius.xs4all.nl [127.0.0.1]) by elgar.sibelius.xs4all.nl (8.13.4/8.13.3) with ESMTP id j5ANCsKp004343; Sat, 11 Jun 2005 01:12:54 +0200 (CEST) Received: (from kettenis@localhost) by elgar.sibelius.xs4all.nl (8.13.4/8.13.4/Submit) id j5ANCoWe018703; Sat, 11 Jun 2005 01:12:50 +0200 (CEST) Date: Fri, 10 Jun 2005 23:13:00 -0000 Message-Id: <200506102312.j5ANCoWe018703@elgar.sibelius.xs4all.nl> From: Mark Kettenis To: kevinb@redhat.com CC: gdb-patches@sources.redhat.com In-reply-to: <20050603172038.449a41b9@ironwood.lan> (message from Kevin Buettner on Fri, 3 Jun 2005 17:20:38 -0700) Subject: Re: [RFC] Add support for Morpho ms1 processor References: <20050603172038.449a41b9@ironwood.lan> X-SW-Source: 2005-06/txt/msg00102.txt.bz2 Date: Fri, 3 Jun 2005 17:20:38 -0700 From: Kevin Buettner The patch below adds support for the Morpho Technologies ms1 processor to GDB. This code was written by Michael Snyder. (Though I and perhaps others have tweaked it here and there...) It's not quite ready to commit; there are still some bits at the top level, and in bfd/, opcodes/, and include/ that need to go in first. Aldy Hernandez is submitting those portions. However, it occurs to me that there may be a few things in this GDB portion that ought to be adjusted before it goes in, so I'm posting it now for comments. So... comments? There are a few coding-style violations, mostly regarding multi-line comment style. There is some excessive spacing too; the GNU coding standards seem to suggest using ^L to seperate larger chunks of code, not multiple lines of whitespace. I also really would like to encourage the usage of target-specific (ms1_) prefixes for function and variable names. Especially naming an enum `gdb_regnum' seems like a bad idea to me. What's the E_-prefix trying to convey? I seem to remember a few other embedded processors using the same E_-prefix for their register names. Internationalization of strings is missing too. Can we somehow make sure this doesn't become yet another commit-and-abondon target? A few more comments down in the code. + +/* Function: register_name + Returns the name of the standard ms1 register N. */ Stating the function name in the comment here doesn't make sense. The GNU coding standards certainly don't suggest it. +static struct gdbarch * +ms1_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) +{ + struct gdbarch *gdbarch; + static void ms1_frame_unwind_init (struct gdbarch *); + ... + /* The MAC register for the MS1-16-003 is 8 bits larger + than it is for the other machine variants. */ + switch (info.bfd_arch_info->mach) { + default: + case bfd_mach_ms1: + break; This doesn't do anything... + + set_gdbarch_register_name (gdbarch, ms1_register_name); + set_gdbarch_num_regs (gdbarch, E_NUM_REGS); + set_gdbarch_num_pseudo_regs (gdbarch, E_NUM_PSEUDO_REGS); + set_gdbarch_pc_regnum (gdbarch, E_PC_REGNUM); + set_gdbarch_sp_regnum (gdbarch, E_SP_REGNUM); + set_gdbarch_pseudo_register_read (gdbarch, ms1_pseudo_register_read); + set_gdbarch_pseudo_register_write (gdbarch, ms1_pseudo_register_write); + set_gdbarch_skip_prologue (gdbarch, ms1_skip_prologue); + set_gdbarch_inner_than (gdbarch, core_addr_lessthan); + set_gdbarch_breakpoint_from_pc (gdbarch, ms1_breakpoint_from_pc); + set_gdbarch_decr_pc_after_break (gdbarch, 0); + set_gdbarch_frame_args_skip (gdbarch, 0); + set_gdbarch_print_insn (gdbarch, print_insn_ms1); + set_gdbarch_register_type (gdbarch, ms1_register_type); + set_gdbarch_register_reggroup_p (gdbarch, ms1_register_reggroup_p); Don't try to line things up here; there should only be a single space. + * Target builtin data types. + */ + set_gdbarch_short_bit (gdbarch, 2 * TARGET_CHAR_BIT); + set_gdbarch_int_bit (gdbarch, 4 * TARGET_CHAR_BIT); + set_gdbarch_long_bit (gdbarch, 4 * TARGET_CHAR_BIT); + set_gdbarch_long_long_bit (gdbarch, 8 * TARGET_CHAR_BIT); + set_gdbarch_float_bit (gdbarch, 4 * TARGET_CHAR_BIT); + set_gdbarch_double_bit (gdbarch, 8 * TARGET_CHAR_BIT); + set_gdbarch_long_double_bit (gdbarch, 8 * TARGET_CHAR_BIT); + set_gdbarch_ptr_bit (gdbarch, 4 * TARGET_CHAR_BIT); Personally I think this TARGET_CHAR_BIT is really silly, as if we're going to support something other than 8. I really thing hardcoding the numbers is much clearer. A `short' is 16 bits, not 2 times some arbitrary constant. + +static void +ms1_frame_unwind_init (struct gdbarch *gdbarch) +{ + /* This is the keystone. You add your "sniffer" to a + sniffer list, and now you're in the game. This registers + a) the sniffer, b) the 'prev_register', and c) the 'this_id' + methods. + */ That last */ defenitely shouldn't sit alone on a line. Mark