[PATCH v3 2/3] gdb: testsuite: Add or1k l.nop inscruction

public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed

* [PATCH v3 2/3] gdb: testsuite: Add or1k l.nop inscruction
  2016-12-22 16:14 [PATCH v3 0/3] OpenRISC gdb port Stafford Horne
  2016-12-22 16:14 ` [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support Stafford Horne
@ 2016-12-22 16:14 ` Stafford Horne
  2016-12-22 16:14 ` [PATCH v3 3/3] Add gdb for or1k build Stafford Horne
  2 siblings, 0 replies; 8+ messages in thread
From: Stafford Horne @ 2016-12-22 16:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: openrisc, Stafford Horne

The test case requires adding a nop instruction.  For or1k the
instruction is `l.nop`. This change uses the correct operation.

gdb/testsuite/ChangeLog:

2016-05-11  Stafford Horne  <shorne@gmail.com>

	* gdb.base/bp-permanent.c: Define nop of or1k.
---
 gdb/testsuite/gdb.base/bp-permanent.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/testsuite/gdb.base/bp-permanent.c b/gdb/testsuite/gdb.base/bp-permanent.c
index 541a89b..7e17dc7 100644
--- a/gdb/testsuite/gdb.base/bp-permanent.c
+++ b/gdb/testsuite/gdb.base/bp-permanent.c
@@ -26,6 +26,8 @@
 
 #if defined(__s390__) || defined(__s390x__)
 #define NOP asm("nopr 0")
+#elif defined(__or1k__)
+#define NOP asm("l.nop")
 #else
 #define NOP asm("nop")
 #endif
-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support
  2016-12-22 16:14 [PATCH v3 0/3] OpenRISC gdb port Stafford Horne
@ 2016-12-22 16:14 ` Stafford Horne
  2016-12-23 21:21   ` Luis Machado
  2016-12-22 16:14 ` [PATCH v3 2/3] gdb: testsuite: Add or1k l.nop inscruction Stafford Horne
  2016-12-22 16:14 ` [PATCH v3 3/3] Add gdb for or1k build Stafford Horne
  2 siblings, 1 reply; 8+ messages in thread
From: Stafford Horne @ 2016-12-22 16:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: openrisc, Franck Jullien

From: Franck Jullien <franck.jullien@gmail.com>

This patch prepates the current GDB port of the openrisc processor from
https://github.com/openrisc/binutils-gdb for upstream merging.

Testing has been done with a cgen sim provided in a separate patch. This
has been tested with 2 toolchains. GCC [1] 5.4.0 from the openrisc
project with Newlib [2] and GCC 5.4.0 with Musl [3] 1.1.4.

It supports or1knd (no delay slot target).
The default target is or1k (with delay slot).

You can change the target arch with:

(gdb) set architecture or1knd
The target architecture is assumed to be or1knd

[1] https://github.com/openrisc/or1k-gcc
[2] https://github.com/openrisc/newlib
[3] https://github.com/openrisc/musl-cross

gdb/doc/ChangeLog:

2016-03-13  Stefan Wallentowitz  <stefan@wallentowitz.de>
	    Franck Jullien  <franck.jullien@gmail.com>
	    Jeremy Bennett  <jeremy.bennett@embecosm.com>

	* gdb.texinfo: Add OpenRISC documentation.

gdb/ChangeLog:

2016-11-23  Stafford Horne  <shorne@gmail.com>
	    Stefan Wallentowitz  <stefan@wallentowitz.de>
	    Stefan Kristiansson  <stefan.kristiansson@saunalahti.fi>
	    Franck Jullien  <franck.jullien@gmail.com>
	    Jeremy Bennett  <jeremy.bennett@embecosm.com>

	* configure.tgt: Add targets for or1k and or1knd.
	* or1k-tdep.c: New.
	* or1k-tdep.h: New.
---
 gdb/configure.tgt   |   12 +
 gdb/doc/gdb.texinfo |   68 +++
 gdb/or1k-tdep.c     | 1436 +++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/or1k-tdep.h     |   57 ++
 4 files changed, 1573 insertions(+)
 create mode 100644 gdb/or1k-tdep.c
 create mode 100644 gdb/or1k-tdep.h

diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 3f2603d..40764c6 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -421,6 +421,18 @@ nios2*-*-*)
 	gdb_target_obs="nios2-tdep.o"
 	;;
 
+or1k-*-*)
+	# Target: OpenCores OpenRISC 1000 32-bit implementation bare metal
+	gdb_target_obs="or1k-tdep.o"
+	gdb_sim=../sim/or1k/libsim.a
+	;;
+
+or1knd-*-*)
+	# Target: OpenCores OpenRISC 1000 32-bit implementation bare metal, without delay slot
+	gdb_target_obs="or1k-tdep.o"
+	gdb_sim=../sim/or1k/libsim.a
+	;;
+
 powerpc*-*-freebsd*)
 	# Target: FreeBSD/powerpc
 	gdb_target_obs="rs6000-tdep.o ppc-sysv-tdep.o ppc64-tdep.o \
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a0de7d1..7ae33d1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -541,6 +541,10 @@ Steve Tjiang, John Newlin, and Scott Foehner.
 Michael Eager and staff of Xilinx, Inc., contributed support for the
 Xilinx MicroBlaze architecture.
 
+The original port to the OpenRISC 1000 is believed to be due to
+Alessandro Forin and Per Bothner. More recent ports have been the work
+of Jeremy Bennett.
+
 @node Sample Session
 @chapter A Sample @value{GDBN} Session
 
@@ -22022,6 +22026,7 @@ acceptable commands.
 * M68K::                        Motorola M68K
 * MicroBlaze::			Xilinx MicroBlaze
 * MIPS Embedded::               MIPS Embedded
+* OpenRISC 1000::               OpenRISC 1000 (or1k)
 * PowerPC Embedded::            PowerPC Embedded
 * AVR::                         Atmel AVR
 * CRIS::                        CRIS
@@ -22228,6 +22233,69 @@ As usual, you can inquire about the @code{mipsfpu} variable with
 @samp{show mipsfpu}.
 @end table
 
+@node OpenRISC 1000
+@subsection OpenRISC 1000
+@cindex OpenRISC 1000
+
+Previous development versions of @value{GDBN} supported remote connection
+via a proprietary JTAG protocol using the @samp{target jtag} command.
+Support for this has now been dropped.
+
+Also, previous verions had support for a @samp{spr} command to access
+OpenRISC's numberous special purpose registers.  These are now available
+via the normal @samp{info registers} command.
+
+@table @code
+
+@kindex target remote
+@item target remote
+
+This is now the only way to connect to a remote OpenRISC 1000
+target.  This is supported by @dfn{Or1ksim}, the OpenRISC 1000
+architectural simulator, Verilatorm and Icarus Verilog
+simulations.  @dfn{Remote serial protocol} servers are also available to
+drive various hardware implementations via JTAG.
+Connects to remote JTAG server.
+
+Example: @code{target remote :51000}
+
+@kindex target sim
+@item target sim
+
+Runs the builtin CPU simulator which can run very basic
+programs, does not support most hardware functions like MMU.
+However for more complex use, the user is advised to run and external
+target, and connect using @samp{target remote}
+
+Example: @code{target sim}
+
+@end table
+
+There are some known problems with the current implementation
+@cindex OpenRISC 1000 known problems
+
+@enumerate
+
+@item
+@cindex OpenRISC 1000 known problems, hardware breakpoints and watchpoints
+Some OpenRISC 1000 targets support hardware breakpoints and watchpoints.
+Consult the target documentation for details.  @value{GDBN} is not
+perfect in handling of watchpoints.  It is possible to allocate hardware
+watchpoints and not discover until running that sufficient watchpoints
+are not available.  It is also possible that GDB will report watchpoints
+being hit spuriously.  This can be down to the assembly code having
+additional memory accesses that are not obviously reflected in the
+source code.
+
+@item
+@cindex OpenRISC 1000 known problems, architectural compatability
+The OpenRISC 1000 architecture has evolved since the first port of @value{GDBN}. In particular the structure of the Unit Present register has
+changed and the CPU Configuration register has been added.  The port of
+@value{GDBN} version @value{GDBVN} uses the @emph{current}
+specification of the OpenRISC 1000.
+
+@end enumerate
+
 @node PowerPC Embedded
 @subsection PowerPC Embedded
 
diff --git a/gdb/or1k-tdep.c b/gdb/or1k-tdep.c
new file mode 100644
index 0000000..d906066
--- /dev/null
+++ b/gdb/or1k-tdep.c
@@ -0,0 +1,1436 @@
+/* Target-dependent code for the 32-bit OpenRISC 1000, for the GDB.
+   Copyright (C) 2008-2016, Free Software Foundation, Inc.
+   Contributed by Alessandro Forin(af@cs.cmu.edu at CMU
+   and by Per Bothner(bothner@cs.wisc.edu) at U.Wisconsin.
+   Contributed by Jeremy Bennett <jeremy.bennett@embecosm.com>
+   Contributed by Franck Jullien <franck.jullien@gmail.com> on behalf of 
+   Embecosm Limited
+   Contributed by Stafford Horne <shorne@gmail.com>
+
+   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 3 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, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "frame.h"
+#include "inferior.h"
+#include "symtab.h"
+#include "value.h"
+#include "gdbcmd.h"
+#include "language.h"
+#include "gdbcore.h"
+#include "symfile.h"
+#include "objfiles.h"
+#include "gdbtypes.h"
+#include "target.h"
+#include "regcache.h"
+#include "safe-ctype.h"
+#include "block.h"
+#include "reggroups.h"
+#include "arch-utils.h"
+#include "frame.h"
+#include "frame-unwind.h"
+#include "frame-base.h"
+#include "dwarf2-frame.h"
+#include "trad-frame.h"
+#include "regset.h"
+#include "remote.h"
+#include "target-descriptions.h"
+
+#include <inttypes.h>
+
+#include "dis-asm.h"
+
+/* OpenRISC specific includes.  */
+#include "or1k-tdep.h"
+\f
+
+/* The target-dependant structure for gdbarch.  */
+
+struct gdbarch_tdep
+{
+  int bytes_per_word;
+  int bytes_per_address;
+  CGEN_CPU_DESC gdb_cgen_cpu_desc;
+};
+
+/* Support functions for the architecture definition.  */
+
+/* Get an instruction from memory.  */
+
+static ULONGEST
+or1k_fetch_instruction (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  gdb_byte buf[OR1K_INSTLEN];
+
+  if (target_read_memory (addr, buf, OR1K_INSTLEN))
+    {
+      memory_error (TARGET_XFER_E_IO, addr);
+    }
+
+  return extract_unsigned_integer (buf, OR1K_INSTLEN, byte_order);
+
+}
+
+
+/* Generic function to read bits from an instruction.  */
+
+static int
+or1k_analyse_inst (uint32_t inst, const char *format, ...)
+{
+  /* Break out each field in turn, validating as we go.  */
+  va_list ap;
+
+  int i;
+  int iptr = 0;			/* Instruction pointer */
+
+  va_start (ap, format);
+
+  for (i = 0; 0 != format[i];)
+    {
+      const char *start_ptr;
+      char *end_ptr;
+
+      uint32_t bits;		/* Bit substring of interest */
+      uint32_t width;		/* Substring width */
+      uint32_t *arg_ptr;
+
+      switch (format[i])
+	{
+	case ' ':
+	  i++;
+	  break;		/* Formatting: ignored */
+
+	case '0':
+	case '1':		/* Constant bit field */
+	  bits = (inst >> (OR1K_INSTBITLEN - iptr - 1)) & 0x1;
+
+	  if ((format[i] - '0') != bits)
+	    {
+	      return 0;
+	    }
+
+	  iptr++;
+	  i++;
+	  break;
+
+	case '%':		/* Bit field */
+	  i++;
+	  start_ptr = &(format[i]);
+	  width = strtoul (start_ptr, &end_ptr, 10);
+
+	  /* Check we got something, and if so skip on */
+	  if (start_ptr == end_ptr)
+	    {
+	      throw_quit
+		("bitstring \"%s\" at offset %d has no length field.\n",
+		 format, i);
+	    }
+
+	  i += end_ptr - start_ptr;
+
+	  /* Look for and skip the terminating 'b'. If it's not there, we
+	     still give a fatal error, because these are fixed strings that
+	     just should not be wrong.  */
+	  if ('b' != format[i++])
+	    {
+	      throw_quit
+		("bitstring \"%s\" at offset %d has no terminating 'b'.\n",
+		 format, i);
+	    }
+
+	  /* Break out the field. There is a special case with a bit width of
+	     32.  */
+	  if (32 == width)
+	    {
+	      bits = inst;
+	    }
+	  else
+	    {
+	      bits =
+		(inst >> (OR1K_INSTBITLEN - iptr - width)) & ((1 << width) -
+							      1);
+	    }
+
+	  arg_ptr = va_arg (ap, uint32_t *);
+	  *arg_ptr = bits;
+	  iptr += width;
+	  break;
+
+	default:
+	  throw_quit ("invalid character in bitstring \"%s\" at offset %d.\n",
+		      format, i);
+	  break;
+	}
+    }
+
+  /* Is the length OK? */
+  gdb_assert (OR1K_INSTBITLEN == iptr);
+
+  return 1; /* We succeeded */
+
+}
+
+
+/* Analyse a l.addi instruction in form: l.addi  rD,rA,I. This is used
+   to parse add instructions during various prologue analysis routines.  */
+
+static int
+or1k_analyse_l_addi (uint32_t inst,
+		     unsigned int *rd_ptr,
+		     unsigned int *ra_ptr, int *simm_ptr)
+{
+  /* Instruction fields */
+  uint32_t rd, ra, i;
+
+  if (or1k_analyse_inst (inst, "10 0111 %5b %5b %16b", &rd, &ra, &i))
+    {
+      /* Found it. Construct the result fields.  */
+      *rd_ptr = (unsigned int) rd;
+      *ra_ptr = (unsigned int) ra;
+      *simm_ptr = (int) (((i & 0x8000) == 0x8000) ? 0xffff0000 | i : i);
+
+      return 1;		/* Success */
+    }
+  else
+    {
+      return 0;		/* Failure */
+    }
+}
+
+
+/* Analyse a l.sw instruction in form: l.sw  I(rA),rB. This is used to
+   to parse store instructions during various prologue analysis routines.  */
+
+static int
+or1k_analyse_l_sw (uint32_t inst,
+		   int *simm_ptr, unsigned int *ra_ptr, unsigned int *rb_ptr)
+{
+  /* Instruction fields */
+  uint32_t ihi, ilo, ra, rb;
+
+  if (or1k_analyse_inst (inst, "11 0101 %5b %5b %5b %11b", &ihi, &ra, &rb,
+			 &ilo))
+
+    {
+      /* Found it. Construct the result fields.  */
+      *simm_ptr = (int) ((ihi << 11) | ilo);
+      *simm_ptr |= ((ihi & 0x10) == 0x10) ? 0xffff0000 : 0;
+
+      *ra_ptr = (unsigned int) ra;
+      *rb_ptr = (unsigned int) rb;
+
+      return 1;		/* Success */
+    }
+  else
+    {
+      return 0;		/* Failure */
+    }
+}
+\f
+
+/* Functions defining the architecture.  */
+
+/* Implement the return_value gdbarch method.  */
+
+static enum return_value_convention
+or1k_return_value (struct gdbarch *gdbarch, struct value *functype,
+		   struct type *valtype, struct regcache *regcache,
+		   gdb_byte *readbuf, const gdb_byte *writebuf)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  enum type_code rv_type = TYPE_CODE (valtype);
+  unsigned int rv_size = TYPE_LENGTH (valtype);
+  unsigned int bpw = (gdbarch_tdep (gdbarch))->bytes_per_word;
+
+  /* Deal with struct/union as addresses. If an array won't fit in a single
+     register it is returned as address. Anything larger than 2 registers needs
+     to also be passed as address (matches gcc default_return_in_memory).  */
+  if ((TYPE_CODE_STRUCT == rv_type) || (TYPE_CODE_UNION == rv_type)
+      || ((TYPE_CODE_ARRAY == rv_type) && (rv_size > bpw))
+      || (rv_size > 2 * bpw))
+    {
+      if (readbuf != NULL)
+	{
+	  ULONGEST tmp;
+
+	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp);
+	  read_memory (tmp, readbuf, rv_size);
+	}
+      if (writebuf != NULL)
+	{
+	  ULONGEST tmp;
+
+	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp);
+	  write_memory (tmp, writebuf, rv_size);
+	}
+
+      return RETURN_VALUE_ABI_RETURNS_ADDRESS;
+    }
+
+  if (rv_size <= bpw)
+    {
+      /* Up to one word scalars are returned in R11.  */
+      if (readbuf != NULL)
+	{
+	  ULONGEST tmp;
+
+	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp);
+	  store_unsigned_integer (readbuf, rv_size, byte_order, tmp);
+
+	}
+      if (writebuf != NULL)
+	{
+	  gdb_byte buf[4];
+	  memset (buf, 0, sizeof (buf)); /* Zero pad if < bpw bytes */
+
+	  if (BFD_ENDIAN_BIG == byte_order)
+	    {
+	      memcpy (buf + sizeof (buf) - rv_size, writebuf, rv_size);
+	    }
+	  else
+	    {
+	      memcpy (buf, writebuf, rv_size);
+	    }
+
+	  regcache_cooked_write (regcache, OR1K_RV_REGNUM, buf);
+	}
+    }
+  else
+    {
+      /* 2 word scalars are returned in r11/r12 (with the MS word in r11).  */
+      if (readbuf != NULL)
+	{
+	  ULONGEST tmp_lo;
+	  ULONGEST tmp_hi;
+	  ULONGEST tmp;
+
+	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp_hi);
+	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM + 1,
+					 &tmp_lo);
+	  tmp = (tmp_hi << (bpw * 8)) | tmp_lo;
+
+	  store_unsigned_integer (readbuf, rv_size, byte_order, tmp);
+	}
+      if (writebuf != NULL)
+	{
+	  gdb_byte buf_lo[4];
+	  gdb_byte buf_hi[4];
+
+	  memset (buf_lo, 0, sizeof (buf_lo));	/* Zero pad if < bpw bytes */
+	  memset (buf_hi, 0, sizeof (buf_hi));	/* Zero pad if < bpw bytes */
+
+	  /* This is cheating. We assume that we fit in 2 words exactly, which
+	     wouldn't work if we had (say) a 6-byte scalar type on a big
+	     endian architecture (with the OpenRISC 1000 usually is).  */
+	  memcpy (buf_hi, writebuf, rv_size - bpw);
+	  memcpy (buf_lo, writebuf + bpw, bpw);
+
+	  regcache_cooked_write (regcache, OR1K_RV_REGNUM, buf_hi);
+	  regcache_cooked_write (regcache, OR1K_RV_REGNUM + 1, buf_lo);
+	}
+    }
+
+  return RETURN_VALUE_REGISTER_CONVENTION;
+
+}
+
+/* OR1K always uses a l.trap instruction for breakpoints.  */
+
+constexpr gdb_byte or1k_break_insn[] = {0x21, 0x00, 0x00, 0x01};
+
+typedef BP_MANIPULATION (or1k_break_insn) or1k_breakpoint;
+
+/* Implement the single_step_through_delay gdbarch method  */
+
+static int
+or1k_single_step_through_delay (struct gdbarch *gdbarch,
+				struct frame_info *this_frame)
+{
+  ULONGEST val;
+  CORE_ADDR ppc;
+  CORE_ADDR npc;
+  CGEN_FIELDS tmp_fields;
+  const CGEN_INSN *insns;
+  struct regcache *regcache = get_current_regcache ();
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  /* Get and the previous and current instruction addresses. If they are not
+     adjacent, we cannot be in a delay slot. */
+  regcache_cooked_read_unsigned (regcache, OR1K_PPC_REGNUM, &val);
+  ppc = (CORE_ADDR) val;
+  regcache_cooked_read_unsigned (regcache, OR1K_NPC_REGNUM, &val);
+  npc = (CORE_ADDR) val;
+
+  if (0x4 != (npc - ppc))
+    {
+      return 0;
+    }
+
+  insns = cgen_lookup_insn (tdep->gdb_cgen_cpu_desc,
+			    NULL,
+			    or1k_fetch_instruction (gdbarch, ppc),
+			    NULL, 32, &tmp_fields, 0);
+
+  /* TODO: we should add a delay slot flag to the CGEN_INSN and remove
+   * this hard coded test. */
+  return ((CGEN_INSN_NUM (insns) == OR1K_INSN_L_J)
+	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_JAL)
+	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_JR)
+	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_JALR)
+	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_BNF)
+	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_BF));
+
+}
+
+/* Name for or1k general registers.  */
+
+static const char *const or1k_reg_names[OR1K_NUM_REGS] = {
+  /* general purpose registers */
+  "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+  "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+  "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
+  "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
+
+  /* previous program counter, next program counter and status register */
+  "ppc", "npc", "sr"
+};
+
+/* Implement the register_name gdbarch method.  */
+
+static const char *
+or1k_register_name (struct gdbarch *gdbarch, int regnum)
+{
+  if (0 <= regnum && regnum < OR1K_NUM_REGS)
+    {
+      return or1k_reg_names[regnum];
+    }
+  else
+      return NULL;
+}
+
+/* Implement the register_type gdbarch method.  */
+
+static struct type *
+or1k_register_type (struct gdbarch *gdbarch, int regnum)
+{
+  if ((regnum >= 0) && (regnum < OR1K_NUM_REGS))
+    {
+      switch (regnum)
+	{
+	case OR1K_PPC_REGNUM:
+	case OR1K_NPC_REGNUM:
+	  return builtin_type (gdbarch)->builtin_func_ptr;	/* Pointer to code */
+
+	case OR1K_SP_REGNUM:
+	case OR1K_FP_REGNUM:
+	  return builtin_type (gdbarch)->builtin_data_ptr;	/* Pointer to data */
+
+	default:
+	  return builtin_type (gdbarch)->builtin_uint32;	/* Data */
+	}
+    }
+
+  internal_error (__FILE__, __LINE__,
+		  _("or1k_register_type: illegal register number %d"),
+		  regnum);
+
+}
+
+/* Implement the register_reggroup_p gdbarch method.  */
+
+static int
+or1k_register_reggroup_p (struct gdbarch *gdbarch,
+			  int regnum, struct reggroup *group)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  /* All register group */
+  if (group == all_reggroup)
+    {
+      return ((regnum >= 0)
+	      && (regnum < OR1K_NUM_REGS)
+	      && (or1k_register_name (gdbarch, regnum)[0] != '\0'));
+    }
+
+  /* For now everything except the PC */
+  if (group == general_reggroup)
+    {
+      return ((regnum >= OR1K_ZERO_REGNUM)
+	      && (regnum < OR1K_MAX_GPR_REGS)
+	      && (regnum != OR1K_PPC_REGNUM) && (regnum != OR1K_NPC_REGNUM));
+    }
+
+  if (group == float_reggroup)
+    {
+      return 0;	/* No float regs.  */
+    }
+
+  if (group == vector_reggroup)
+    {
+      return 0;	/* No vector regs.  */
+    }
+
+  /* For any that are not handled above.  */
+  return default_register_reggroup_p (gdbarch, regnum, group);
+
+}
+
+
+static int
+or1k_is_arg_reg (unsigned int regnum)
+{
+  return (OR1K_FIRST_ARG_REGNUM <= regnum)
+    && (regnum <= OR1K_LAST_ARG_REGNUM);
+
+}
+
+
+static int
+or1k_is_callee_saved_reg (unsigned int regnum)
+{
+  return (OR1K_FIRST_SAVED_REGNUM <= regnum) && (0 == regnum % 2);
+
+}
+
+
+/* Implement the skip_prologue gdbarch method.  */
+
+static CORE_ADDR
+or1k_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  CORE_ADDR start_pc;
+  CORE_ADDR addr;
+  uint32_t inst;
+
+  unsigned int ra, rb, rd; /* for instruction analysis */
+  int simm;
+
+  int frame_size = 0;
+
+  /* Try using SAL first if we have symbolic information available. This only
+     works for DWARF 2, not STABS.  */
+
+  if (find_pc_partial_function (pc, NULL, &start_pc, NULL))
+    {
+      CORE_ADDR prologue_end = skip_prologue_using_sal (gdbarch, pc);
+
+      if (0 != prologue_end)
+	{
+	  struct symtab_and_line prologue_sal = find_pc_line (start_pc, 0);
+	  struct compunit_symtab *compunit
+	    = SYMTAB_COMPUNIT (prologue_sal.symtab);
+	  const char *debug_format = COMPUNIT_DEBUGFORMAT (compunit);
+
+	  if ((NULL != debug_format)
+	      && (strlen ("dwarf") <= strlen (debug_format))
+	      && (0 == strncasecmp ("dwarf", debug_format, strlen ("dwarf"))))
+	    {
+	      return (prologue_end > pc) ? prologue_end : pc;
+	    }
+	}
+    }
+
+  /* Look to see if we can find any of the standard prologue sequence. All
+     quite difficult, since any or all of it may be missing. So this is just a
+     best guess!  */
+
+  addr = pc; /* Where we have got to */
+  inst = or1k_fetch_instruction (gdbarch, addr);
+
+  /* Look for the new stack pointer being set up.  */
+  if (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
+      && (OR1K_SP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
+      && (simm < 0) && (0 == (simm % 4)))
+    {
+      frame_size = -simm;
+      addr += OR1K_INSTLEN;
+      inst = or1k_fetch_instruction (gdbarch, addr);
+    }
+
+  /* Look for the frame pointer being manipulated.  */
+  if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
+      && (OR1K_SP_REGNUM == ra) && (OR1K_FP_REGNUM == rb)
+      && (simm >= 0) && (0 == (simm % 4)))
+    {
+      addr += OR1K_INSTLEN;
+      inst = or1k_fetch_instruction (gdbarch, addr);
+
+      gdb_assert (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
+		  && (OR1K_FP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
+		  && (simm == frame_size));
+
+      addr += OR1K_INSTLEN;
+      inst = or1k_fetch_instruction (gdbarch, addr);
+    }
+
+  /* Look for the link register being saved.  */
+  if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
+      && (OR1K_SP_REGNUM == ra) && (OR1K_LR_REGNUM == rb)
+      && (simm >= 0) && (0 == (simm % 4)))
+    {
+      addr += OR1K_INSTLEN;
+      inst = or1k_fetch_instruction (gdbarch, addr);
+    }
+
+  /* Look for arguments or callee-saved register being saved. The register
+     must be one of the arguments (r3-r8) or the 10 callee saved registers
+     (r10, r12, r14, r16, r18, r20, r22, r24, r26, r28, r30). The base
+     register must be the FP (for the args) or the SP (for the callee_saved
+     registers).  */
+  while (1)
+    {
+      if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
+	  && (((OR1K_FP_REGNUM == ra) && or1k_is_arg_reg (rb))
+	      || ((OR1K_SP_REGNUM == ra) && or1k_is_callee_saved_reg (rb)))
+	  && (0 == (simm % 4)))
+	{
+	  addr += OR1K_INSTLEN;
+	  inst = or1k_fetch_instruction (gdbarch, addr);
+	}
+      else
+	{
+	  /* Nothing else to look for. We have found the end of the prologue. */
+	  return addr;
+	}
+    }
+}
+
+
+/* Implement the frame_align gdbarch method.  */
+
+static CORE_ADDR
+or1k_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
+{
+  return align_down (sp, OR1K_STACK_ALIGN);
+
+}
+
+/* Implement the unwind_pc gdbarch method.  */
+
+static CORE_ADDR
+or1k_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
+{
+  CORE_ADDR pc;
+
+  if (frame_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_pc, next_frame=%d\n",
+			  frame_relative_level (next_frame));
+    }
+
+  pc = frame_unwind_register_unsigned (next_frame, OR1K_NPC_REGNUM);
+
+  if (frame_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_pc, pc=0x%p\n",
+			  (void *) pc);
+    }
+
+  return pc;
+
+}
+
+/* Implement the unwind_sp gdbarch method.  */
+
+static CORE_ADDR
+or1k_unwind_sp (struct gdbarch *gdbarch, struct frame_info *next_frame)
+{
+  CORE_ADDR sp;
+
+  if (frame_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_sp, next_frame=%d\n",
+			  frame_relative_level (next_frame));
+    }
+
+  sp = frame_unwind_register_unsigned (next_frame, OR1K_SP_REGNUM);
+
+  if (frame_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_sp, sp=0x%p\n",
+			  (void *) sp);
+    }
+
+  return sp;
+
+}
+
+/* Implement the push_dummy_code gdbarch method.  */
+
+static CORE_ADDR
+or1k_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
+		      CORE_ADDR function, struct value **args, int nargs,
+		      struct type *value_type, CORE_ADDR * real_pc,
+		      CORE_ADDR * bp_addr, struct regcache *regcache)
+{
+  CORE_ADDR bp_slot;
+
+  /* Reserve enough room on the stack for our breakpoint instruction.  */
+  bp_slot = sp - 4;
+  /* Store the address of that breakpoint.  */
+  *bp_addr = bp_slot;
+  /* keeping the stack aligned.  */
+  sp = or1k_frame_align (gdbarch, bp_slot);
+  /* The call starts at the callee's entry point.  */
+  *real_pc = function;
+
+  return sp;
+
+}
+
+/* Implement the push_dummy_call gdbarch method.  */
+
+static CORE_ADDR
+or1k_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
+		      struct regcache *regcache, CORE_ADDR bp_addr,
+		      int nargs, struct value **args, CORE_ADDR sp,
+		      int struct_return, CORE_ADDR struct_addr)
+{
+
+  int argreg;
+  int argnum;
+  int first_stack_arg;
+  int stack_offset = 0;
+  int heap_offset = 0;
+  CORE_ADDR heap_sp = sp - 128;
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  unsigned int bpa = (gdbarch_tdep (gdbarch))->bytes_per_address;
+  unsigned int bpw = (gdbarch_tdep (gdbarch))->bytes_per_word;
+  struct type *func_type = value_type (function);
+
+  /* Return address */
+  regcache_cooked_write_unsigned (regcache, OR1K_LR_REGNUM, bp_addr);
+
+  /* Register for the next argument.  */
+  argreg = OR1K_FIRST_ARG_REGNUM;
+
+  /* Location for a returned structure. This is passed as a silent first
+     argument.  */
+  if (struct_return)
+    {
+      regcache_cooked_write_unsigned (regcache, OR1K_FIRST_ARG_REGNUM,
+				      struct_addr);
+      argreg++;
+    }
+
+  /* Put as many args as possible in registers */
+  for (argnum = 0; argnum < nargs; argnum++)
+    {
+      const gdb_byte *val;
+      gdb_byte valbuf[sizeof (ULONGEST)];
+
+      struct value *arg = args[argnum];
+      struct type *arg_type = check_typedef (value_type (arg));
+      int len = arg_type->length;
+      enum type_code typecode = arg_type->main_type->code;
+
+      if (TYPE_VARARGS (func_type) && argnum >= TYPE_NFIELDS (func_type))
+	{
+	  break; /* end or regular args, varargs go to stack */
+	}
+
+      /* Extract the value, either a reference or the data */
+      if ((TYPE_CODE_STRUCT == typecode) || (TYPE_CODE_UNION == typecode)
+	  || (len > bpw * 2))
+	{
+	  CORE_ADDR valaddr = value_address (arg);
+
+	  /* if the arg is fabricated (i.e. 3*i, instead of i) valaddr is undefined */
+	  if (valaddr == 0)
+	    {
+	      /* The argument needs to be copied into the target space. Since
+	         the bottom of the stack is reserved for function arguments
+	         we store this at the these at the top growing down.  */
+	      heap_offset += align_up (len, bpw);
+	      valaddr = heap_sp + heap_offset;
+
+	      write_memory (valaddr, value_contents (arg), len);
+	    }
+
+	  /* The ABI passes all structures by reference, so get its address.  */
+	  store_unsigned_integer (valbuf, bpa, byte_order, valaddr);
+	  len = bpa;
+	  val = valbuf;
+	}
+      else
+	{
+	  /* Everything else, we just get the value. */
+	  val = value_contents (arg);
+	}
+
+      /* Stick the value in a register */
+      if (len > bpw)
+	{
+	  /* Big scalars use two registers, but need NOT be pair aligned.  */
+
+	  if (argreg <= (OR1K_LAST_ARG_REGNUM - 1))
+	    {
+	      ULONGEST regval =	extract_unsigned_integer (val, len, byte_order);
+
+	      unsigned int bits_per_word = bpw * 8;
+	      ULONGEST mask = (((ULONGEST) 1) << bits_per_word) - 1;
+	      ULONGEST lo = regval & mask;
+	      ULONGEST hi = regval >> bits_per_word;
+
+	      regcache_cooked_write_unsigned (regcache, argreg, hi);
+	      regcache_cooked_write_unsigned (regcache, argreg + 1, lo);
+	      argreg += 2;
+	    }
+	  else
+	    {
+	      /* Run out of regs */
+	      break;
+	    }
+	}
+      else if (argreg <= OR1K_LAST_ARG_REGNUM)
+	{
+	  /* Smaller scalars fit in a single register */
+	  regcache_cooked_write_unsigned (regcache, argreg,
+					  extract_unsigned_integer (val, len,
+								    byte_order));
+	  argreg++;
+	}
+      else
+	{
+	  /* Run out of regs */
+	  break;
+	}
+    }
+
+  first_stack_arg = argnum;
+
+  /* If we get here with argnum < nargs, then arguments remain to be placed on
+     the stack. This is tricky, since they must be pushed in reverse order and
+     the stack in the end must be aligned. The only solution is to do it in
+     two stages, the first to compute the stack size, the second to save the
+     args.  */
+
+  for (argnum = first_stack_arg; argnum < nargs; argnum++)
+    {
+      struct value *arg = args[argnum];
+      struct type *arg_type = check_typedef (value_type (arg));
+      int len = arg_type->length;
+      enum type_code typecode = arg_type->main_type->code;
+
+      if ((TYPE_CODE_STRUCT == typecode) || (TYPE_CODE_UNION == typecode)
+	  || (len > bpw * 2))
+	{
+	  /* Structures are passed as addresses */
+	  sp -= bpa;
+	}
+      else
+	{
+	  /* Big scalars use more than one word. Code here allows for future
+	     quad-word entities (e.g. long double) */
+	  sp -= align_up (len, bpw);
+	}
+
+      /* ensure our dummy heap doesn't touch the stack, this could only happen
+         if we have many arguments including fabricated arguments */
+      gdb_assert (heap_offset == 0 || ((heap_sp + heap_offset) < sp));
+    }
+
+  sp = gdbarch_frame_align (gdbarch, sp);
+  stack_offset = 0;
+
+  /* Push the remaining args on the stack */
+  for (argnum = first_stack_arg; argnum < nargs; argnum++)
+    {
+      const gdb_byte *val;
+      gdb_byte valbuf[sizeof (ULONGEST)];
+
+      struct value *arg = args[argnum];
+      struct type *arg_type = check_typedef (value_type (arg));
+      int len = arg_type->length;
+      enum type_code typecode = arg_type->main_type->code;
+      /* The EABI passes structures that do not fit in a register by
+         reference. In all other cases, pass the structure by value.  */
+      if ((TYPE_CODE_STRUCT == typecode) || (TYPE_CODE_UNION == typecode)
+	  || (len > bpw * 2))
+	{
+	  store_unsigned_integer (valbuf, bpa, byte_order,
+				  value_address (arg));
+	  len = bpa;
+	  val = valbuf;
+	}
+      else
+	{
+	  val = value_contents (arg);
+	}
+
+      while (len > 0)
+	{
+	  int partial_len = (len < bpw ? len : bpw);
+
+	  write_memory (sp + stack_offset, val, partial_len);
+	  stack_offset += align_up (partial_len, bpw);
+	  len -= partial_len;
+	  val += partial_len;
+	}
+    }
+
+  /* Save the updated stack pointer */
+  regcache_cooked_write_unsigned (regcache, OR1K_SP_REGNUM, sp);
+
+  if (heap_offset > 0)
+    {
+      sp = heap_sp;
+    }
+
+  return sp;
+
+}
+
+
+/* Implement the dummy_id gdbarch method.  */
+
+static struct frame_id
+or1k_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame)
+{
+  return frame_id_build (get_frame_sp (this_frame),
+			 get_frame_pc (this_frame));
+
+}
+\f
+
+
+
+/* Support functions for frame handling */
+
+/* Initialize a prologue cache
+
+   We build a cache, saying where registers of the PREV frame can be found
+   from the data so far set up in this THIS.
+
+   We also compute a unique ID for this frame, based on the function start
+   address and the stack pointer (as it will be, even if it has yet to be
+   computed.
+
+   STACK FORMAT
+   ============
+
+   The OR1K has a falling stack frame and a simple prolog. The Stack pointer
+   is R1 and the frame pointer R2. The frame base is therefore the address
+   held in R2 and the stack pointer (R1) is the frame base of the NEXT frame.
+
+   l.addi  r1,r1,-frame_size	# SP now points to end of new stack frame
+
+   The stack pointer may not be set up in a frameless function (e.g. a simple
+   leaf function).
+
+   l.sw    fp_loc(r1),r2        # old FP saved in new stack frame
+   l.addi  r2,r1,frame_size     # FP now points to base of new stack frame
+
+   The frame pointer is not necessarily saved right at the end of the stack
+   frame - OR1K saves enough space for any args to called functions right at
+   the end (this is a difference from the Architecture Manual).
+
+   l.sw    lr_loc(r1),r9        # Link (return) address
+
+   The link register is usally saved at fp_loc - 4. It may not be saved at all
+   in a leaf function.
+
+   l.sw    reg_loc(r1),ry       # Save any callee saved regs
+
+   The offsets x for the callee saved registers generally (always?) rise in
+   increments of 4, starting at fp_loc + 4. If the frame pointer is omitted
+   (an option to GCC), then it may not be saved at all. There may be no callee
+   saved registers.
+
+   So in summary none of this may be present. However what is present seems
+   always to follow this fixed order, and occur before any substantive code
+   (it is possible for GCC to have more flexible scheduling of the prologue,
+   but this does not seem to occur for OR1K).
+
+   ANALYSIS
+   ========
+
+   This prolog is used, even for -O3 with GCC.
+
+   All this analysis must allow for the possibility that the PC is in the
+   middle of the prologue. Data in the cache should only be set up insofar as
+   it has been computed.
+
+   HOWEVER. The frame_id must be created with the SP *as it will be* at the
+   end of the Prologue. Otherwise a recursive call, checking the frame with
+   the PC at the start address will end up with the same frame_id as the
+   caller.
+
+   A suite of "helper" routines are used, allowing reuse for
+   or1k_skip_prologue().
+
+   Reportedly, this is only valid for frames less than 0x7fff in size.  */
+
+static struct trad_frame_cache *
+or1k_frame_cache (struct frame_info *this_frame, void **prologue_cache)
+{
+  struct gdbarch *gdbarch;
+  struct trad_frame_cache *info;
+
+  CORE_ADDR this_pc;
+  CORE_ADDR this_sp;
+  CORE_ADDR this_sp_for_id;
+  int frame_size = 0;
+
+  CORE_ADDR start_addr;
+  CORE_ADDR end_addr;
+
+  if (frame_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog,
+			  "or1k_frame_cache, prologue_cache = 0x%p\n",
+			  *prologue_cache);
+    }
+
+  /* Nothing to do if we already have this info.  */
+  if (NULL != *prologue_cache)
+    {
+      return (struct trad_frame_cache *) *prologue_cache;
+    }
+
+  /* Get a new prologue cache and populate it with default values.  */
+  info = trad_frame_cache_zalloc (this_frame);
+  *prologue_cache = info;
+
+  /* Find the start address of THIS function (which is a NORMAL frame, even if
+     the NEXT frame is the sentinel frame) and the end of its prologue.  */
+  this_pc = get_frame_pc (this_frame);
+  find_pc_partial_function (this_pc, NULL, &start_addr, NULL);
+
+  /* Get the stack pointer if we have one (if there's no process executing yet
+     we won't have a frame.  */
+  this_sp = (NULL == this_frame) ? 0 :
+    get_frame_register_unsigned (this_frame, OR1K_SP_REGNUM);
+
+  /* Return early if GDB couldn't find the function.  */
+  if (start_addr == 0)
+    {
+      if (frame_debug)
+	{
+	  fprintf_unfiltered (gdb_stdlog, "  couldn't find function\n");
+	}
+
+      /* JPB: 28-Apr-11. This is a temporary patch, to get round GDB crashing
+         right at the beginning. Build the frame ID as best we can. */
+      trad_frame_set_id (info, frame_id_build (this_sp, this_pc));
+
+      return info;
+    }
+
+
+  /* The default frame base of THIS frame (for ID purposes only - frame base
+     is an overloaded term) is its stack pointer. For now we use the value of
+     the SP register in THIS frame. However if the PC is in the prologue of
+     THIS frame, before the SP has been set up, then the value will actually
+     be that of the PREV frame, and we'll need to adjust it later. */
+  trad_frame_set_this_base (info, this_sp);
+  this_sp_for_id = this_sp;
+
+  /* The default is to find the PC of the PREVIOUS frame in the link register
+     of this frame. This may be changed if we find the link register was saved
+     on the stack. */
+  trad_frame_set_reg_realreg (info, OR1K_NPC_REGNUM, OR1K_LR_REGNUM);
+
+  /* We should only examine code that is in the prologue. This is all code up
+     to (but not including) end_addr. We should only populate the cache while
+     the address is up to (but not including) the PC or end_addr, whichever is
+     first. */
+  gdbarch = get_frame_arch (this_frame);
+  end_addr = or1k_skip_prologue (gdbarch, start_addr);
+
+  /* All the following analysis only occurs if we are in the prologue and have
+     executed the code. Check we have a sane prologue size, and if zero we
+     are frameless and can give up here. */
+  if (end_addr < start_addr)
+    {
+      throw_quit ("end addr 0x%08x is less than start addr 0x%08x\n",
+		  (unsigned int) end_addr, (unsigned int) start_addr);
+    }
+
+  if (end_addr == start_addr)
+    {
+      frame_size = 0;
+    }
+  else
+    {
+      /* have a frame. Look for the various components */
+      CORE_ADDR addr = start_addr;	/* Where we have got to */
+      uint32_t inst = or1k_fetch_instruction (gdbarch, addr);
+
+      unsigned int ra, rb, rd;	/* For instruction analysis */
+      int simm;
+
+      /* Look for the new stack pointer being set up. */
+      if (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
+	  && (OR1K_SP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
+	  && (simm < 0) && (0 == (simm % 4)))
+	{
+	  frame_size = -simm;
+	  addr += OR1K_INSTLEN;
+	  inst = or1k_fetch_instruction (gdbarch, addr);
+
+	  /* If the PC has not actually got to this point, then the frame base
+	     will be wrong, and we adjust it.
+
+	     If we are past this point, then we need to populate the stack
+	     accoringly. */
+	  if (this_pc <= addr)
+	    {
+	      /* Only do if executing */
+	      if (0 != this_sp)
+		{
+		  this_sp_for_id = this_sp + frame_size;
+		  trad_frame_set_this_base (info, this_sp_for_id);
+		}
+	    }
+	  else
+	    {
+	      /* We are past this point, so the stack pointer of the PREV
+	         frame is frame_size greater than the stack pointer of THIS
+	         frame. */
+	      trad_frame_set_reg_value (info, OR1K_SP_REGNUM,
+					this_sp + frame_size);
+	    }
+	}
+
+      /* From now on we are only populating the cache, so we stop once we get
+         to either the end OR the current PC. */
+      end_addr = (this_pc < end_addr) ? this_pc : end_addr;
+
+      /* Look for the frame pointer being manipulated. */
+      if ((addr < end_addr)
+	  && or1k_analyse_l_sw (inst, &simm, &ra, &rb)
+	  && (OR1K_SP_REGNUM == ra) && (OR1K_FP_REGNUM == rb)
+	  && (simm >= 0) && (0 == (simm % 4)))
+	{
+	  addr += OR1K_INSTLEN;
+	  inst = or1k_fetch_instruction (gdbarch, addr);
+
+	  /* At this stage, we can find the frame pointer of the PREVIOUS
+	     frame on the stack of the current frame. */
+	  trad_frame_set_reg_addr (info, OR1K_FP_REGNUM, this_sp + simm);
+
+	  /* Look for the new frame pointer being set up */
+	  if (addr < end_addr)
+	    {
+	      gdb_assert (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
+			  && (OR1K_FP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
+			  && (simm == frame_size));
+
+	      addr += OR1K_INSTLEN;
+	      inst = or1k_fetch_instruction (gdbarch, addr);
+
+	      /* If we have got this far, the stack pointer of the PREVIOUS
+	         frame is the frame pointer of THIS frame. */
+	      trad_frame_set_reg_realreg (info, OR1K_SP_REGNUM,
+					  OR1K_FP_REGNUM);
+	    }
+	}
+
+      /* Look for the link register being saved */
+      if ((addr < end_addr)
+	  && or1k_analyse_l_sw (inst, &simm, &ra, &rb)
+	  && (OR1K_SP_REGNUM == ra) && (OR1K_LR_REGNUM == rb)
+	  && (simm >= 0) && (0 == (simm % 4)))
+	{
+	  addr += OR1K_INSTLEN;
+	  inst = or1k_fetch_instruction (gdbarch, addr);
+
+	  /* If the link register is saved in the THIS frame, it holds the
+	     value of the PC in the PREVIOUS frame. This overwrites the
+	     previous information about finding the PC in the link
+	     register.  */
+	  trad_frame_set_reg_addr (info, OR1K_NPC_REGNUM, this_sp + simm);
+	}
+
+      /* Look for arguments or callee-saved register being saved. The register
+         must be one of the arguments (r3-r8) or the 10 callee saved registers
+         (r10, r12, r14, r16, r18, r20, r22, r24, r26, r28, r30). The base
+         register must be the FP (for the args) or the SP (for the
+         callee_saved registers).  */
+      while (addr < end_addr)
+	{
+	  if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
+	      && (((OR1K_FP_REGNUM == ra) && or1k_is_arg_reg (rb))
+		  || ((OR1K_SP_REGNUM == ra)
+		      && or1k_is_callee_saved_reg (rb))) && (0 == (simm % 4)))
+	    {
+	      addr += OR1K_INSTLEN;
+	      inst = or1k_fetch_instruction (gdbarch, addr);
+
+	      /* The register in the PREVIOUS frame can be found at this
+	         location in THIS frame */
+	      trad_frame_set_reg_addr (info, rb, this_sp + simm);
+	    }
+	  else
+	    {
+	      break;		/* Not a register save instruction */
+	    }
+	}
+    }
+
+  /* Build the frame ID */
+  trad_frame_set_id (info, frame_id_build (this_sp_for_id, start_addr));
+
+  if (frame_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog, "  this_sp_for_id = 0x%p\n",
+			  (void *) this_sp_for_id);
+      fprintf_unfiltered (gdb_stdlog, "  start_addr     = 0x%p\n",
+			  (void *) start_addr);
+    }
+
+  return info;
+
+}
+
+
+/* Implement the this_id function for the stub unwinder.  */
+
+static void
+or1k_frame_this_id (struct frame_info *this_frame,
+		    void **prologue_cache, struct frame_id *this_id)
+{
+  struct trad_frame_cache *info = or1k_frame_cache (this_frame,
+						    prologue_cache);
+
+  trad_frame_get_id (info, this_id);
+
+}
+
+
+/* Implement the prev_register function for the stub unwinder.  */
+
+static struct value *
+or1k_frame_prev_register (struct frame_info *this_frame,
+			  void **prologue_cache, int regnum)
+{
+  struct trad_frame_cache *info = or1k_frame_cache (this_frame,
+						    prologue_cache);
+
+  return trad_frame_get_register (info, this_frame, regnum);
+
+}
+
+/* Data structures for the normal prologue-analysis-based
+   unwinder.  */
+ 
+static const struct frame_unwind or1k_frame_unwind = {
+  .type = NORMAL_FRAME,
+  .stop_reason = default_frame_unwind_stop_reason,
+  .this_id = or1k_frame_this_id,
+  .prev_register = or1k_frame_prev_register,
+  .unwind_data = NULL,
+  .sniffer = default_frame_sniffer,
+  .dealloc_cache = NULL,
+  .prev_arch = NULL
+};
+
+/* Architecture initialization for OpenRISC 1000.  */
+
+static struct gdbarch *
+or1k_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
+{
+  static struct frame_base or1k_frame_base;
+  struct gdbarch *gdbarch;
+  struct gdbarch_tdep *tdep;
+  const struct bfd_arch_info *binfo;
+  struct tdesc_arch_data *tdesc_data = NULL;
+
+  int i;
+  int reg_index = 0;
+  int retval;
+  int group;
+
+  /* Find a candidate among the list of pre-declared architectures.  */
+  arches = gdbarch_list_lookup_by_info (arches, &info);
+  if (NULL != arches)
+    {
+      return arches->gdbarch;
+    }
+
+  /* None found, create a new architecture from the information
+     provided. Can't initialize all the target dependencies until we actually
+     know which target we are talking to, but put in some defaults for now. */
+
+  binfo = info.bfd_arch_info;
+  tdep = XNEW (struct gdbarch_tdep);
+  tdep->bytes_per_word = binfo->bits_per_word / binfo->bits_per_byte;
+  tdep->bytes_per_address = binfo->bits_per_address / binfo->bits_per_byte;
+  gdbarch = gdbarch_alloc (&info, tdep);
+
+  /* Target data types.  */
+  set_gdbarch_short_bit (gdbarch, 16);
+  set_gdbarch_int_bit (gdbarch, 32);
+  set_gdbarch_long_bit (gdbarch, 32);
+  set_gdbarch_long_long_bit (gdbarch, 64);
+  set_gdbarch_float_bit (gdbarch, 32);
+  set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
+  set_gdbarch_double_bit (gdbarch, 64);
+  set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
+  set_gdbarch_long_double_bit (gdbarch, 64);
+  set_gdbarch_long_double_format (gdbarch, floatformats_ieee_double);
+  set_gdbarch_ptr_bit (gdbarch, binfo->bits_per_address);
+  set_gdbarch_addr_bit (gdbarch, binfo->bits_per_address);
+  set_gdbarch_char_signed (gdbarch, 1);
+
+  /* Information about the target architecture */
+  set_gdbarch_return_value (gdbarch, or1k_return_value);
+  set_gdbarch_breakpoint_kind_from_pc (gdbarch, or1k_breakpoint::kind_from_pc);
+  set_gdbarch_sw_breakpoint_from_kind (gdbarch, or1k_breakpoint::bp_from_kind);
+  set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
+
+  set_gdbarch_print_insn (gdbarch, print_insn_or1k);
+
+  /* Register architecture */
+  set_gdbarch_num_regs (gdbarch, OR1K_NUM_REGS);
+  set_gdbarch_num_pseudo_regs (gdbarch, OR1K_NUM_PSEUDO_REGS);
+  set_gdbarch_sp_regnum (gdbarch, OR1K_SP_REGNUM);
+  set_gdbarch_pc_regnum (gdbarch, OR1K_NPC_REGNUM);
+  set_gdbarch_ps_regnum (gdbarch, OR1K_SR_REGNUM);
+  set_gdbarch_deprecated_fp_regnum (gdbarch, OR1K_FP_REGNUM);
+
+  /* Functions to supply register information */
+  set_gdbarch_register_name (gdbarch, or1k_register_name);
+  set_gdbarch_register_type (gdbarch, or1k_register_type);
+  set_gdbarch_register_reggroup_p (gdbarch, or1k_register_reggroup_p);
+
+  /* Functions to analyse frames */
+  set_gdbarch_skip_prologue (gdbarch, or1k_skip_prologue);
+  set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
+  set_gdbarch_frame_align (gdbarch, or1k_frame_align);
+  set_gdbarch_frame_red_zone_size (gdbarch, OR1K_FRAME_RED_ZONE_SIZE);
+
+  /* Functions to access frame data */
+  set_gdbarch_unwind_pc (gdbarch, or1k_unwind_pc);
+  set_gdbarch_unwind_sp (gdbarch, or1k_unwind_sp);
+
+  /* Functions handling dummy frames */
+  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
+  set_gdbarch_push_dummy_code (gdbarch, or1k_push_dummy_code);
+  set_gdbarch_push_dummy_call (gdbarch, or1k_push_dummy_call);
+  set_gdbarch_dummy_id (gdbarch, or1k_dummy_id);
+
+  /* Frame unwinders. Use DWARF debug info if available, otherwise use our
+     own unwinder.  */
+  dwarf2_append_unwinders (gdbarch);
+  frame_unwind_append_unwinder (gdbarch, &or1k_frame_unwind);
+
+  /* Get a CGEN CPU descriptor for this architecture.  */
+  {
+
+    const char *mach_name = binfo->printable_name;
+    enum cgen_endian endian = (info.byte_order == BFD_ENDIAN_BIG
+			       ? CGEN_ENDIAN_BIG : CGEN_ENDIAN_LITTLE);
+
+    tdep->gdb_cgen_cpu_desc =
+      or1k_cgen_cpu_open (CGEN_CPU_OPEN_BFDMACH, mach_name,
+			  CGEN_CPU_OPEN_ENDIAN, endian, CGEN_CPU_OPEN_END);
+
+    or1k_cgen_init_asm (tdep->gdb_cgen_cpu_desc);
+  }
+
+  /* If this mach has a delay slot.  */
+  if (binfo->mach == bfd_mach_or1k)
+    {
+      set_gdbarch_single_step_through_delay
+	(gdbarch, or1k_single_step_through_delay);
+    }
+
+  /* Check any target description for validity.  */
+  if (tdesc_has_registers (info.target_desc))
+    { 
+      const struct tdesc_feature *feature;
+      int valid_p;
+
+      feature = tdesc_find_feature (info.target_desc, "org.gnu.gdb.or1k.group0");
+      if (feature == NULL)
+        return NULL;
+
+      tdesc_data = tdesc_data_alloc ();
+
+      valid_p = 1;
+
+      for (i = 0; i < OR1K_NUM_REGS; i++)
+        valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
+                                            or1k_reg_names[i]);
+
+      if (!valid_p)
+        { 
+          tdesc_data_cleanup (tdesc_data);
+          return NULL;
+        }
+    }
+
+  if (tdesc_data)
+    {
+      tdesc_use_registers (gdbarch, info.target_desc, tdesc_data);
+
+      /* Target descriptions may extend into the following groups.  */
+      reggroup_add (gdbarch, general_reggroup);
+      reggroup_add (gdbarch, system_reggroup);
+      reggroup_add (gdbarch, float_reggroup);
+      reggroup_add (gdbarch, vector_reggroup);
+      reggroup_add (gdbarch, reggroup_new ("immu", USER_REGGROUP));
+      reggroup_add (gdbarch, reggroup_new ("dmmu", USER_REGGROUP));
+      reggroup_add (gdbarch, reggroup_new ("icache", USER_REGGROUP));
+      reggroup_add (gdbarch, reggroup_new ("dcache", USER_REGGROUP));
+      reggroup_add (gdbarch, reggroup_new ("pic", USER_REGGROUP));
+      reggroup_add (gdbarch, reggroup_new ("timer", USER_REGGROUP));
+      reggroup_add (gdbarch, reggroup_new ("power", USER_REGGROUP));
+      reggroup_add (gdbarch, reggroup_new ("perf", USER_REGGROUP));
+      reggroup_add (gdbarch, reggroup_new ("mac", USER_REGGROUP));
+      reggroup_add (gdbarch, reggroup_new ("debug", USER_REGGROUP));
+      reggroup_add (gdbarch, all_reggroup);
+      reggroup_add (gdbarch, save_reggroup);
+      reggroup_add (gdbarch, restore_reggroup);
+    }
+
+  return gdbarch;
+
+}
+
+/* Dump the target specific data for this architecture.  */
+
+static void
+or1k_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  if (NULL == tdep)
+    {
+      return;			/* Nothing to report */
+    }
+
+  fprintf_unfiltered (file, "or1k_dump_tdep: %d bytes per word\n",
+		      tdep->bytes_per_word);
+  fprintf_unfiltered (file, "or1k_dump_tdep: %d bytes per address\n",
+		      tdep->bytes_per_address);
+
+}
+\f
+
+extern initialize_file_ftype _initialize_or1k_tdep; /* -Wmissing-prototypes */
+
+void
+_initialize_or1k_tdep (void)
+{
+  /* Register this architecture.  */
+  gdbarch_register (bfd_arch_or1k, or1k_gdbarch_init, or1k_dump_tdep);
+
+  /* Tell remote stub that we support XML target description.  */
+  register_remote_support_xml ("or1k");
+}
diff --git a/gdb/or1k-tdep.h b/gdb/or1k-tdep.h
new file mode 100644
index 0000000..43ae7c3
--- /dev/null
+++ b/gdb/or1k-tdep.h
@@ -0,0 +1,57 @@
+/* Definitions to target GDB to OpenRISC 1000 32-bit targets.
+   Copyright (C) 2008-2016 Free Software Foundation, Inc.
+   Contributed by Jeremy Bennett <jeremy.bennett@embecosm.com>
+
+   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 3 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, see <http://www.gnu.org/licenses/>.  */
+
+
+#ifndef OR1K_TDEP__H
+#define OR1K_TDEP__H
+
+#ifndef TARGET_OR1K
+#define TARGET_OR1K
+#endif
+
+#include "opcodes/or1k-desc.h"
+#include "opcodes/or1k-opc.h"
+
+/* General Purpose Registers */
+#define OR1K_ZERO_REGNUM          0
+#define OR1K_SP_REGNUM            1
+#define OR1K_FP_REGNUM            2
+#define OR1K_FIRST_ARG_REGNUM     3
+#define OR1K_LAST_ARG_REGNUM      8
+#define OR1K_LR_REGNUM            9
+#define OR1K_FIRST_SAVED_REGNUM  10
+#define OR1K_RV_REGNUM           11
+#define OR1K_PPC_REGNUM          (OR1K_MAX_GPR_REGS + 0)
+#define OR1K_NPC_REGNUM          (OR1K_MAX_GPR_REGS + 1)
+#define OR1K_SR_REGNUM           (OR1K_MAX_GPR_REGS + 2)
+
+/* Properties of the architecture. GDB mapping of registers is all the GPRs
+   and SPRs followed by the PPC, NPC and SR at the end. Red zone is the area
+   past the end of the stack reserved for exception handlers etc.  */
+
+#define OR1K_MAX_GPR_REGS            32
+#define OR1K_NUM_PSEUDO_REGS         0
+#define OR1K_NUM_REGS               (OR1K_MAX_GPR_REGS + 3)
+#define OR1K_STACK_ALIGN             4
+#define OR1K_INSTLEN                 4
+#define OR1K_INSTBITLEN             (OR1K_INSTLEN * 8)
+#define OR1K_NUM_TAP_RECORDS         8
+#define OR1K_FRAME_RED_ZONE_SIZE     2536
+
+#endif /* OR1K_TDEP__H */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] Add gdb for or1k build
  2016-12-22 16:14 [PATCH v3 0/3] OpenRISC gdb port Stafford Horne
  2016-12-22 16:14 ` [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support Stafford Horne
  2016-12-22 16:14 ` [PATCH v3 2/3] gdb: testsuite: Add or1k l.nop inscruction Stafford Horne
@ 2016-12-22 16:14 ` Stafford Horne
  2 siblings, 0 replies; 8+ messages in thread
From: Stafford Horne @ 2016-12-22 16:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: openrisc, Stefan Wallentowitz

From: Stefan Wallentowitz <stefan@wallentowitz.de>

* ChangeLog:

	* configure.ac: remove logic adding gdb to noconfigsdirs for or1k
	* configure: regenerate
---
 configure    | 7 -------
 configure.ac | 7 -------
 2 files changed, 14 deletions(-)

diff --git a/configure b/configure
index f6d12b8..d3862d2 100755
--- a/configure
+++ b/configure
@@ -3729,10 +3729,6 @@ case "${target}" in
     ;;
   *-*-rtems*)
     noconfigdirs="$noconfigdirs target-libgloss"
-    # this is not caught below because this stanza matches earlier
-    case $target in
-      or1k*-*-*) noconfigdirs="$noconfigdirs gdb" ;;
-    esac
     ;;
     # The tpf target doesn't support gdb yet.
   *-*-tpf*)
@@ -3940,9 +3936,6 @@ case "${target}" in
   nvptx*-*-*)
     noconfigdirs="$noconfigdirs target-libssp target-libstdc++-v3 target-libobjc"
     ;;
-  or1k*-*-*)
-    noconfigdirs="$noconfigdirs gdb"
-    ;;
   sh-*-* | sh64-*-*)
     case "${target}" in
       sh*-*-elf)
diff --git a/configure.ac b/configure.ac
index 3ec86c1..a3a914d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1065,10 +1065,6 @@ case "${target}" in
     ;;
   *-*-rtems*)
     noconfigdirs="$noconfigdirs target-libgloss"
-    # this is not caught below because this stanza matches earlier
-    case $target in
-      or1k*-*-*) noconfigdirs="$noconfigdirs gdb" ;;
-    esac
     ;;
     # The tpf target doesn't support gdb yet.
   *-*-tpf*)
@@ -1276,9 +1272,6 @@ case "${target}" in
   nvptx*-*-*)
     noconfigdirs="$noconfigdirs target-libssp target-libstdc++-v3 target-libobjc"
     ;;
-  or1k*-*-*)
-    noconfigdirs="$noconfigdirs gdb"
-    ;;
   sh-*-* | sh64-*-*)
     case "${target}" in
       sh*-*-elf)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 0/3] OpenRISC gdb port
@ 2016-12-22 16:14 Stafford Horne
  2016-12-22 16:14 ` [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support Stafford Horne
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Stafford Horne @ 2016-12-22 16:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: openrisc, Stafford Horne

Hello,

This is the openrisc port of GDB that has been in openrisc
repositories for a very long time. The main original author
was Jeremy Bennett as can be seen in file copyrights. Other
are willing to do what it takes for FSF copyright assignment.

This along with the sim patches are the last of what has not
made its way upstream. The patches are based on current gdb
git master branch.

I have run the testsuite and results are as follows:

# of expected passes            19241
# of unexpected failures        406
# of expected failures          29
# of known failures             57
# of unresolved testcases       2
# of untested testcases         190
# of unsupported tests          264

--
Changes from v2
 * Updates as per Yao's comments
  - Fix copyrights, jullien embecosm, remove embecosm
  - indentation to use gnu style (i.e. && and || on left)
  - Fix comments to use gnu style
  - Move some includes around
  - Remove constant variables, replace with constants
  - Remove doxygen on a few calls
  - Have only 1 space between type and var name
  - remove todo about 4 const
  - do fix *buf checks (use buf != NULL)
  - BP_MANIPULATION
  - Use the builtin types for reg register functions
  - or1k_iterate_over_regset_sections - remove no linux, no cores yet
 * Remove unimplements pseudo register functions
 * Remove spr commands as we can now get those via `info reg`

Changes from v1
 * Merged or1k-tdep.[ch] changes into a single commit
 * Futher fixes to change log bringing in history back to 2008
 * Fix doc issue on sw_breakpoint_from_kind


Franck Jullien (1):
  gdb: Add OpenRISC or1k and or1knd target support

Stafford Horne (1):
  gdb: testsuite: Add or1k l.nop inscruction

Stefan Wallentowitz (1):
  Add gdb for or1k build

 configure                             |    7 -
 configure.ac                          |    7 -
 gdb/configure.tgt                     |   12 +
 gdb/doc/gdb.texinfo                   |   68 ++
 gdb/or1k-tdep.c                       | 1436 +++++++++++++++++++++++++++++++++
 gdb/or1k-tdep.h                       |   57 ++
 gdb/testsuite/gdb.base/bp-permanent.c |    2 +
 7 files changed, 1575 insertions(+), 14 deletions(-)
 create mode 100644 gdb/or1k-tdep.c
 create mode 100644 gdb/or1k-tdep.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support
  2016-12-22 16:14 ` [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support Stafford Horne
@ 2016-12-23 21:21   ` Luis Machado
  2016-12-25  1:31     ` Stafford Horne
  0 siblings, 1 reply; 8+ messages in thread
From: Luis Machado @ 2016-12-23 21:21 UTC (permalink / raw)
  To: Stafford Horne, gdb-patches; +Cc: openrisc, Franck Jullien

On 12/22/2016 10:14 AM, Stafford Horne wrote:
> From: Franck Jullien <franck.jullien@gmail.com>
>
> This patch prepates the current GDB port of the openrisc processor from
> https://github.com/openrisc/binutils-gdb for upstream merging.
>
> Testing has been done with a cgen sim provided in a separate patch. This
> has been tested with 2 toolchains. GCC [1] 5.4.0 from the openrisc
> project with Newlib [2] and GCC 5.4.0 with Musl [3] 1.1.4.
>
> It supports or1knd (no delay slot target).
> The default target is or1k (with delay slot).
>
> You can change the target arch with:
>
> (gdb) set architecture or1knd
> The target architecture is assumed to be or1knd
>
> [1] https://github.com/openrisc/or1k-gcc
> [2] https://github.com/openrisc/newlib
> [3] https://github.com/openrisc/musl-cross
>
> gdb/doc/ChangeLog:
>
> 2016-03-13  Stefan Wallentowitz  <stefan@wallentowitz.de>
> 	    Franck Jullien  <franck.jullien@gmail.com>
> 	    Jeremy Bennett  <jeremy.bennett@embecosm.com>
>
> 	* gdb.texinfo: Add OpenRISC documentation.
>
> gdb/ChangeLog:
>
> 2016-11-23  Stafford Horne  <shorne@gmail.com>
> 	    Stefan Wallentowitz  <stefan@wallentowitz.de>
> 	    Stefan Kristiansson  <stefan.kristiansson@saunalahti.fi>
> 	    Franck Jullien  <franck.jullien@gmail.com>
> 	    Jeremy Bennett  <jeremy.bennett@embecosm.com>
>
> 	* configure.tgt: Add targets for or1k and or1knd.
> 	* or1k-tdep.c: New.
> 	* or1k-tdep.h: New.
> ---
>  gdb/configure.tgt   |   12 +
>  gdb/doc/gdb.texinfo |   68 +++
>  gdb/or1k-tdep.c     | 1436 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/or1k-tdep.h     |   57 ++
>  4 files changed, 1573 insertions(+)
>  create mode 100644 gdb/or1k-tdep.c
>  create mode 100644 gdb/or1k-tdep.h
>
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index 3f2603d..40764c6 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -421,6 +421,18 @@ nios2*-*-*)
>  	gdb_target_obs="nios2-tdep.o"
>  	;;
>
> +or1k-*-*)
> +	# Target: OpenCores OpenRISC 1000 32-bit implementation bare metal
> +	gdb_target_obs="or1k-tdep.o"
> +	gdb_sim=../sim/or1k/libsim.a
> +	;;
> +
> +or1knd-*-*)
> +	# Target: OpenCores OpenRISC 1000 32-bit implementation bare metal, without delay slot
> +	gdb_target_obs="or1k-tdep.o"
> +	gdb_sim=../sim/or1k/libsim.a
> +	;;
> +

I noticed both targets use the same files. Is there a reason why we need 
both and not a single entry that can be identified at runtime?

>  powerpc*-*-freebsd*)
>  	# Target: FreeBSD/powerpc
>  	gdb_target_obs="rs6000-tdep.o ppc-sysv-tdep.o ppc64-tdep.o \
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index a0de7d1..7ae33d1 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -541,6 +541,10 @@ Steve Tjiang, John Newlin, and Scott Foehner.
>  Michael Eager and staff of Xilinx, Inc., contributed support for the
>  Xilinx MicroBlaze architecture.
>
> +The original port to the OpenRISC 1000 is believed to be due to
> +Alessandro Forin and Per Bothner. More recent ports have been the work
> +of Jeremy Bennett.
> +
>  @node Sample Session
>  @chapter A Sample @value{GDBN} Session
>
> @@ -22022,6 +22026,7 @@ acceptable commands.
>  * M68K::                        Motorola M68K
>  * MicroBlaze::			Xilinx MicroBlaze
>  * MIPS Embedded::               MIPS Embedded
> +* OpenRISC 1000::               OpenRISC 1000 (or1k)
>  * PowerPC Embedded::            PowerPC Embedded
>  * AVR::                         Atmel AVR
>  * CRIS::                        CRIS
> @@ -22228,6 +22233,69 @@ As usual, you can inquire about the @code{mipsfpu} variable with
>  @samp{show mipsfpu}.
>  @end table
>
> +@node OpenRISC 1000
> +@subsection OpenRISC 1000
> +@cindex OpenRISC 1000
> +
> +Previous development versions of @value{GDBN} supported remote connection
> +via a proprietary JTAG protocol using the @samp{target jtag} command.
> +Support for this has now been dropped.
> +
> +Also, previous verions had support for a @samp{spr} command to access
> +OpenRISC's numberous special purpose registers.  These are now available
> +via the normal @samp{info registers} command.
> +
> +@table @code
> +
> +@kindex target remote
> +@item target remote
> +
> +This is now the only way to connect to a remote OpenRISC 1000
> +target.  This is supported by @dfn{Or1ksim}, the OpenRISC 1000
> +architectural simulator, Verilatorm and Icarus Verilog
> +simulations.  @dfn{Remote serial protocol} servers are also available to

Verilatorm and Icarus simulations or simulators?

> +drive various hardware implementations via JTAG.
> +Connects to remote JTAG server.
> +
> +Example: @code{target remote :51000}
> +
> +@kindex target sim
> +@item target sim
> +
> +Runs the builtin CPU simulator which can run very basic
> +programs, does not support most hardware functions like MMU.

"...programs but does not support..."?

> +However for more complex use, the user is advised to run and external

"... more complex use cases..."? Or maybe "... more complex uses..."?

> +target, and connect using @samp{target remote}
> +
> +Example: @code{target sim}
> +
> +@end table
> +
> +There are some known problems with the current implementation
> +@cindex OpenRISC 1000 known problems
> +
> +@enumerate
> +
> +@item
> +@cindex OpenRISC 1000 known problems, hardware breakpoints and watchpoints
> +Some OpenRISC 1000 targets support hardware breakpoints and watchpoints.
> +Consult the target documentation for details.  @value{GDBN} is not
> +perfect in handling of watchpoints.  It is possible to allocate hardware
> +watchpoints and not discover until running that sufficient watchpoints
> +are not available.  It is also possible that GDB will report watchpoints
> +being hit spuriously.  This can be down to the assembly code having
> +additional memory accesses that are not obviously reflected in the
> +source code.
> +
> +@item
> +@cindex OpenRISC 1000 known problems, architectural compatability

typo, compatibility.

> +The OpenRISC 1000 architecture has evolved since the first port of @value{GDBN}. In particular the structure of the Unit Present register has

"...first port FOR ..."

> +changed and the CPU Configuration register has been added.  The port of
> +@value{GDBN} version @value{GDBVN} uses the @emph{current}
> +specification of the OpenRISC 1000.
> +
> +@end enumerate
> +
>  @node PowerPC Embedded
>  @subsection PowerPC Embedded
>
> diff --git a/gdb/or1k-tdep.c b/gdb/or1k-tdep.c
> new file mode 100644
> index 0000000..d906066
> --- /dev/null
> +++ b/gdb/or1k-tdep.c
> @@ -0,0 +1,1436 @@
> +/* Target-dependent code for the 32-bit OpenRISC 1000, for the GDB.
> +   Copyright (C) 2008-2016, Free Software Foundation, Inc.
> +   Contributed by Alessandro Forin(af@cs.cmu.edu at CMU
> +   and by Per Bothner(bothner@cs.wisc.edu) at U.Wisconsin.
> +   Contributed by Jeremy Bennett <jeremy.bennett@embecosm.com>
> +   Contributed by Franck Jullien <franck.jullien@gmail.com> on behalf of
> +   Embecosm Limited
> +   Contributed by Stafford Horne <shorne@gmail.com>

I've seen recent reviews that point at not mentioning these 
contributions in the source code but rather in the documentation. So i 
think these should be moved somewhere else (maybe they have already been 
moved and you just forgot to remove the above?)

> +
> +   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 3 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, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "frame.h"
> +#include "inferior.h"
> +#include "symtab.h"
> +#include "value.h"
> +#include "gdbcmd.h"
> +#include "language.h"
> +#include "gdbcore.h"
> +#include "symfile.h"
> +#include "objfiles.h"
> +#include "gdbtypes.h"
> +#include "target.h"
> +#include "regcache.h"
> +#include "safe-ctype.h"
> +#include "block.h"
> +#include "reggroups.h"
> +#include "arch-utils.h"
> +#include "frame.h"
> +#include "frame-unwind.h"
> +#include "frame-base.h"
> +#include "dwarf2-frame.h"
> +#include "trad-frame.h"
> +#include "regset.h"
> +#include "remote.h"
> +#include "target-descriptions.h"
> +
> +#include <inttypes.h>
> +
> +#include "dis-asm.h"

Push these two includes up and together with the rest.

> +
> +/* OpenRISC specific includes.  */
> +#include "or1k-tdep.h"
> +\f
> +
> +/* The target-dependant structure for gdbarch.  */
> +
> +struct gdbarch_tdep
> +{
> +  int bytes_per_word;
> +  int bytes_per_address;
> +  CGEN_CPU_DESC gdb_cgen_cpu_desc;
> +};
> +
> +/* Support functions for the architecture definition.  */
> +
> +/* Get an instruction from memory.  */
> +
> +static ULONGEST
> +or1k_fetch_instruction (struct gdbarch *gdbarch, CORE_ADDR addr)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  gdb_byte buf[OR1K_INSTLEN];
> +
> +  if (target_read_memory (addr, buf, OR1K_INSTLEN))
> +    {
> +      memory_error (TARGET_XFER_E_IO, addr);
> +    }
> +

No need for curly braces for single-statement conditional blocks.

> +  return extract_unsigned_integer (buf, OR1K_INSTLEN, byte_order);
> +
> +}
> +
> +

spurious newline.

> +/* Generic function to read bits from an instruction.  */
> +
> +static int
> +or1k_analyse_inst (uint32_t inst, const char *format, ...)
> +{
> +  /* Break out each field in turn, validating as we go.  */
> +  va_list ap;
> +
> +  int i;
> +  int iptr = 0;			/* Instruction pointer */
> +
> +  va_start (ap, format);
> +
> +  for (i = 0; 0 != format[i];)
> +    {
> +      const char *start_ptr;
> +      char *end_ptr;
> +
> +      uint32_t bits;		/* Bit substring of interest */
> +      uint32_t width;		/* Substring width */
> +      uint32_t *arg_ptr;
> +
> +      switch (format[i])
> +	{
> +	case ' ':
> +	  i++;
> +	  break;		/* Formatting: ignored */
> +
> +	case '0':
> +	case '1':		/* Constant bit field */
> +	  bits = (inst >> (OR1K_INSTBITLEN - iptr - 1)) & 0x1;
> +
> +	  if ((format[i] - '0') != bits)
> +	    {
> +	      return 0;
> +	    }

No need for curly braces. Possibly more occurrences of this.

Also, the identation seems a bit off. In special for some of the comments.

> +
> +	  iptr++;
> +	  i++;
> +	  break;
> +
> +	case '%':		/* Bit field */
> +	  i++;
> +	  start_ptr = &(format[i]);
> +	  width = strtoul (start_ptr, &end_ptr, 10);
> +
> +	  /* Check we got something, and if so skip on */

period and two spaces after end of comment. More occurrences of this 
throughout.

> +	  if (start_ptr == end_ptr)
> +	    {
> +	      throw_quit
> +		("bitstring \"%s\" at offset %d has no length field.\n",
> +		 format, i);
> +	    }
> +
> +	  i += end_ptr - start_ptr;
> +
> +	  /* Look for and skip the terminating 'b'. If it's not there, we
> +	     still give a fatal error, because these are fixed strings that
> +	     just should not be wrong.  */

Two spaces after period. More occurrences throughout.


> +	  if ('b' != format[i++])
> +	    {
> +	      throw_quit
> +		("bitstring \"%s\" at offset %d has no terminating 'b'.\n",
> +		 format, i);
> +	    }
> +
> +	  /* Break out the field. There is a special case with a bit width of
> +	     32.  */
> +	  if (32 == width)
> +	    {
> +	      bits = inst;
> +	    }
> +	  else
> +	    {
> +	      bits =
> +		(inst >> (OR1K_INSTBITLEN - iptr - width)) & ((1 << width) -
> +							      1);
> +	    }
> +
> +	  arg_ptr = va_arg (ap, uint32_t *);
> +	  *arg_ptr = bits;
> +	  iptr += width;
> +	  break;
> +
> +	default:
> +	  throw_quit ("invalid character in bitstring \"%s\" at offset %d.\n",
> +		      format, i);
> +	  break;
> +	}
> +    }
> +
> +  /* Is the length OK? */
> +  gdb_assert (OR1K_INSTBITLEN == iptr);
> +
> +  return 1; /* We succeeded */
> +

Spurious newline. Possibly more occurrences throughout

> +}
> +
> +
> +/* Analyse a l.addi instruction in form: l.addi  rD,rA,I. This is used
> +   to parse add instructions during various prologue analysis routines.  */
> +
> +static int
> +or1k_analyse_l_addi (uint32_t inst,
> +		     unsigned int *rd_ptr,
> +		     unsigned int *ra_ptr, int *simm_ptr)
> +{
> +  /* Instruction fields */
> +  uint32_t rd, ra, i;
> +
> +  if (or1k_analyse_inst (inst, "10 0111 %5b %5b %16b", &rd, &ra, &i))
> +    {
> +      /* Found it. Construct the result fields.  */
> +      *rd_ptr = (unsigned int) rd;
> +      *ra_ptr = (unsigned int) ra;

Do we need these explicit cast or can we use types that make the casts 
go away?

> +      *simm_ptr = (int) (((i & 0x8000) == 0x8000) ? 0xffff0000 | i : i);
> +
> +      return 1;		/* Success */
> +    }
> +  else
> +    {
> +      return 0;		/* Failure */
> +    }
> +}
> +
> +
> +/* Analyse a l.sw instruction in form: l.sw  I(rA),rB. This is used to
> +   to parse store instructions during various prologue analysis routines.  */
> +
> +static int
> +or1k_analyse_l_sw (uint32_t inst,
> +		   int *simm_ptr, unsigned int *ra_ptr, unsigned int *rb_ptr)
> +{
> +  /* Instruction fields */
> +  uint32_t ihi, ilo, ra, rb;
> +
> +  if (or1k_analyse_inst (inst, "11 0101 %5b %5b %5b %11b", &ihi, &ra, &rb,
> +			 &ilo))
> +
> +    {
> +      /* Found it. Construct the result fields.  */
> +      *simm_ptr = (int) ((ihi << 11) | ilo);
> +      *simm_ptr |= ((ihi & 0x10) == 0x10) ? 0xffff0000 : 0;
> +
> +      *ra_ptr = (unsigned int) ra;
> +      *rb_ptr = (unsigned int) rb;
> +
> +      return 1;		/* Success */
> +    }
> +  else
> +    {
> +      return 0;		/* Failure */
> +    }
> +}
> +\f
> +
> +/* Functions defining the architecture.  */
> +
> +/* Implement the return_value gdbarch method.  */
> +
> +static enum return_value_convention
> +or1k_return_value (struct gdbarch *gdbarch, struct value *functype,
> +		   struct type *valtype, struct regcache *regcache,
> +		   gdb_byte *readbuf, const gdb_byte *writebuf)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  enum type_code rv_type = TYPE_CODE (valtype);
> +  unsigned int rv_size = TYPE_LENGTH (valtype);
> +  unsigned int bpw = (gdbarch_tdep (gdbarch))->bytes_per_word;
> +
> +  /* Deal with struct/union as addresses. If an array won't fit in a single
> +     register it is returned as address. Anything larger than 2 registers needs
> +     to also be passed as address (matches gcc default_return_in_memory).  */
> +  if ((TYPE_CODE_STRUCT == rv_type) || (TYPE_CODE_UNION == rv_type)
> +      || ((TYPE_CODE_ARRAY == rv_type) && (rv_size > bpw))
> +      || (rv_size > 2 * bpw))
> +    {
> +      if (readbuf != NULL)
> +	{
> +	  ULONGEST tmp;
> +
> +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp);
> +	  read_memory (tmp, readbuf, rv_size);
> +	}
> +      if (writebuf != NULL)
> +	{
> +	  ULONGEST tmp;
> +
> +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp);
> +	  write_memory (tmp, writebuf, rv_size);
> +	}
> +
> +      return RETURN_VALUE_ABI_RETURNS_ADDRESS;
> +    }
> +
> +  if (rv_size <= bpw)
> +    {
> +      /* Up to one word scalars are returned in R11.  */
> +      if (readbuf != NULL)
> +	{
> +	  ULONGEST tmp;
> +
> +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp);
> +	  store_unsigned_integer (readbuf, rv_size, byte_order, tmp);
> +
> +	}
> +      if (writebuf != NULL)
> +	{
> +	  gdb_byte buf[4];
> +	  memset (buf, 0, sizeof (buf)); /* Zero pad if < bpw bytes */
> +
> +	  if (BFD_ENDIAN_BIG == byte_order)
> +	    {
> +	      memcpy (buf + sizeof (buf) - rv_size, writebuf, rv_size);
> +	    }
> +	  else
> +	    {
> +	      memcpy (buf, writebuf, rv_size);
> +	    }
> +
> +	  regcache_cooked_write (regcache, OR1K_RV_REGNUM, buf);
> +	}
> +    }
> +  else
> +    {
> +      /* 2 word scalars are returned in r11/r12 (with the MS word in r11).  */
> +      if (readbuf != NULL)
> +	{
> +	  ULONGEST tmp_lo;
> +	  ULONGEST tmp_hi;
> +	  ULONGEST tmp;
> +
> +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp_hi);
> +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM + 1,
> +					 &tmp_lo);
> +	  tmp = (tmp_hi << (bpw * 8)) | tmp_lo;
> +
> +	  store_unsigned_integer (readbuf, rv_size, byte_order, tmp);
> +	}
> +      if (writebuf != NULL)
> +	{
> +	  gdb_byte buf_lo[4];
> +	  gdb_byte buf_hi[4];
> +
> +	  memset (buf_lo, 0, sizeof (buf_lo));	/* Zero pad if < bpw bytes */
> +	  memset (buf_hi, 0, sizeof (buf_hi));	/* Zero pad if < bpw bytes */
> +
> +	  /* This is cheating. We assume that we fit in 2 words exactly, which
> +	     wouldn't work if we had (say) a 6-byte scalar type on a big
> +	     endian architecture (with the OpenRISC 1000 usually is).  */
> +	  memcpy (buf_hi, writebuf, rv_size - bpw);
> +	  memcpy (buf_lo, writebuf + bpw, bpw);
> +
> +	  regcache_cooked_write (regcache, OR1K_RV_REGNUM, buf_hi);
> +	  regcache_cooked_write (regcache, OR1K_RV_REGNUM + 1, buf_lo);
> +	}
> +    }
> +
> +  return RETURN_VALUE_REGISTER_CONVENTION;
> +
> +}
> +
> +/* OR1K always uses a l.trap instruction for breakpoints.  */
> +
> +constexpr gdb_byte or1k_break_insn[] = {0x21, 0x00, 0x00, 0x01};
> +
> +typedef BP_MANIPULATION (or1k_break_insn) or1k_breakpoint;
> +
> +/* Implement the single_step_through_delay gdbarch method  */
> +
> +static int
> +or1k_single_step_through_delay (struct gdbarch *gdbarch,
> +				struct frame_info *this_frame)
> +{
> +  ULONGEST val;
> +  CORE_ADDR ppc;
> +  CORE_ADDR npc;
> +  CGEN_FIELDS tmp_fields;
> +  const CGEN_INSN *insns;
> +  struct regcache *regcache = get_current_regcache ();
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> +  /* Get and the previous and current instruction addresses. If they are not
> +     adjacent, we cannot be in a delay slot. */
> +  regcache_cooked_read_unsigned (regcache, OR1K_PPC_REGNUM, &val);
> +  ppc = (CORE_ADDR) val;
> +  regcache_cooked_read_unsigned (regcache, OR1K_NPC_REGNUM, &val);
> +  npc = (CORE_ADDR) val;
> +
> +  if (0x4 != (npc - ppc))
> +    {
> +      return 0;
> +    }
> +
> +  insns = cgen_lookup_insn (tdep->gdb_cgen_cpu_desc,
> +			    NULL,
> +			    or1k_fetch_instruction (gdbarch, ppc),
> +			    NULL, 32, &tmp_fields, 0);
> +
> +  /* TODO: we should add a delay slot flag to the CGEN_INSN and remove
> +   * this hard coded test. */
> +  return ((CGEN_INSN_NUM (insns) == OR1K_INSN_L_J)
> +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_JAL)
> +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_JR)
> +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_JALR)
> +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_BNF)
> +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_BF));
> +
> +}
> +
> +/* Name for or1k general registers.  */
> +
> +static const char *const or1k_reg_names[OR1K_NUM_REGS] = {
> +  /* general purpose registers */
> +  "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
> +  "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
> +  "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
> +  "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
> +
> +  /* previous program counter, next program counter and status register */
> +  "ppc", "npc", "sr"
> +};
> +
> +/* Implement the register_name gdbarch method.  */
> +
> +static const char *
> +or1k_register_name (struct gdbarch *gdbarch, int regnum)
> +{
> +  if (0 <= regnum && regnum < OR1K_NUM_REGS)
> +    {
> +      return or1k_reg_names[regnum];
> +    }
> +  else
> +      return NULL;
> +}
> +
> +/* Implement the register_type gdbarch method.  */
> +
> +static struct type *
> +or1k_register_type (struct gdbarch *gdbarch, int regnum)
> +{
> +  if ((regnum >= 0) && (regnum < OR1K_NUM_REGS))
> +    {
> +      switch (regnum)
> +	{
> +	case OR1K_PPC_REGNUM:
> +	case OR1K_NPC_REGNUM:
> +	  return builtin_type (gdbarch)->builtin_func_ptr;	/* Pointer to code */
> +
> +	case OR1K_SP_REGNUM:
> +	case OR1K_FP_REGNUM:
> +	  return builtin_type (gdbarch)->builtin_data_ptr;	/* Pointer to data */
> +
> +	default:
> +	  return builtin_type (gdbarch)->builtin_uint32;	/* Data */
> +	}
> +    }
> +
> +  internal_error (__FILE__, __LINE__,
> +		  _("or1k_register_type: illegal register number %d"),
> +		  regnum);
> +
> +}
> +
> +/* Implement the register_reggroup_p gdbarch method.  */
> +
> +static int
> +or1k_register_reggroup_p (struct gdbarch *gdbarch,
> +			  int regnum, struct reggroup *group)
> +{
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> +  /* All register group */
> +  if (group == all_reggroup)
> +    {
> +      return ((regnum >= 0)
> +	      && (regnum < OR1K_NUM_REGS)
> +	      && (or1k_register_name (gdbarch, regnum)[0] != '\0'));
> +    }
> +
> +  /* For now everything except the PC */
> +  if (group == general_reggroup)
> +    {
> +      return ((regnum >= OR1K_ZERO_REGNUM)
> +	      && (regnum < OR1K_MAX_GPR_REGS)
> +	      && (regnum != OR1K_PPC_REGNUM) && (regnum != OR1K_NPC_REGNUM));
> +    }
> +
> +  if (group == float_reggroup)
> +    {
> +      return 0;	/* No float regs.  */
> +    }
> +
> +  if (group == vector_reggroup)
> +    {
> +      return 0;	/* No vector regs.  */
> +    }
> +
> +  /* For any that are not handled above.  */
> +  return default_register_reggroup_p (gdbarch, regnum, group);
> +
> +}
> +
> +
> +static int
> +or1k_is_arg_reg (unsigned int regnum)
> +{
> +  return (OR1K_FIRST_ARG_REGNUM <= regnum)
> +    && (regnum <= OR1K_LAST_ARG_REGNUM);
> +
> +}
> +
> +
> +static int
> +or1k_is_callee_saved_reg (unsigned int regnum)
> +{
> +  return (OR1K_FIRST_SAVED_REGNUM <= regnum) && (0 == regnum % 2);
> +
> +}
> +
> +
> +/* Implement the skip_prologue gdbarch method.  */
> +
> +static CORE_ADDR
> +or1k_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
> +{
> +  CORE_ADDR start_pc;
> +  CORE_ADDR addr;
> +  uint32_t inst;
> +
> +  unsigned int ra, rb, rd; /* for instruction analysis */
> +  int simm;
> +
> +  int frame_size = 0;
> +
> +  /* Try using SAL first if we have symbolic information available. This only
> +     works for DWARF 2, not STABS.  */
> +
> +  if (find_pc_partial_function (pc, NULL, &start_pc, NULL))
> +    {
> +      CORE_ADDR prologue_end = skip_prologue_using_sal (gdbarch, pc);
> +
> +      if (0 != prologue_end)
> +	{
> +	  struct symtab_and_line prologue_sal = find_pc_line (start_pc, 0);
> +	  struct compunit_symtab *compunit
> +	    = SYMTAB_COMPUNIT (prologue_sal.symtab);
> +	  const char *debug_format = COMPUNIT_DEBUGFORMAT (compunit);
> +
> +	  if ((NULL != debug_format)
> +	      && (strlen ("dwarf") <= strlen (debug_format))
> +	      && (0 == strncasecmp ("dwarf", debug_format, strlen ("dwarf"))))
> +	    {
> +	      return (prologue_end > pc) ? prologue_end : pc;
> +	    }
> +	}
> +    }
> +
> +  /* Look to see if we can find any of the standard prologue sequence. All
> +     quite difficult, since any or all of it may be missing. So this is just a
> +     best guess!  */
> +
> +  addr = pc; /* Where we have got to */
> +  inst = or1k_fetch_instruction (gdbarch, addr);
> +
> +  /* Look for the new stack pointer being set up.  */
> +  if (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
> +      && (OR1K_SP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
> +      && (simm < 0) && (0 == (simm % 4)))
> +    {
> +      frame_size = -simm;
> +      addr += OR1K_INSTLEN;
> +      inst = or1k_fetch_instruction (gdbarch, addr);
> +    }
> +
> +  /* Look for the frame pointer being manipulated.  */
> +  if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> +      && (OR1K_SP_REGNUM == ra) && (OR1K_FP_REGNUM == rb)
> +      && (simm >= 0) && (0 == (simm % 4)))
> +    {
> +      addr += OR1K_INSTLEN;
> +      inst = or1k_fetch_instruction (gdbarch, addr);
> +
> +      gdb_assert (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
> +		  && (OR1K_FP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
> +		  && (simm == frame_size));
> +
> +      addr += OR1K_INSTLEN;
> +      inst = or1k_fetch_instruction (gdbarch, addr);
> +    }
> +
> +  /* Look for the link register being saved.  */
> +  if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> +      && (OR1K_SP_REGNUM == ra) && (OR1K_LR_REGNUM == rb)
> +      && (simm >= 0) && (0 == (simm % 4)))
> +    {
> +      addr += OR1K_INSTLEN;
> +      inst = or1k_fetch_instruction (gdbarch, addr);
> +    }
> +
> +  /* Look for arguments or callee-saved register being saved. The register
> +     must be one of the arguments (r3-r8) or the 10 callee saved registers
> +     (r10, r12, r14, r16, r18, r20, r22, r24, r26, r28, r30). The base
> +     register must be the FP (for the args) or the SP (for the callee_saved
> +     registers).  */
> +  while (1)
> +    {
> +      if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> +	  && (((OR1K_FP_REGNUM == ra) && or1k_is_arg_reg (rb))
> +	      || ((OR1K_SP_REGNUM == ra) && or1k_is_callee_saved_reg (rb)))
> +	  && (0 == (simm % 4)))
> +	{
> +	  addr += OR1K_INSTLEN;
> +	  inst = or1k_fetch_instruction (gdbarch, addr);
> +	}
> +      else
> +	{
> +	  /* Nothing else to look for. We have found the end of the prologue. */
> +	  return addr;
> +	}
> +    }
> +}
> +
> +
> +/* Implement the frame_align gdbarch method.  */
> +
> +static CORE_ADDR
> +or1k_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
> +{
> +  return align_down (sp, OR1K_STACK_ALIGN);
> +
> +}
> +
> +/* Implement the unwind_pc gdbarch method.  */
> +
> +static CORE_ADDR
> +or1k_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
> +{
> +  CORE_ADDR pc;
> +
> +  if (frame_debug)
> +    {
> +      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_pc, next_frame=%d\n",
> +			  frame_relative_level (next_frame));
> +    }
> +
> +  pc = frame_unwind_register_unsigned (next_frame, OR1K_NPC_REGNUM);
> +
> +  if (frame_debug)
> +    {
> +      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_pc, pc=0x%p\n",
> +			  (void *) pc);
> +    }
> +
> +  return pc;
> +
> +}
> +
> +/* Implement the unwind_sp gdbarch method.  */
> +
> +static CORE_ADDR
> +or1k_unwind_sp (struct gdbarch *gdbarch, struct frame_info *next_frame)
> +{
> +  CORE_ADDR sp;
> +
> +  if (frame_debug)
> +    {
> +      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_sp, next_frame=%d\n",
> +			  frame_relative_level (next_frame));
> +    }
> +
> +  sp = frame_unwind_register_unsigned (next_frame, OR1K_SP_REGNUM);
> +
> +  if (frame_debug)
> +    {
> +      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_sp, sp=0x%p\n",
> +			  (void *) sp);
> +    }
> +
> +  return sp;
> +
> +}
> +
> +/* Implement the push_dummy_code gdbarch method.  */
> +
> +static CORE_ADDR
> +or1k_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
> +		      CORE_ADDR function, struct value **args, int nargs,
> +		      struct type *value_type, CORE_ADDR * real_pc,
> +		      CORE_ADDR * bp_addr, struct regcache *regcache)
> +{
> +  CORE_ADDR bp_slot;
> +
> +  /* Reserve enough room on the stack for our breakpoint instruction.  */
> +  bp_slot = sp - 4;
> +  /* Store the address of that breakpoint.  */
> +  *bp_addr = bp_slot;
> +  /* keeping the stack aligned.  */
> +  sp = or1k_frame_align (gdbarch, bp_slot);
> +  /* The call starts at the callee's entry point.  */
> +  *real_pc = function;
> +
> +  return sp;
> +
> +}
> +
> +/* Implement the push_dummy_call gdbarch method.  */
> +
> +static CORE_ADDR
> +or1k_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
> +		      struct regcache *regcache, CORE_ADDR bp_addr,
> +		      int nargs, struct value **args, CORE_ADDR sp,
> +		      int struct_return, CORE_ADDR struct_addr)
> +{
> +
> +  int argreg;
> +  int argnum;
> +  int first_stack_arg;
> +  int stack_offset = 0;
> +  int heap_offset = 0;
> +  CORE_ADDR heap_sp = sp - 128;
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  unsigned int bpa = (gdbarch_tdep (gdbarch))->bytes_per_address;
> +  unsigned int bpw = (gdbarch_tdep (gdbarch))->bytes_per_word;
> +  struct type *func_type = value_type (function);
> +
> +  /* Return address */
> +  regcache_cooked_write_unsigned (regcache, OR1K_LR_REGNUM, bp_addr);
> +
> +  /* Register for the next argument.  */
> +  argreg = OR1K_FIRST_ARG_REGNUM;
> +
> +  /* Location for a returned structure. This is passed as a silent first
> +     argument.  */
> +  if (struct_return)
> +    {
> +      regcache_cooked_write_unsigned (regcache, OR1K_FIRST_ARG_REGNUM,
> +				      struct_addr);
> +      argreg++;
> +    }
> +
> +  /* Put as many args as possible in registers */
> +  for (argnum = 0; argnum < nargs; argnum++)
> +    {
> +      const gdb_byte *val;
> +      gdb_byte valbuf[sizeof (ULONGEST)];
> +
> +      struct value *arg = args[argnum];
> +      struct type *arg_type = check_typedef (value_type (arg));
> +      int len = arg_type->length;
> +      enum type_code typecode = arg_type->main_type->code;
> +
> +      if (TYPE_VARARGS (func_type) && argnum >= TYPE_NFIELDS (func_type))
> +	{
> +	  break; /* end or regular args, varargs go to stack */
> +	}
> +
> +      /* Extract the value, either a reference or the data */
> +      if ((TYPE_CODE_STRUCT == typecode) || (TYPE_CODE_UNION == typecode)
> +	  || (len > bpw * 2))
> +	{
> +	  CORE_ADDR valaddr = value_address (arg);
> +
> +	  /* if the arg is fabricated (i.e. 3*i, instead of i) valaddr is undefined */
> +	  if (valaddr == 0)
> +	    {
> +	      /* The argument needs to be copied into the target space. Since
> +	         the bottom of the stack is reserved for function arguments
> +	         we store this at the these at the top growing down.  */
> +	      heap_offset += align_up (len, bpw);
> +	      valaddr = heap_sp + heap_offset;
> +
> +	      write_memory (valaddr, value_contents (arg), len);
> +	    }
> +
> +	  /* The ABI passes all structures by reference, so get its address.  */
> +	  store_unsigned_integer (valbuf, bpa, byte_order, valaddr);
> +	  len = bpa;
> +	  val = valbuf;
> +	}
> +      else
> +	{
> +	  /* Everything else, we just get the value. */
> +	  val = value_contents (arg);
> +	}
> +
> +      /* Stick the value in a register */
> +      if (len > bpw)
> +	{
> +	  /* Big scalars use two registers, but need NOT be pair aligned.  */
> +
> +	  if (argreg <= (OR1K_LAST_ARG_REGNUM - 1))
> +	    {
> +	      ULONGEST regval =	extract_unsigned_integer (val, len, byte_order);
> +
> +	      unsigned int bits_per_word = bpw * 8;
> +	      ULONGEST mask = (((ULONGEST) 1) << bits_per_word) - 1;
> +	      ULONGEST lo = regval & mask;
> +	      ULONGEST hi = regval >> bits_per_word;
> +
> +	      regcache_cooked_write_unsigned (regcache, argreg, hi);
> +	      regcache_cooked_write_unsigned (regcache, argreg + 1, lo);
> +	      argreg += 2;
> +	    }
> +	  else
> +	    {
> +	      /* Run out of regs */
> +	      break;
> +	    }
> +	}
> +      else if (argreg <= OR1K_LAST_ARG_REGNUM)
> +	{
> +	  /* Smaller scalars fit in a single register */
> +	  regcache_cooked_write_unsigned (regcache, argreg,
> +					  extract_unsigned_integer (val, len,
> +								    byte_order));
> +	  argreg++;
> +	}
> +      else
> +	{
> +	  /* Run out of regs */
> +	  break;
> +	}
> +    }
> +
> +  first_stack_arg = argnum;
> +
> +  /* If we get here with argnum < nargs, then arguments remain to be placed on
> +     the stack. This is tricky, since they must be pushed in reverse order and
> +     the stack in the end must be aligned. The only solution is to do it in
> +     two stages, the first to compute the stack size, the second to save the
> +     args.  */
> +
> +  for (argnum = first_stack_arg; argnum < nargs; argnum++)
> +    {
> +      struct value *arg = args[argnum];
> +      struct type *arg_type = check_typedef (value_type (arg));
> +      int len = arg_type->length;
> +      enum type_code typecode = arg_type->main_type->code;
> +
> +      if ((TYPE_CODE_STRUCT == typecode) || (TYPE_CODE_UNION == typecode)
> +	  || (len > bpw * 2))
> +	{
> +	  /* Structures are passed as addresses */
> +	  sp -= bpa;
> +	}
> +      else
> +	{
> +	  /* Big scalars use more than one word. Code here allows for future
> +	     quad-word entities (e.g. long double) */
> +	  sp -= align_up (len, bpw);
> +	}
> +
> +      /* ensure our dummy heap doesn't touch the stack, this could only happen
> +         if we have many arguments including fabricated arguments */
> +      gdb_assert (heap_offset == 0 || ((heap_sp + heap_offset) < sp));
> +    }
> +
> +  sp = gdbarch_frame_align (gdbarch, sp);
> +  stack_offset = 0;
> +
> +  /* Push the remaining args on the stack */
> +  for (argnum = first_stack_arg; argnum < nargs; argnum++)
> +    {
> +      const gdb_byte *val;
> +      gdb_byte valbuf[sizeof (ULONGEST)];
> +
> +      struct value *arg = args[argnum];
> +      struct type *arg_type = check_typedef (value_type (arg));
> +      int len = arg_type->length;
> +      enum type_code typecode = arg_type->main_type->code;
> +      /* The EABI passes structures that do not fit in a register by
> +         reference. In all other cases, pass the structure by value.  */
> +      if ((TYPE_CODE_STRUCT == typecode) || (TYPE_CODE_UNION == typecode)
> +	  || (len > bpw * 2))
> +	{
> +	  store_unsigned_integer (valbuf, bpa, byte_order,
> +				  value_address (arg));
> +	  len = bpa;
> +	  val = valbuf;
> +	}
> +      else
> +	{
> +	  val = value_contents (arg);
> +	}
> +
> +      while (len > 0)
> +	{
> +	  int partial_len = (len < bpw ? len : bpw);
> +
> +	  write_memory (sp + stack_offset, val, partial_len);
> +	  stack_offset += align_up (partial_len, bpw);
> +	  len -= partial_len;
> +	  val += partial_len;
> +	}
> +    }
> +
> +  /* Save the updated stack pointer */
> +  regcache_cooked_write_unsigned (regcache, OR1K_SP_REGNUM, sp);
> +
> +  if (heap_offset > 0)
> +    {
> +      sp = heap_sp;
> +    }
> +
> +  return sp;
> +
> +}
> +
> +
> +/* Implement the dummy_id gdbarch method.  */
> +
> +static struct frame_id
> +or1k_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame)
> +{
> +  return frame_id_build (get_frame_sp (this_frame),
> +			 get_frame_pc (this_frame));
> +
> +}
> +\f
> +
> +
> +
> +/* Support functions for frame handling */
> +
> +/* Initialize a prologue cache
> +
> +   We build a cache, saying where registers of the PREV frame can be found
> +   from the data so far set up in this THIS.
> +
> +   We also compute a unique ID for this frame, based on the function start
> +   address and the stack pointer (as it will be, even if it has yet to be
> +   computed.
> +
> +   STACK FORMAT
> +   ============
> +
> +   The OR1K has a falling stack frame and a simple prolog. The Stack pointer
> +   is R1 and the frame pointer R2. The frame base is therefore the address
> +   held in R2 and the stack pointer (R1) is the frame base of the NEXT frame.
> +
> +   l.addi  r1,r1,-frame_size	# SP now points to end of new stack frame
> +
> +   The stack pointer may not be set up in a frameless function (e.g. a simple
> +   leaf function).
> +
> +   l.sw    fp_loc(r1),r2        # old FP saved in new stack frame
> +   l.addi  r2,r1,frame_size     # FP now points to base of new stack frame
> +
> +   The frame pointer is not necessarily saved right at the end of the stack
> +   frame - OR1K saves enough space for any args to called functions right at
> +   the end (this is a difference from the Architecture Manual).
> +
> +   l.sw    lr_loc(r1),r9        # Link (return) address
> +
> +   The link register is usally saved at fp_loc - 4. It may not be saved at all
> +   in a leaf function.
> +
> +   l.sw    reg_loc(r1),ry       # Save any callee saved regs
> +
> +   The offsets x for the callee saved registers generally (always?) rise in
> +   increments of 4, starting at fp_loc + 4. If the frame pointer is omitted
> +   (an option to GCC), then it may not be saved at all. There may be no callee
> +   saved registers.
> +
> +   So in summary none of this may be present. However what is present seems
> +   always to follow this fixed order, and occur before any substantive code
> +   (it is possible for GCC to have more flexible scheduling of the prologue,
> +   but this does not seem to occur for OR1K).
> +
> +   ANALYSIS
> +   ========
> +
> +   This prolog is used, even for -O3 with GCC.
> +
> +   All this analysis must allow for the possibility that the PC is in the
> +   middle of the prologue. Data in the cache should only be set up insofar as
> +   it has been computed.
> +
> +   HOWEVER. The frame_id must be created with the SP *as it will be* at the
> +   end of the Prologue. Otherwise a recursive call, checking the frame with
> +   the PC at the start address will end up with the same frame_id as the
> +   caller.
> +
> +   A suite of "helper" routines are used, allowing reuse for
> +   or1k_skip_prologue().
> +
> +   Reportedly, this is only valid for frames less than 0x7fff in size.  */
> +
> +static struct trad_frame_cache *
> +or1k_frame_cache (struct frame_info *this_frame, void **prologue_cache)
> +{
> +  struct gdbarch *gdbarch;
> +  struct trad_frame_cache *info;
> +
> +  CORE_ADDR this_pc;
> +  CORE_ADDR this_sp;
> +  CORE_ADDR this_sp_for_id;
> +  int frame_size = 0;
> +
> +  CORE_ADDR start_addr;
> +  CORE_ADDR end_addr;
> +
> +  if (frame_debug)
> +    {
> +      fprintf_unfiltered (gdb_stdlog,
> +			  "or1k_frame_cache, prologue_cache = 0x%p\n",
> +			  *prologue_cache);
> +    }
> +
> +  /* Nothing to do if we already have this info.  */
> +  if (NULL != *prologue_cache)
> +    {
> +      return (struct trad_frame_cache *) *prologue_cache;
> +    }
> +
> +  /* Get a new prologue cache and populate it with default values.  */
> +  info = trad_frame_cache_zalloc (this_frame);
> +  *prologue_cache = info;
> +
> +  /* Find the start address of THIS function (which is a NORMAL frame, even if
> +     the NEXT frame is the sentinel frame) and the end of its prologue.  */
> +  this_pc = get_frame_pc (this_frame);
> +  find_pc_partial_function (this_pc, NULL, &start_addr, NULL);
> +
> +  /* Get the stack pointer if we have one (if there's no process executing yet
> +     we won't have a frame.  */
> +  this_sp = (NULL == this_frame) ? 0 :
> +    get_frame_register_unsigned (this_frame, OR1K_SP_REGNUM);
> +
> +  /* Return early if GDB couldn't find the function.  */
> +  if (start_addr == 0)
> +    {
> +      if (frame_debug)
> +	{
> +	  fprintf_unfiltered (gdb_stdlog, "  couldn't find function\n");
> +	}
> +
> +      /* JPB: 28-Apr-11. This is a temporary patch, to get round GDB crashing
> +         right at the beginning. Build the frame ID as best we can. */
> +      trad_frame_set_id (info, frame_id_build (this_sp, this_pc));
> +
> +      return info;
> +    }
> +
> +
> +  /* The default frame base of THIS frame (for ID purposes only - frame base
> +     is an overloaded term) is its stack pointer. For now we use the value of
> +     the SP register in THIS frame. However if the PC is in the prologue of
> +     THIS frame, before the SP has been set up, then the value will actually
> +     be that of the PREV frame, and we'll need to adjust it later. */
> +  trad_frame_set_this_base (info, this_sp);
> +  this_sp_for_id = this_sp;
> +
> +  /* The default is to find the PC of the PREVIOUS frame in the link register
> +     of this frame. This may be changed if we find the link register was saved
> +     on the stack. */
> +  trad_frame_set_reg_realreg (info, OR1K_NPC_REGNUM, OR1K_LR_REGNUM);
> +
> +  /* We should only examine code that is in the prologue. This is all code up
> +     to (but not including) end_addr. We should only populate the cache while
> +     the address is up to (but not including) the PC or end_addr, whichever is
> +     first. */
> +  gdbarch = get_frame_arch (this_frame);
> +  end_addr = or1k_skip_prologue (gdbarch, start_addr);
> +
> +  /* All the following analysis only occurs if we are in the prologue and have
> +     executed the code. Check we have a sane prologue size, and if zero we
> +     are frameless and can give up here. */
> +  if (end_addr < start_addr)
> +    {
> +      throw_quit ("end addr 0x%08x is less than start addr 0x%08x\n",
> +		  (unsigned int) end_addr, (unsigned int) start_addr);
> +    }
> +
> +  if (end_addr == start_addr)
> +    {
> +      frame_size = 0;
> +    }
> +  else
> +    {
> +      /* have a frame. Look for the various components */
> +      CORE_ADDR addr = start_addr;	/* Where we have got to */
> +      uint32_t inst = or1k_fetch_instruction (gdbarch, addr);
> +
> +      unsigned int ra, rb, rd;	/* For instruction analysis */
> +      int simm;
> +
> +      /* Look for the new stack pointer being set up. */
> +      if (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
> +	  && (OR1K_SP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
> +	  && (simm < 0) && (0 == (simm % 4)))
> +	{
> +	  frame_size = -simm;
> +	  addr += OR1K_INSTLEN;
> +	  inst = or1k_fetch_instruction (gdbarch, addr);
> +
> +	  /* If the PC has not actually got to this point, then the frame base
> +	     will be wrong, and we adjust it.
> +
> +	     If we are past this point, then we need to populate the stack
> +	     accoringly. */
> +	  if (this_pc <= addr)
> +	    {
> +	      /* Only do if executing */
> +	      if (0 != this_sp)
> +		{
> +		  this_sp_for_id = this_sp + frame_size;
> +		  trad_frame_set_this_base (info, this_sp_for_id);
> +		}
> +	    }
> +	  else
> +	    {
> +	      /* We are past this point, so the stack pointer of the PREV
> +	         frame is frame_size greater than the stack pointer of THIS
> +	         frame. */
> +	      trad_frame_set_reg_value (info, OR1K_SP_REGNUM,
> +					this_sp + frame_size);
> +	    }
> +	}
> +
> +      /* From now on we are only populating the cache, so we stop once we get
> +         to either the end OR the current PC. */
> +      end_addr = (this_pc < end_addr) ? this_pc : end_addr;
> +
> +      /* Look for the frame pointer being manipulated. */
> +      if ((addr < end_addr)
> +	  && or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> +	  && (OR1K_SP_REGNUM == ra) && (OR1K_FP_REGNUM == rb)
> +	  && (simm >= 0) && (0 == (simm % 4)))
> +	{
> +	  addr += OR1K_INSTLEN;
> +	  inst = or1k_fetch_instruction (gdbarch, addr);
> +
> +	  /* At this stage, we can find the frame pointer of the PREVIOUS
> +	     frame on the stack of the current frame. */
> +	  trad_frame_set_reg_addr (info, OR1K_FP_REGNUM, this_sp + simm);
> +
> +	  /* Look for the new frame pointer being set up */
> +	  if (addr < end_addr)
> +	    {
> +	      gdb_assert (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
> +			  && (OR1K_FP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
> +			  && (simm == frame_size));
> +
> +	      addr += OR1K_INSTLEN;
> +	      inst = or1k_fetch_instruction (gdbarch, addr);
> +
> +	      /* If we have got this far, the stack pointer of the PREVIOUS
> +	         frame is the frame pointer of THIS frame. */
> +	      trad_frame_set_reg_realreg (info, OR1K_SP_REGNUM,
> +					  OR1K_FP_REGNUM);
> +	    }
> +	}
> +
> +      /* Look for the link register being saved */
> +      if ((addr < end_addr)
> +	  && or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> +	  && (OR1K_SP_REGNUM == ra) && (OR1K_LR_REGNUM == rb)
> +	  && (simm >= 0) && (0 == (simm % 4)))
> +	{
> +	  addr += OR1K_INSTLEN;
> +	  inst = or1k_fetch_instruction (gdbarch, addr);
> +
> +	  /* If the link register is saved in the THIS frame, it holds the
> +	     value of the PC in the PREVIOUS frame. This overwrites the
> +	     previous information about finding the PC in the link
> +	     register.  */
> +	  trad_frame_set_reg_addr (info, OR1K_NPC_REGNUM, this_sp + simm);
> +	}
> +
> +      /* Look for arguments or callee-saved register being saved. The register
> +         must be one of the arguments (r3-r8) or the 10 callee saved registers
> +         (r10, r12, r14, r16, r18, r20, r22, r24, r26, r28, r30). The base
> +         register must be the FP (for the args) or the SP (for the
> +         callee_saved registers).  */
> +      while (addr < end_addr)
> +	{
> +	  if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> +	      && (((OR1K_FP_REGNUM == ra) && or1k_is_arg_reg (rb))
> +		  || ((OR1K_SP_REGNUM == ra)
> +		      && or1k_is_callee_saved_reg (rb))) && (0 == (simm % 4)))
> +	    {
> +	      addr += OR1K_INSTLEN;
> +	      inst = or1k_fetch_instruction (gdbarch, addr);
> +
> +	      /* The register in the PREVIOUS frame can be found at this
> +	         location in THIS frame */
> +	      trad_frame_set_reg_addr (info, rb, this_sp + simm);
> +	    }
> +	  else
> +	    {
> +	      break;		/* Not a register save instruction */
> +	    }
> +	}
> +    }
> +
> +  /* Build the frame ID */
> +  trad_frame_set_id (info, frame_id_build (this_sp_for_id, start_addr));
> +
> +  if (frame_debug)
> +    {
> +      fprintf_unfiltered (gdb_stdlog, "  this_sp_for_id = 0x%p\n",
> +			  (void *) this_sp_for_id);
> +      fprintf_unfiltered (gdb_stdlog, "  start_addr     = 0x%p\n",
> +			  (void *) start_addr);
> +    }
> +
> +  return info;
> +
> +}
> +
> +
> +/* Implement the this_id function for the stub unwinder.  */
> +
> +static void
> +or1k_frame_this_id (struct frame_info *this_frame,
> +		    void **prologue_cache, struct frame_id *this_id)
> +{
> +  struct trad_frame_cache *info = or1k_frame_cache (this_frame,
> +						    prologue_cache);
> +
> +  trad_frame_get_id (info, this_id);
> +
> +}
> +
> +
> +/* Implement the prev_register function for the stub unwinder.  */
> +
> +static struct value *
> +or1k_frame_prev_register (struct frame_info *this_frame,
> +			  void **prologue_cache, int regnum)
> +{
> +  struct trad_frame_cache *info = or1k_frame_cache (this_frame,
> +						    prologue_cache);
> +
> +  return trad_frame_get_register (info, this_frame, regnum);
> +
> +}
> +
> +/* Data structures for the normal prologue-analysis-based
> +   unwinder.  */
> +
> +static const struct frame_unwind or1k_frame_unwind = {
> +  .type = NORMAL_FRAME,
> +  .stop_reason = default_frame_unwind_stop_reason,
> +  .this_id = or1k_frame_this_id,
> +  .prev_register = or1k_frame_prev_register,
> +  .unwind_data = NULL,
> +  .sniffer = default_frame_sniffer,
> +  .dealloc_cache = NULL,
> +  .prev_arch = NULL
> +};
> +
> +/* Architecture initialization for OpenRISC 1000.  */
> +
> +static struct gdbarch *
> +or1k_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> +{
> +  static struct frame_base or1k_frame_base;
> +  struct gdbarch *gdbarch;
> +  struct gdbarch_tdep *tdep;
> +  const struct bfd_arch_info *binfo;
> +  struct tdesc_arch_data *tdesc_data = NULL;
> +
> +  int i;
> +  int reg_index = 0;
> +  int retval;
> +  int group;
> +
> +  /* Find a candidate among the list of pre-declared architectures.  */
> +  arches = gdbarch_list_lookup_by_info (arches, &info);
> +  if (NULL != arches)
> +    {
> +      return arches->gdbarch;
> +    }
> +
> +  /* None found, create a new architecture from the information
> +     provided. Can't initialize all the target dependencies until we actually
> +     know which target we are talking to, but put in some defaults for now. */
> +
> +  binfo = info.bfd_arch_info;
> +  tdep = XNEW (struct gdbarch_tdep);

XCNEW may be better suited since it zeroes out all fields?

> +  tdep->bytes_per_word = binfo->bits_per_word / binfo->bits_per_byte;
> +  tdep->bytes_per_address = binfo->bits_per_address / binfo->bits_per_byte;
> +  gdbarch = gdbarch_alloc (&info, tdep);
> +
> +  /* Target data types.  */
> +  set_gdbarch_short_bit (gdbarch, 16);
> +  set_gdbarch_int_bit (gdbarch, 32);
> +  set_gdbarch_long_bit (gdbarch, 32);
> +  set_gdbarch_long_long_bit (gdbarch, 64);
> +  set_gdbarch_float_bit (gdbarch, 32);
> +  set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
> +  set_gdbarch_double_bit (gdbarch, 64);
> +  set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
> +  set_gdbarch_long_double_bit (gdbarch, 64);
> +  set_gdbarch_long_double_format (gdbarch, floatformats_ieee_double);
> +  set_gdbarch_ptr_bit (gdbarch, binfo->bits_per_address);
> +  set_gdbarch_addr_bit (gdbarch, binfo->bits_per_address);
> +  set_gdbarch_char_signed (gdbarch, 1);
> +
> +  /* Information about the target architecture */
> +  set_gdbarch_return_value (gdbarch, or1k_return_value);
> +  set_gdbarch_breakpoint_kind_from_pc (gdbarch, or1k_breakpoint::kind_from_pc);
> +  set_gdbarch_sw_breakpoint_from_kind (gdbarch, or1k_breakpoint::bp_from_kind);
> +  set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
> +
> +  set_gdbarch_print_insn (gdbarch, print_insn_or1k);
> +
> +  /* Register architecture */
> +  set_gdbarch_num_regs (gdbarch, OR1K_NUM_REGS);
> +  set_gdbarch_num_pseudo_regs (gdbarch, OR1K_NUM_PSEUDO_REGS);
> +  set_gdbarch_sp_regnum (gdbarch, OR1K_SP_REGNUM);
> +  set_gdbarch_pc_regnum (gdbarch, OR1K_NPC_REGNUM);
> +  set_gdbarch_ps_regnum (gdbarch, OR1K_SR_REGNUM);
> +  set_gdbarch_deprecated_fp_regnum (gdbarch, OR1K_FP_REGNUM);
> +
> +  /* Functions to supply register information */
> +  set_gdbarch_register_name (gdbarch, or1k_register_name);
> +  set_gdbarch_register_type (gdbarch, or1k_register_type);
> +  set_gdbarch_register_reggroup_p (gdbarch, or1k_register_reggroup_p);
> +
> +  /* Functions to analyse frames */
> +  set_gdbarch_skip_prologue (gdbarch, or1k_skip_prologue);
> +  set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
> +  set_gdbarch_frame_align (gdbarch, or1k_frame_align);
> +  set_gdbarch_frame_red_zone_size (gdbarch, OR1K_FRAME_RED_ZONE_SIZE);
> +
> +  /* Functions to access frame data */
> +  set_gdbarch_unwind_pc (gdbarch, or1k_unwind_pc);
> +  set_gdbarch_unwind_sp (gdbarch, or1k_unwind_sp);
> +
> +  /* Functions handling dummy frames */
> +  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
> +  set_gdbarch_push_dummy_code (gdbarch, or1k_push_dummy_code);
> +  set_gdbarch_push_dummy_call (gdbarch, or1k_push_dummy_call);
> +  set_gdbarch_dummy_id (gdbarch, or1k_dummy_id);
> +
> +  /* Frame unwinders. Use DWARF debug info if available, otherwise use our
> +     own unwinder.  */
> +  dwarf2_append_unwinders (gdbarch);
> +  frame_unwind_append_unwinder (gdbarch, &or1k_frame_unwind);
> +
> +  /* Get a CGEN CPU descriptor for this architecture.  */
> +  {
> +
> +    const char *mach_name = binfo->printable_name;
> +    enum cgen_endian endian = (info.byte_order == BFD_ENDIAN_BIG
> +			       ? CGEN_ENDIAN_BIG : CGEN_ENDIAN_LITTLE);
> +
> +    tdep->gdb_cgen_cpu_desc =
> +      or1k_cgen_cpu_open (CGEN_CPU_OPEN_BFDMACH, mach_name,
> +			  CGEN_CPU_OPEN_ENDIAN, endian, CGEN_CPU_OPEN_END);
> +
> +    or1k_cgen_init_asm (tdep->gdb_cgen_cpu_desc);
> +  }
> +
> +  /* If this mach has a delay slot.  */
> +  if (binfo->mach == bfd_mach_or1k)
> +    {
> +      set_gdbarch_single_step_through_delay
> +	(gdbarch, or1k_single_step_through_delay);
> +    }
> +
> +  /* Check any target description for validity.  */
> +  if (tdesc_has_registers (info.target_desc))
> +    {
> +      const struct tdesc_feature *feature;
> +      int valid_p;
> +
> +      feature = tdesc_find_feature (info.target_desc, "org.gnu.gdb.or1k.group0");

Where is this target description coming from? I don't see an xml file 
with the patch series. Is it going to be submitted? Or this something a 
remote stub/simulator sends GDB upon connection?

> +      if (feature == NULL)
> +        return NULL;
> +
> +      tdesc_data = tdesc_data_alloc ();
> +
> +      valid_p = 1;
> +
> +      for (i = 0; i < OR1K_NUM_REGS; i++)
> +        valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
> +                                            or1k_reg_names[i]);
> +
> +      if (!valid_p)
> +        {
> +          tdesc_data_cleanup (tdesc_data);
> +          return NULL;
> +        }
> +    }
> +
> +  if (tdesc_data)
> +    {
> +      tdesc_use_registers (gdbarch, info.target_desc, tdesc_data);
> +
> +      /* Target descriptions may extend into the following groups.  */
> +      reggroup_add (gdbarch, general_reggroup);
> +      reggroup_add (gdbarch, system_reggroup);
> +      reggroup_add (gdbarch, float_reggroup);
> +      reggroup_add (gdbarch, vector_reggroup);
> +      reggroup_add (gdbarch, reggroup_new ("immu", USER_REGGROUP));
> +      reggroup_add (gdbarch, reggroup_new ("dmmu", USER_REGGROUP));
> +      reggroup_add (gdbarch, reggroup_new ("icache", USER_REGGROUP));
> +      reggroup_add (gdbarch, reggroup_new ("dcache", USER_REGGROUP));
> +      reggroup_add (gdbarch, reggroup_new ("pic", USER_REGGROUP));
> +      reggroup_add (gdbarch, reggroup_new ("timer", USER_REGGROUP));
> +      reggroup_add (gdbarch, reggroup_new ("power", USER_REGGROUP));
> +      reggroup_add (gdbarch, reggroup_new ("perf", USER_REGGROUP));
> +      reggroup_add (gdbarch, reggroup_new ("mac", USER_REGGROUP));
> +      reggroup_add (gdbarch, reggroup_new ("debug", USER_REGGROUP));
> +      reggroup_add (gdbarch, all_reggroup);
> +      reggroup_add (gdbarch, save_reggroup);
> +      reggroup_add (gdbarch, restore_reggroup);
> +    }
> +
> +  return gdbarch;
> +
> +}
> +
> +/* Dump the target specific data for this architecture.  */
> +
> +static void
> +or1k_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
> +{
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> +  if (NULL == tdep)
> +    {
> +      return;			/* Nothing to report */
> +    }
> +
> +  fprintf_unfiltered (file, "or1k_dump_tdep: %d bytes per word\n",
> +		      tdep->bytes_per_word);
> +  fprintf_unfiltered (file, "or1k_dump_tdep: %d bytes per address\n",
> +		      tdep->bytes_per_address);
> +
> +}
> +\f
> +
> +extern initialize_file_ftype _initialize_or1k_tdep; /* -Wmissing-prototypes */
> +
> +void
> +_initialize_or1k_tdep (void)
> +{
> +  /* Register this architecture.  */
> +  gdbarch_register (bfd_arch_or1k, or1k_gdbarch_init, or1k_dump_tdep);
> +
> +  /* Tell remote stub that we support XML target description.  */
> +  register_remote_support_xml ("or1k");
> +}
> diff --git a/gdb/or1k-tdep.h b/gdb/or1k-tdep.h
> new file mode 100644
> index 0000000..43ae7c3
> --- /dev/null
> +++ b/gdb/or1k-tdep.h
> @@ -0,0 +1,57 @@
> +/* Definitions to target GDB to OpenRISC 1000 32-bit targets.
> +   Copyright (C) 2008-2016 Free Software Foundation, Inc.
> +   Contributed by Jeremy Bennett <jeremy.bennett@embecosm.com>
> +
> +   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 3 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, see <http://www.gnu.org/licenses/>.  */
> +
> +
> +#ifndef OR1K_TDEP__H
> +#define OR1K_TDEP__H
> +
> +#ifndef TARGET_OR1K
> +#define TARGET_OR1K
> +#endif
> +
> +#include "opcodes/or1k-desc.h"
> +#include "opcodes/or1k-opc.h"
> +
> +/* General Purpose Registers */
> +#define OR1K_ZERO_REGNUM          0
> +#define OR1K_SP_REGNUM            1
> +#define OR1K_FP_REGNUM            2
> +#define OR1K_FIRST_ARG_REGNUM     3
> +#define OR1K_LAST_ARG_REGNUM      8
> +#define OR1K_LR_REGNUM            9
> +#define OR1K_FIRST_SAVED_REGNUM  10
> +#define OR1K_RV_REGNUM           11
> +#define OR1K_PPC_REGNUM          (OR1K_MAX_GPR_REGS + 0)
> +#define OR1K_NPC_REGNUM          (OR1K_MAX_GPR_REGS + 1)
> +#define OR1K_SR_REGNUM           (OR1K_MAX_GPR_REGS + 2)
> +
> +/* Properties of the architecture. GDB mapping of registers is all the GPRs
> +   and SPRs followed by the PPC, NPC and SR at the end. Red zone is the area
> +   past the end of the stack reserved for exception handlers etc.  */
> +
> +#define OR1K_MAX_GPR_REGS            32
> +#define OR1K_NUM_PSEUDO_REGS         0
> +#define OR1K_NUM_REGS               (OR1K_MAX_GPR_REGS + 3)
> +#define OR1K_STACK_ALIGN             4
> +#define OR1K_INSTLEN                 4
> +#define OR1K_INSTBITLEN             (OR1K_INSTLEN * 8)
> +#define OR1K_NUM_TAP_RECORDS         8
> +#define OR1K_FRAME_RED_ZONE_SIZE     2536
> +
> +#endif /* OR1K_TDEP__H */
>

It would be nice to have all the formatting and cosmetics polished/fixed 
before we can give it another look for correctness of the code itself. 
Right now there are quite a bit of formatting issues.

Patches 2/3 and 3/3 look fine to me.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support
  2016-12-23 21:21   ` Luis Machado
@ 2016-12-25  1:31     ` Stafford Horne
  2016-12-26 16:10       ` Luis Machado
  0 siblings, 1 reply; 8+ messages in thread
From: Stafford Horne @ 2016-12-25  1:31 UTC (permalink / raw)
  To: Luis Machado; +Cc: gdb-patches, openrisc, Franck Jullien

On Fri, Dec 23, 2016 at 03:20:52PM -0600, Luis Machado wrote:
> On 12/22/2016 10:14 AM, Stafford Horne wrote:
> > From: Franck Jullien <franck.jullien@gmail.com>
> > 
> > This patch prepates the current GDB port of the openrisc processor from
> > https://github.com/openrisc/binutils-gdb for upstream merging.
> > 
> > Testing has been done with a cgen sim provided in a separate patch. This
> > has been tested with 2 toolchains. GCC [1] 5.4.0 from the openrisc
> > project with Newlib [2] and GCC 5.4.0 with Musl [3] 1.1.4.
> > 
> > It supports or1knd (no delay slot target).
> > The default target is or1k (with delay slot).
> > 
> > You can change the target arch with:
> > 
> > (gdb) set architecture or1knd
> > The target architecture is assumed to be or1knd
> > 
> > [1] https://github.com/openrisc/or1k-gcc
> > [2] https://github.com/openrisc/newlib
> > [3] https://github.com/openrisc/musl-cross
> > 
> > gdb/doc/ChangeLog:
> > 
> > 2016-03-13  Stefan Wallentowitz  <stefan@wallentowitz.de>
> > 	    Franck Jullien  <franck.jullien@gmail.com>
> > 	    Jeremy Bennett  <jeremy.bennett@embecosm.com>
> > 
> > 	* gdb.texinfo: Add OpenRISC documentation.
> > 
> > gdb/ChangeLog:
> > 
> > 2016-11-23  Stafford Horne  <shorne@gmail.com>
> > 	    Stefan Wallentowitz  <stefan@wallentowitz.de>
> > 	    Stefan Kristiansson  <stefan.kristiansson@saunalahti.fi>
> > 	    Franck Jullien  <franck.jullien@gmail.com>
> > 	    Jeremy Bennett  <jeremy.bennett@embecosm.com>
> > 
> > 	* configure.tgt: Add targets for or1k and or1knd.
> > 	* or1k-tdep.c: New.
> > 	* or1k-tdep.h: New.
> > ---
> >  gdb/configure.tgt   |   12 +
> >  gdb/doc/gdb.texinfo |   68 +++
> >  gdb/or1k-tdep.c     | 1436 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  gdb/or1k-tdep.h     |   57 ++
> >  4 files changed, 1573 insertions(+)
> >  create mode 100644 gdb/or1k-tdep.c
> >  create mode 100644 gdb/or1k-tdep.h
> > 
> > diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> > index 3f2603d..40764c6 100644
> > --- a/gdb/configure.tgt
> > +++ b/gdb/configure.tgt
> > @@ -421,6 +421,18 @@ nios2*-*-*)
> >  	gdb_target_obs="nios2-tdep.o"
> >  	;;
> > 
> > +or1k-*-*)
> > +	# Target: OpenCores OpenRISC 1000 32-bit implementation bare metal
> > +	gdb_target_obs="or1k-tdep.o"
> > +	gdb_sim=../sim/or1k/libsim.a
> > +	;;
> > +
> > +or1knd-*-*)
> > +	# Target: OpenCores OpenRISC 1000 32-bit implementation bare metal, without delay slot
> > +	gdb_target_obs="or1k-tdep.o"
> > +	gdb_sim=../sim/or1k/libsim.a
> > +	;;
> > +
> 
> I noticed both targets use the same files. Is there a reason why we need
> both and not a single entry that can be identified at runtime?

Good point, the code is already there to identify delay slot / no delay
slot in or1k-tdep.c so I think just combining these is all that is
needed.
 
> >  powerpc*-*-freebsd*)
> >  	# Target: FreeBSD/powerpc
> >  	gdb_target_obs="rs6000-tdep.o ppc-sysv-tdep.o ppc64-tdep.o \
> > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> > index a0de7d1..7ae33d1 100644
> > --- a/gdb/doc/gdb.texinfo
> > +++ b/gdb/doc/gdb.texinfo
> > @@ -541,6 +541,10 @@ Steve Tjiang, John Newlin, and Scott Foehner.
> >  Michael Eager and staff of Xilinx, Inc., contributed support for the
> >  Xilinx MicroBlaze architecture.
> > 
> > +The original port to the OpenRISC 1000 is believed to be due to
> > +Alessandro Forin and Per Bothner. More recent ports have been the work
> > +of Jeremy Bennett.
> > +
> >  @node Sample Session
> >  @chapter A Sample @value{GDBN} Session
> > 
> > @@ -22022,6 +22026,7 @@ acceptable commands.
> >  * M68K::                        Motorola M68K
> >  * MicroBlaze::			Xilinx MicroBlaze
> >  * MIPS Embedded::               MIPS Embedded
> > +* OpenRISC 1000::               OpenRISC 1000 (or1k)
> >  * PowerPC Embedded::            PowerPC Embedded
> >  * AVR::                         Atmel AVR
> >  * CRIS::                        CRIS
> > @@ -22228,6 +22233,69 @@ As usual, you can inquire about the @code{mipsfpu} variable with
> >  @samp{show mipsfpu}.
> >  @end table
> > 
> > +@node OpenRISC 1000
> > +@subsection OpenRISC 1000
> > +@cindex OpenRISC 1000
> > +
> > +Previous development versions of @value{GDBN} supported remote connection
> > +via a proprietary JTAG protocol using the @samp{target jtag} command.
> > +Support for this has now been dropped.
> > +
> > +Also, previous verions had support for a @samp{spr} command to access
> > +OpenRISC's numberous special purpose registers.  These are now available
> > +via the normal @samp{info registers} command.
> > +
> > +@table @code
> > +
> > +@kindex target remote
> > +@item target remote
> > +
> > +This is now the only way to connect to a remote OpenRISC 1000
> > +target.  This is supported by @dfn{Or1ksim}, the OpenRISC 1000
> > +architectural simulator, Verilatorm and Icarus Verilog
> > +simulations.  @dfn{Remote serial protocol} servers are also available to
> 
> Verilatorm and Icarus simulations or simulators?
> 
> > +drive various hardware implementations via JTAG.
> > +Connects to remote JTAG server.
> > +
> > +Example: @code{target remote :51000}
> > +
> > +@kindex target sim
> > +@item target sim
> > +
> > +Runs the builtin CPU simulator which can run very basic
> > +programs, does not support most hardware functions like MMU.
> 
> "...programs but does not support..."?
> 
> > +However for more complex use, the user is advised to run and external
> 
> "... more complex use cases..."? Or maybe "... more complex uses..."?
> 
> > +target, and connect using @samp{target remote}
> > +
> > +Example: @code{target sim}
> > +
> > +@end table
> > +
> > +There are some known problems with the current implementation
> > +@cindex OpenRISC 1000 known problems
> > +
> > +@enumerate
> > +
> > +@item
> > +@cindex OpenRISC 1000 known problems, hardware breakpoints and watchpoints
> > +Some OpenRISC 1000 targets support hardware breakpoints and watchpoints.
> > +Consult the target documentation for details.  @value{GDBN} is not
> > +perfect in handling of watchpoints.  It is possible to allocate hardware
> > +watchpoints and not discover until running that sufficient watchpoints
> > +are not available.  It is also possible that GDB will report watchpoints
> > +being hit spuriously.  This can be down to the assembly code having
> > +additional memory accesses that are not obviously reflected in the
> > +source code.
> > +
> > +@item
> > +@cindex OpenRISC 1000 known problems, architectural compatability
> 
> typo, compatibility.
> 
> > +The OpenRISC 1000 architecture has evolved since the first port of @value{GDBN}. In particular the structure of the Unit Present register has
> 
> "...first port FOR ..."

Thanks for this documentation proof-read.  I will fix this up.

> > +changed and the CPU Configuration register has been added.  The port of
> > +@value{GDBN} version @value{GDBVN} uses the @emph{current}
> > +specification of the OpenRISC 1000.
> > +
> > +@end enumerate
> > +
> >  @node PowerPC Embedded
> >  @subsection PowerPC Embedded
> > 
> > diff --git a/gdb/or1k-tdep.c b/gdb/or1k-tdep.c
> > new file mode 100644
> > index 0000000..d906066
> > --- /dev/null
> > +++ b/gdb/or1k-tdep.c
> > @@ -0,0 +1,1436 @@
> > +/* Target-dependent code for the 32-bit OpenRISC 1000, for the GDB.
> > +   Copyright (C) 2008-2016, Free Software Foundation, Inc.
> > +   Contributed by Alessandro Forin(af@cs.cmu.edu at CMU
> > +   and by Per Bothner(bothner@cs.wisc.edu) at U.Wisconsin.
> > +   Contributed by Jeremy Bennett <jeremy.bennett@embecosm.com>
> > +   Contributed by Franck Jullien <franck.jullien@gmail.com> on behalf of
> > +   Embecosm Limited
> > +   Contributed by Stafford Horne <shorne@gmail.com>
> 
> I've seen recent reviews that point at not mentioning these contributions in
> the source code but rather in the documentation. So i think these should be
> moved somewhere else (maybe they have already been moved and you just forgot
> to remove the above?)

This is fine.  I will remove them.  Most of these are already in the
documentation.  I may as well add the rest there.
 
> > +
> > +   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 3 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, see <http://www.gnu.org/licenses/>.  */
> > +
> > +#include "defs.h"
> > +#include "frame.h"
> > +#include "inferior.h"
> > +#include "symtab.h"
> > +#include "value.h"
> > +#include "gdbcmd.h"
> > +#include "language.h"
> > +#include "gdbcore.h"
> > +#include "symfile.h"
> > +#include "objfiles.h"
> > +#include "gdbtypes.h"
> > +#include "target.h"
> > +#include "regcache.h"
> > +#include "safe-ctype.h"
> > +#include "block.h"
> > +#include "reggroups.h"
> > +#include "arch-utils.h"
> > +#include "frame.h"
> > +#include "frame-unwind.h"
> > +#include "frame-base.h"
> > +#include "dwarf2-frame.h"
> > +#include "trad-frame.h"
> > +#include "regset.h"
> > +#include "remote.h"
> > +#include "target-descriptions.h"
> > +
> > +#include <inttypes.h>
> > +
> > +#include "dis-asm.h"
> 
> Push these two includes up and together with the rest.

Sure I could do that. 

> > +
> > +/* OpenRISC specific includes.  */
> > +#include "or1k-tdep.h"
> > +\f
> > +
> > +/* The target-dependant structure for gdbarch.  */
> > +
> > +struct gdbarch_tdep
> > +{
> > +  int bytes_per_word;
> > +  int bytes_per_address;
> > +  CGEN_CPU_DESC gdb_cgen_cpu_desc;
> > +};
> > +
> > +/* Support functions for the architecture definition.  */
> > +
> > +/* Get an instruction from memory.  */
> > +
> > +static ULONGEST
> > +or1k_fetch_instruction (struct gdbarch *gdbarch, CORE_ADDR addr)
> > +{
> > +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> > +  gdb_byte buf[OR1K_INSTLEN];
> > +
> > +  if (target_read_memory (addr, buf, OR1K_INSTLEN))
> > +    {
> > +      memory_error (TARGET_XFER_E_IO, addr);
> > +    }
> > +
> 
> No need for curly braces for single-statement conditional blocks.

Alright, I missed that part of the gnu style spec. I have been looking
at the formatting guide [0].  Is there something better or is there a
way to get emacs (or something else) to validate everything including
comments and spurious newlines?

I have gone through again and fixed up a few more comments, curly braces
around single statements and spurious newlines.

[0] https://www.gnu.org/prep/standards/html_node/Formatting.html#Formatting

> > +  return extract_unsigned_integer (buf, OR1K_INSTLEN, byte_order);
> > +
> > +}
> > +
> > +
> 
> spurious newline.

Fixed.

> > +/* Generic function to read bits from an instruction.  */
> > +
> > +static int
> > +or1k_analyse_inst (uint32_t inst, const char *format, ...)
> > +{
> > +  /* Break out each field in turn, validating as we go.  */
> > +  va_list ap;
> > +
> > +  int i;
> > +  int iptr = 0;			/* Instruction pointer */
> > +
> > +  va_start (ap, format);
> > +
> > +  for (i = 0; 0 != format[i];)
> > +    {
> > +      const char *start_ptr;
> > +      char *end_ptr;
> > +
> > +      uint32_t bits;		/* Bit substring of interest */
> > +      uint32_t width;		/* Substring width */
> > +      uint32_t *arg_ptr;
> > +
> > +      switch (format[i])
> > +	{
> > +	case ' ':
> > +	  i++;
> > +	  break;		/* Formatting: ignored */
> > +
> > +	case '0':
> > +	case '1':		/* Constant bit field */
> > +	  bits = (inst >> (OR1K_INSTBITLEN - iptr - 1)) & 0x1;
> > +
> > +	  if ((format[i] - '0') != bits)
> > +	    {
> > +	      return 0;
> > +	    }
> 
> No need for curly braces. Possibly more occurrences of this.
> 
> Also, the identation seems a bit off. In special for some of the comments.

Are you meaning the right side comments should be aligned? I think the
code indentation looked alright for the most part.

> > +
> > +	  iptr++;
> > +	  i++;
> > +	  break;
> > +
> > +	case '%':		/* Bit field */
> > +	  i++;
> > +	  start_ptr = &(format[i]);
> > +	  width = strtoul (start_ptr, &end_ptr, 10);
> > +
> > +	  /* Check we got something, and if so skip on */
> 
> period and two spaces after end of comment. More occurrences of this
> throughout.

I will fix this one and a few more which are sentences.  For short (label)
comments I have left them without the period.

Let me know if there is an issue with this.  There is not much mentioned
about this (short comments) in the style guide [1]

[1] https://www.gnu.org/prep/standards/html_node/Comments.html#Comments

> > +	  if (start_ptr == end_ptr)
> > +	    {
> > +	      throw_quit
> > +		("bitstring \"%s\" at offset %d has no length field.\n",
> > +		 format, i);
> > +	    }
> > +
> > +	  i += end_ptr - start_ptr;
> > +
> > +	  /* Look for and skip the terminating 'b'. If it's not there, we
> > +	     still give a fatal error, because these are fixed strings that
> > +	     just should not be wrong.  */
> 
> Two spaces after period. More occurrences throughout.

This one does have 2 spaces after period.  Am I missing something? Or do
you mean within the comment (... 'b'._If it's ...)?

> > +	  if ('b' != format[i++])
> > +	    {
> > +	      throw_quit
> > +		("bitstring \"%s\" at offset %d has no terminating 'b'.\n",
> > +		 format, i);
> > +	    }
> > +
> > +	  /* Break out the field. There is a special case with a bit width of
> > +	     32.  */
> > +	  if (32 == width)
> > +	    {
> > +	      bits = inst;
> > +	    }
> > +	  else
> > +	    {
> > +	      bits =
> > +		(inst >> (OR1K_INSTBITLEN - iptr - width)) & ((1 << width) -
> > +							      1);
> > +	    }
> > +
> > +	  arg_ptr = va_arg (ap, uint32_t *);
> > +	  *arg_ptr = bits;
> > +	  iptr += width;
> > +	  break;
> > +
> > +	default:
> > +	  throw_quit ("invalid character in bitstring \"%s\" at offset %d.\n",
> > +		      format, i);
> > +	  break;
> > +	}
> > +    }
> > +
> > +  /* Is the length OK? */
> > +  gdb_assert (OR1K_INSTBITLEN == iptr);
> > +
> > +  return 1; /* We succeeded */
> > +
> 
> Spurious newline. Possibly more occurrences throughout

Fixed, it seems the original author's style was to put a newline after
the return statement.  Fixed.

> > +}
> > +
> > +
> > +/* Analyse a l.addi instruction in form: l.addi  rD,rA,I. This is used
> > +   to parse add instructions during various prologue analysis routines.  */
> > +
> > +static int
> > +or1k_analyse_l_addi (uint32_t inst,
> > +		     unsigned int *rd_ptr,
> > +		     unsigned int *ra_ptr, int *simm_ptr)
> > +{
> > +  /* Instruction fields */
> > +  uint32_t rd, ra, i;
> > +
> > +  if (or1k_analyse_inst (inst, "10 0111 %5b %5b %16b", &rd, &ra, &i))
> > +    {
> > +      /* Found it. Construct the result fields.  */
> > +      *rd_ptr = (unsigned int) rd;
> > +      *ra_ptr = (unsigned int) ra;
> 
> Do we need these explicit cast or can we use types that make the casts go
> away?

I had a look, its going to be a bit of work to fix this. The return
values ra/rd/i are derived from the input uint32_t so they retain that
type.  It would require either changing the instruction type, or
changing the types of rd_ptr/ra_ptr which might require casts in
less convenient places.

Will spend some more time to see what can be done.  If you have a quick
idea let me know.

> > +      *simm_ptr = (int) (((i & 0x8000) == 0x8000) ? 0xffff0000 | i : i);
> > +
> > +      return 1;		/* Success */
> > +    }
> > +  else
> > +    {
> > +      return 0;		/* Failure */
> > +    }
> > +}
> > +
> > +
> > +/* Analyse a l.sw instruction in form: l.sw  I(rA),rB. This is used to
> > +   to parse store instructions during various prologue analysis routines.  */
> > +
> > +static int
> > +or1k_analyse_l_sw (uint32_t inst,
> > +		   int *simm_ptr, unsigned int *ra_ptr, unsigned int *rb_ptr)
> > +{
> > +  /* Instruction fields */
> > +  uint32_t ihi, ilo, ra, rb;
> > +
> > +  if (or1k_analyse_inst (inst, "11 0101 %5b %5b %5b %11b", &ihi, &ra, &rb,
> > +			 &ilo))
> > +
> > +    {
> > +      /* Found it. Construct the result fields.  */
> > +      *simm_ptr = (int) ((ihi << 11) | ilo);
> > +      *simm_ptr |= ((ihi & 0x10) == 0x10) ? 0xffff0000 : 0;
> > +
> > +      *ra_ptr = (unsigned int) ra;
> > +      *rb_ptr = (unsigned int) rb;
> > +
> > +      return 1;		/* Success */
> > +    }
> > +  else
> > +    {
> > +      return 0;		/* Failure */
> > +    }
> > +}
> > +\f
> > +
> > +/* Functions defining the architecture.  */
> > +
> > +/* Implement the return_value gdbarch method.  */
> > +
> > +static enum return_value_convention
> > +or1k_return_value (struct gdbarch *gdbarch, struct value *functype,
> > +		   struct type *valtype, struct regcache *regcache,
> > +		   gdb_byte *readbuf, const gdb_byte *writebuf)
> > +{
> > +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> > +  enum type_code rv_type = TYPE_CODE (valtype);
> > +  unsigned int rv_size = TYPE_LENGTH (valtype);
> > +  unsigned int bpw = (gdbarch_tdep (gdbarch))->bytes_per_word;
> > +
> > +  /* Deal with struct/union as addresses. If an array won't fit in a single
> > +     register it is returned as address. Anything larger than 2 registers needs
> > +     to also be passed as address (matches gcc default_return_in_memory).  */
> > +  if ((TYPE_CODE_STRUCT == rv_type) || (TYPE_CODE_UNION == rv_type)
> > +      || ((TYPE_CODE_ARRAY == rv_type) && (rv_size > bpw))
> > +      || (rv_size > 2 * bpw))
> > +    {
> > +      if (readbuf != NULL)
> > +	{
> > +	  ULONGEST tmp;
> > +
> > +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp);
> > +	  read_memory (tmp, readbuf, rv_size);
> > +	}
> > +      if (writebuf != NULL)
> > +	{
> > +	  ULONGEST tmp;
> > +
> > +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp);
> > +	  write_memory (tmp, writebuf, rv_size);
> > +	}
> > +
> > +      return RETURN_VALUE_ABI_RETURNS_ADDRESS;
> > +    }
> > +
> > +  if (rv_size <= bpw)
> > +    {
> > +      /* Up to one word scalars are returned in R11.  */
> > +      if (readbuf != NULL)
> > +	{
> > +	  ULONGEST tmp;
> > +
> > +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp);
> > +	  store_unsigned_integer (readbuf, rv_size, byte_order, tmp);
> > +
> > +	}
> > +      if (writebuf != NULL)
> > +	{
> > +	  gdb_byte buf[4];
> > +	  memset (buf, 0, sizeof (buf)); /* Zero pad if < bpw bytes */
> > +
> > +	  if (BFD_ENDIAN_BIG == byte_order)
> > +	    {
> > +	      memcpy (buf + sizeof (buf) - rv_size, writebuf, rv_size);
> > +	    }
> > +	  else
> > +	    {
> > +	      memcpy (buf, writebuf, rv_size);
> > +	    }
> > +
> > +	  regcache_cooked_write (regcache, OR1K_RV_REGNUM, buf);
> > +	}
> > +    }
> > +  else
> > +    {
> > +      /* 2 word scalars are returned in r11/r12 (with the MS word in r11).  */
> > +      if (readbuf != NULL)
> > +	{
> > +	  ULONGEST tmp_lo;
> > +	  ULONGEST tmp_hi;
> > +	  ULONGEST tmp;
> > +
> > +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp_hi);
> > +	  regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM + 1,
> > +					 &tmp_lo);
> > +	  tmp = (tmp_hi << (bpw * 8)) | tmp_lo;
> > +
> > +	  store_unsigned_integer (readbuf, rv_size, byte_order, tmp);
> > +	}
> > +      if (writebuf != NULL)
> > +	{
> > +	  gdb_byte buf_lo[4];
> > +	  gdb_byte buf_hi[4];
> > +
> > +	  memset (buf_lo, 0, sizeof (buf_lo));	/* Zero pad if < bpw bytes */
> > +	  memset (buf_hi, 0, sizeof (buf_hi));	/* Zero pad if < bpw bytes */
> > +
> > +	  /* This is cheating. We assume that we fit in 2 words exactly, which
> > +	     wouldn't work if we had (say) a 6-byte scalar type on a big
> > +	     endian architecture (with the OpenRISC 1000 usually is).  */
> > +	  memcpy (buf_hi, writebuf, rv_size - bpw);
> > +	  memcpy (buf_lo, writebuf + bpw, bpw);
> > +
> > +	  regcache_cooked_write (regcache, OR1K_RV_REGNUM, buf_hi);
> > +	  regcache_cooked_write (regcache, OR1K_RV_REGNUM + 1, buf_lo);
> > +	}
> > +    }
> > +
> > +  return RETURN_VALUE_REGISTER_CONVENTION;
> > +
> > +}
> > +
> > +/* OR1K always uses a l.trap instruction for breakpoints.  */
> > +
> > +constexpr gdb_byte or1k_break_insn[] = {0x21, 0x00, 0x00, 0x01};
> > +
> > +typedef BP_MANIPULATION (or1k_break_insn) or1k_breakpoint;
> > +
> > +/* Implement the single_step_through_delay gdbarch method  */
> > +
> > +static int
> > +or1k_single_step_through_delay (struct gdbarch *gdbarch,
> > +				struct frame_info *this_frame)
> > +{
> > +  ULONGEST val;
> > +  CORE_ADDR ppc;
> > +  CORE_ADDR npc;
> > +  CGEN_FIELDS tmp_fields;
> > +  const CGEN_INSN *insns;
> > +  struct regcache *regcache = get_current_regcache ();
> > +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> > +
> > +  /* Get and the previous and current instruction addresses. If they are not
> > +     adjacent, we cannot be in a delay slot. */
> > +  regcache_cooked_read_unsigned (regcache, OR1K_PPC_REGNUM, &val);
> > +  ppc = (CORE_ADDR) val;
> > +  regcache_cooked_read_unsigned (regcache, OR1K_NPC_REGNUM, &val);
> > +  npc = (CORE_ADDR) val;
> > +
> > +  if (0x4 != (npc - ppc))
> > +    {
> > +      return 0;
> > +    }
> > +
> > +  insns = cgen_lookup_insn (tdep->gdb_cgen_cpu_desc,
> > +			    NULL,
> > +			    or1k_fetch_instruction (gdbarch, ppc),
> > +			    NULL, 32, &tmp_fields, 0);
> > +
> > +  /* TODO: we should add a delay slot flag to the CGEN_INSN and remove
> > +   * this hard coded test. */
> > +  return ((CGEN_INSN_NUM (insns) == OR1K_INSN_L_J)
> > +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_JAL)
> > +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_JR)
> > +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_JALR)
> > +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_BNF)
> > +	  || (CGEN_INSN_NUM (insns) == OR1K_INSN_L_BF));
> > +
> > +}
> > +
> > +/* Name for or1k general registers.  */
> > +
> > +static const char *const or1k_reg_names[OR1K_NUM_REGS] = {
> > +  /* general purpose registers */
> > +  "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
> > +  "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
> > +  "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
> > +  "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
> > +
> > +  /* previous program counter, next program counter and status register */
> > +  "ppc", "npc", "sr"
> > +};
> > +
> > +/* Implement the register_name gdbarch method.  */
> > +
> > +static const char *
> > +or1k_register_name (struct gdbarch *gdbarch, int regnum)
> > +{
> > +  if (0 <= regnum && regnum < OR1K_NUM_REGS)
> > +    {
> > +      return or1k_reg_names[regnum];
> > +    }
> > +  else
> > +      return NULL;
> > +}
> > +
> > +/* Implement the register_type gdbarch method.  */
> > +
> > +static struct type *
> > +or1k_register_type (struct gdbarch *gdbarch, int regnum)
> > +{
> > +  if ((regnum >= 0) && (regnum < OR1K_NUM_REGS))
> > +    {
> > +      switch (regnum)
> > +	{
> > +	case OR1K_PPC_REGNUM:
> > +	case OR1K_NPC_REGNUM:
> > +	  return builtin_type (gdbarch)->builtin_func_ptr;	/* Pointer to code */
> > +
> > +	case OR1K_SP_REGNUM:
> > +	case OR1K_FP_REGNUM:
> > +	  return builtin_type (gdbarch)->builtin_data_ptr;	/* Pointer to data */
> > +
> > +	default:
> > +	  return builtin_type (gdbarch)->builtin_uint32;	/* Data */
> > +	}
> > +    }
> > +
> > +  internal_error (__FILE__, __LINE__,
> > +		  _("or1k_register_type: illegal register number %d"),
> > +		  regnum);
> > +
> > +}
> > +
> > +/* Implement the register_reggroup_p gdbarch method.  */
> > +
> > +static int
> > +or1k_register_reggroup_p (struct gdbarch *gdbarch,
> > +			  int regnum, struct reggroup *group)
> > +{
> > +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> > +
> > +  /* All register group */
> > +  if (group == all_reggroup)
> > +    {
> > +      return ((regnum >= 0)
> > +	      && (regnum < OR1K_NUM_REGS)
> > +	      && (or1k_register_name (gdbarch, regnum)[0] != '\0'));
> > +    }
> > +
> > +  /* For now everything except the PC */
> > +  if (group == general_reggroup)
> > +    {
> > +      return ((regnum >= OR1K_ZERO_REGNUM)
> > +	      && (regnum < OR1K_MAX_GPR_REGS)
> > +	      && (regnum != OR1K_PPC_REGNUM) && (regnum != OR1K_NPC_REGNUM));
> > +    }
> > +
> > +  if (group == float_reggroup)
> > +    {
> > +      return 0;	/* No float regs.  */
> > +    }
> > +
> > +  if (group == vector_reggroup)
> > +    {
> > +      return 0;	/* No vector regs.  */
> > +    }
> > +
> > +  /* For any that are not handled above.  */
> > +  return default_register_reggroup_p (gdbarch, regnum, group);
> > +
> > +}
> > +
> > +
> > +static int
> > +or1k_is_arg_reg (unsigned int regnum)
> > +{
> > +  return (OR1K_FIRST_ARG_REGNUM <= regnum)
> > +    && (regnum <= OR1K_LAST_ARG_REGNUM);
> > +
> > +}
> > +
> > +
> > +static int
> > +or1k_is_callee_saved_reg (unsigned int regnum)
> > +{
> > +  return (OR1K_FIRST_SAVED_REGNUM <= regnum) && (0 == regnum % 2);
> > +
> > +}
> > +
> > +
> > +/* Implement the skip_prologue gdbarch method.  */
> > +
> > +static CORE_ADDR
> > +or1k_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
> > +{
> > +  CORE_ADDR start_pc;
> > +  CORE_ADDR addr;
> > +  uint32_t inst;
> > +
> > +  unsigned int ra, rb, rd; /* for instruction analysis */
> > +  int simm;
> > +
> > +  int frame_size = 0;
> > +
> > +  /* Try using SAL first if we have symbolic information available. This only
> > +     works for DWARF 2, not STABS.  */
> > +
> > +  if (find_pc_partial_function (pc, NULL, &start_pc, NULL))
> > +    {
> > +      CORE_ADDR prologue_end = skip_prologue_using_sal (gdbarch, pc);
> > +
> > +      if (0 != prologue_end)
> > +	{
> > +	  struct symtab_and_line prologue_sal = find_pc_line (start_pc, 0);
> > +	  struct compunit_symtab *compunit
> > +	    = SYMTAB_COMPUNIT (prologue_sal.symtab);
> > +	  const char *debug_format = COMPUNIT_DEBUGFORMAT (compunit);
> > +
> > +	  if ((NULL != debug_format)
> > +	      && (strlen ("dwarf") <= strlen (debug_format))
> > +	      && (0 == strncasecmp ("dwarf", debug_format, strlen ("dwarf"))))
> > +	    {
> > +	      return (prologue_end > pc) ? prologue_end : pc;
> > +	    }
> > +	}
> > +    }
> > +
> > +  /* Look to see if we can find any of the standard prologue sequence. All
> > +     quite difficult, since any or all of it may be missing. So this is just a
> > +     best guess!  */
> > +
> > +  addr = pc; /* Where we have got to */
> > +  inst = or1k_fetch_instruction (gdbarch, addr);
> > +
> > +  /* Look for the new stack pointer being set up.  */
> > +  if (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
> > +      && (OR1K_SP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
> > +      && (simm < 0) && (0 == (simm % 4)))
> > +    {
> > +      frame_size = -simm;
> > +      addr += OR1K_INSTLEN;
> > +      inst = or1k_fetch_instruction (gdbarch, addr);
> > +    }
> > +
> > +  /* Look for the frame pointer being manipulated.  */
> > +  if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> > +      && (OR1K_SP_REGNUM == ra) && (OR1K_FP_REGNUM == rb)
> > +      && (simm >= 0) && (0 == (simm % 4)))
> > +    {
> > +      addr += OR1K_INSTLEN;
> > +      inst = or1k_fetch_instruction (gdbarch, addr);
> > +
> > +      gdb_assert (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
> > +		  && (OR1K_FP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
> > +		  && (simm == frame_size));
> > +
> > +      addr += OR1K_INSTLEN;
> > +      inst = or1k_fetch_instruction (gdbarch, addr);
> > +    }
> > +
> > +  /* Look for the link register being saved.  */
> > +  if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> > +      && (OR1K_SP_REGNUM == ra) && (OR1K_LR_REGNUM == rb)
> > +      && (simm >= 0) && (0 == (simm % 4)))
> > +    {
> > +      addr += OR1K_INSTLEN;
> > +      inst = or1k_fetch_instruction (gdbarch, addr);
> > +    }
> > +
> > +  /* Look for arguments or callee-saved register being saved. The register
> > +     must be one of the arguments (r3-r8) or the 10 callee saved registers
> > +     (r10, r12, r14, r16, r18, r20, r22, r24, r26, r28, r30). The base
> > +     register must be the FP (for the args) or the SP (for the callee_saved
> > +     registers).  */
> > +  while (1)
> > +    {
> > +      if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> > +	  && (((OR1K_FP_REGNUM == ra) && or1k_is_arg_reg (rb))
> > +	      || ((OR1K_SP_REGNUM == ra) && or1k_is_callee_saved_reg (rb)))
> > +	  && (0 == (simm % 4)))
> > +	{
> > +	  addr += OR1K_INSTLEN;
> > +	  inst = or1k_fetch_instruction (gdbarch, addr);
> > +	}
> > +      else
> > +	{
> > +	  /* Nothing else to look for. We have found the end of the prologue. */
> > +	  return addr;
> > +	}
> > +    }
> > +}
> > +
> > +
> > +/* Implement the frame_align gdbarch method.  */
> > +
> > +static CORE_ADDR
> > +or1k_frame_align (struct gdbarch *gdbarch, CORE_ADDR sp)
> > +{
> > +  return align_down (sp, OR1K_STACK_ALIGN);
> > +
> > +}
> > +
> > +/* Implement the unwind_pc gdbarch method.  */
> > +
> > +static CORE_ADDR
> > +or1k_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame)
> > +{
> > +  CORE_ADDR pc;
> > +
> > +  if (frame_debug)
> > +    {
> > +      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_pc, next_frame=%d\n",
> > +			  frame_relative_level (next_frame));
> > +    }
> > +
> > +  pc = frame_unwind_register_unsigned (next_frame, OR1K_NPC_REGNUM);
> > +
> > +  if (frame_debug)
> > +    {
> > +      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_pc, pc=0x%p\n",
> > +			  (void *) pc);
> > +    }
> > +
> > +  return pc;
> > +
> > +}
> > +
> > +/* Implement the unwind_sp gdbarch method.  */
> > +
> > +static CORE_ADDR
> > +or1k_unwind_sp (struct gdbarch *gdbarch, struct frame_info *next_frame)
> > +{
> > +  CORE_ADDR sp;
> > +
> > +  if (frame_debug)
> > +    {
> > +      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_sp, next_frame=%d\n",
> > +			  frame_relative_level (next_frame));
> > +    }
> > +
> > +  sp = frame_unwind_register_unsigned (next_frame, OR1K_SP_REGNUM);
> > +
> > +  if (frame_debug)
> > +    {
> > +      fprintf_unfiltered (gdb_stdlog, "or1k_unwind_sp, sp=0x%p\n",
> > +			  (void *) sp);
> > +    }
> > +
> > +  return sp;
> > +
> > +}
> > +
> > +/* Implement the push_dummy_code gdbarch method.  */
> > +
> > +static CORE_ADDR
> > +or1k_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
> > +		      CORE_ADDR function, struct value **args, int nargs,
> > +		      struct type *value_type, CORE_ADDR * real_pc,
> > +		      CORE_ADDR * bp_addr, struct regcache *regcache)
> > +{
> > +  CORE_ADDR bp_slot;
> > +
> > +  /* Reserve enough room on the stack for our breakpoint instruction.  */
> > +  bp_slot = sp - 4;
> > +  /* Store the address of that breakpoint.  */
> > +  *bp_addr = bp_slot;
> > +  /* keeping the stack aligned.  */
> > +  sp = or1k_frame_align (gdbarch, bp_slot);
> > +  /* The call starts at the callee's entry point.  */
> > +  *real_pc = function;
> > +
> > +  return sp;
> > +
> > +}
> > +
> > +/* Implement the push_dummy_call gdbarch method.  */
> > +
> > +static CORE_ADDR
> > +or1k_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
> > +		      struct regcache *regcache, CORE_ADDR bp_addr,
> > +		      int nargs, struct value **args, CORE_ADDR sp,
> > +		      int struct_return, CORE_ADDR struct_addr)
> > +{
> > +
> > +  int argreg;
> > +  int argnum;
> > +  int first_stack_arg;
> > +  int stack_offset = 0;
> > +  int heap_offset = 0;
> > +  CORE_ADDR heap_sp = sp - 128;
> > +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> > +  unsigned int bpa = (gdbarch_tdep (gdbarch))->bytes_per_address;
> > +  unsigned int bpw = (gdbarch_tdep (gdbarch))->bytes_per_word;
> > +  struct type *func_type = value_type (function);
> > +
> > +  /* Return address */
> > +  regcache_cooked_write_unsigned (regcache, OR1K_LR_REGNUM, bp_addr);
> > +
> > +  /* Register for the next argument.  */
> > +  argreg = OR1K_FIRST_ARG_REGNUM;
> > +
> > +  /* Location for a returned structure. This is passed as a silent first
> > +     argument.  */
> > +  if (struct_return)
> > +    {
> > +      regcache_cooked_write_unsigned (regcache, OR1K_FIRST_ARG_REGNUM,
> > +				      struct_addr);
> > +      argreg++;
> > +    }
> > +
> > +  /* Put as many args as possible in registers */
> > +  for (argnum = 0; argnum < nargs; argnum++)
> > +    {
> > +      const gdb_byte *val;
> > +      gdb_byte valbuf[sizeof (ULONGEST)];
> > +
> > +      struct value *arg = args[argnum];
> > +      struct type *arg_type = check_typedef (value_type (arg));
> > +      int len = arg_type->length;
> > +      enum type_code typecode = arg_type->main_type->code;
> > +
> > +      if (TYPE_VARARGS (func_type) && argnum >= TYPE_NFIELDS (func_type))
> > +	{
> > +	  break; /* end or regular args, varargs go to stack */
> > +	}
> > +
> > +      /* Extract the value, either a reference or the data */
> > +      if ((TYPE_CODE_STRUCT == typecode) || (TYPE_CODE_UNION == typecode)
> > +	  || (len > bpw * 2))
> > +	{
> > +	  CORE_ADDR valaddr = value_address (arg);
> > +
> > +	  /* if the arg is fabricated (i.e. 3*i, instead of i) valaddr is undefined */
> > +	  if (valaddr == 0)
> > +	    {
> > +	      /* The argument needs to be copied into the target space. Since
> > +	         the bottom of the stack is reserved for function arguments
> > +	         we store this at the these at the top growing down.  */
> > +	      heap_offset += align_up (len, bpw);
> > +	      valaddr = heap_sp + heap_offset;
> > +
> > +	      write_memory (valaddr, value_contents (arg), len);
> > +	    }
> > +
> > +	  /* The ABI passes all structures by reference, so get its address.  */
> > +	  store_unsigned_integer (valbuf, bpa, byte_order, valaddr);
> > +	  len = bpa;
> > +	  val = valbuf;
> > +	}
> > +      else
> > +	{
> > +	  /* Everything else, we just get the value. */
> > +	  val = value_contents (arg);
> > +	}
> > +
> > +      /* Stick the value in a register */
> > +      if (len > bpw)
> > +	{
> > +	  /* Big scalars use two registers, but need NOT be pair aligned.  */
> > +
> > +	  if (argreg <= (OR1K_LAST_ARG_REGNUM - 1))
> > +	    {
> > +	      ULONGEST regval =	extract_unsigned_integer (val, len, byte_order);
> > +
> > +	      unsigned int bits_per_word = bpw * 8;
> > +	      ULONGEST mask = (((ULONGEST) 1) << bits_per_word) - 1;
> > +	      ULONGEST lo = regval & mask;
> > +	      ULONGEST hi = regval >> bits_per_word;
> > +
> > +	      regcache_cooked_write_unsigned (regcache, argreg, hi);
> > +	      regcache_cooked_write_unsigned (regcache, argreg + 1, lo);
> > +	      argreg += 2;
> > +	    }
> > +	  else
> > +	    {
> > +	      /* Run out of regs */
> > +	      break;
> > +	    }
> > +	}
> > +      else if (argreg <= OR1K_LAST_ARG_REGNUM)
> > +	{
> > +	  /* Smaller scalars fit in a single register */
> > +	  regcache_cooked_write_unsigned (regcache, argreg,
> > +					  extract_unsigned_integer (val, len,
> > +								    byte_order));
> > +	  argreg++;
> > +	}
> > +      else
> > +	{
> > +	  /* Run out of regs */
> > +	  break;
> > +	}
> > +    }
> > +
> > +  first_stack_arg = argnum;
> > +
> > +  /* If we get here with argnum < nargs, then arguments remain to be placed on
> > +     the stack. This is tricky, since they must be pushed in reverse order and
> > +     the stack in the end must be aligned. The only solution is to do it in
> > +     two stages, the first to compute the stack size, the second to save the
> > +     args.  */
> > +
> > +  for (argnum = first_stack_arg; argnum < nargs; argnum++)
> > +    {
> > +      struct value *arg = args[argnum];
> > +      struct type *arg_type = check_typedef (value_type (arg));
> > +      int len = arg_type->length;
> > +      enum type_code typecode = arg_type->main_type->code;
> > +
> > +      if ((TYPE_CODE_STRUCT == typecode) || (TYPE_CODE_UNION == typecode)
> > +	  || (len > bpw * 2))
> > +	{
> > +	  /* Structures are passed as addresses */
> > +	  sp -= bpa;
> > +	}
> > +      else
> > +	{
> > +	  /* Big scalars use more than one word. Code here allows for future
> > +	     quad-word entities (e.g. long double) */
> > +	  sp -= align_up (len, bpw);
> > +	}
> > +
> > +      /* ensure our dummy heap doesn't touch the stack, this could only happen
> > +         if we have many arguments including fabricated arguments */
> > +      gdb_assert (heap_offset == 0 || ((heap_sp + heap_offset) < sp));
> > +    }
> > +
> > +  sp = gdbarch_frame_align (gdbarch, sp);
> > +  stack_offset = 0;
> > +
> > +  /* Push the remaining args on the stack */
> > +  for (argnum = first_stack_arg; argnum < nargs; argnum++)
> > +    {
> > +      const gdb_byte *val;
> > +      gdb_byte valbuf[sizeof (ULONGEST)];
> > +
> > +      struct value *arg = args[argnum];
> > +      struct type *arg_type = check_typedef (value_type (arg));
> > +      int len = arg_type->length;
> > +      enum type_code typecode = arg_type->main_type->code;
> > +      /* The EABI passes structures that do not fit in a register by
> > +         reference. In all other cases, pass the structure by value.  */
> > +      if ((TYPE_CODE_STRUCT == typecode) || (TYPE_CODE_UNION == typecode)
> > +	  || (len > bpw * 2))
> > +	{
> > +	  store_unsigned_integer (valbuf, bpa, byte_order,
> > +				  value_address (arg));
> > +	  len = bpa;
> > +	  val = valbuf;
> > +	}
> > +      else
> > +	{
> > +	  val = value_contents (arg);
> > +	}
> > +
> > +      while (len > 0)
> > +	{
> > +	  int partial_len = (len < bpw ? len : bpw);
> > +
> > +	  write_memory (sp + stack_offset, val, partial_len);
> > +	  stack_offset += align_up (partial_len, bpw);
> > +	  len -= partial_len;
> > +	  val += partial_len;
> > +	}
> > +    }
> > +
> > +  /* Save the updated stack pointer */
> > +  regcache_cooked_write_unsigned (regcache, OR1K_SP_REGNUM, sp);
> > +
> > +  if (heap_offset > 0)
> > +    {
> > +      sp = heap_sp;
> > +    }
> > +
> > +  return sp;
> > +
> > +}
> > +
> > +
> > +/* Implement the dummy_id gdbarch method.  */
> > +
> > +static struct frame_id
> > +or1k_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame)
> > +{
> > +  return frame_id_build (get_frame_sp (this_frame),
> > +			 get_frame_pc (this_frame));
> > +
> > +}
> > +\f
> > +
> > +
> > +
> > +/* Support functions for frame handling */
> > +
> > +/* Initialize a prologue cache
> > +
> > +   We build a cache, saying where registers of the PREV frame can be found
> > +   from the data so far set up in this THIS.
> > +
> > +   We also compute a unique ID for this frame, based on the function start
> > +   address and the stack pointer (as it will be, even if it has yet to be
> > +   computed.
> > +
> > +   STACK FORMAT
> > +   ============
> > +
> > +   The OR1K has a falling stack frame and a simple prolog. The Stack pointer
> > +   is R1 and the frame pointer R2. The frame base is therefore the address
> > +   held in R2 and the stack pointer (R1) is the frame base of the NEXT frame.
> > +
> > +   l.addi  r1,r1,-frame_size	# SP now points to end of new stack frame
> > +
> > +   The stack pointer may not be set up in a frameless function (e.g. a simple
> > +   leaf function).
> > +
> > +   l.sw    fp_loc(r1),r2        # old FP saved in new stack frame
> > +   l.addi  r2,r1,frame_size     # FP now points to base of new stack frame
> > +
> > +   The frame pointer is not necessarily saved right at the end of the stack
> > +   frame - OR1K saves enough space for any args to called functions right at
> > +   the end (this is a difference from the Architecture Manual).
> > +
> > +   l.sw    lr_loc(r1),r9        # Link (return) address
> > +
> > +   The link register is usally saved at fp_loc - 4. It may not be saved at all
> > +   in a leaf function.
> > +
> > +   l.sw    reg_loc(r1),ry       # Save any callee saved regs
> > +
> > +   The offsets x for the callee saved registers generally (always?) rise in
> > +   increments of 4, starting at fp_loc + 4. If the frame pointer is omitted
> > +   (an option to GCC), then it may not be saved at all. There may be no callee
> > +   saved registers.
> > +
> > +   So in summary none of this may be present. However what is present seems
> > +   always to follow this fixed order, and occur before any substantive code
> > +   (it is possible for GCC to have more flexible scheduling of the prologue,
> > +   but this does not seem to occur for OR1K).
> > +
> > +   ANALYSIS
> > +   ========
> > +
> > +   This prolog is used, even for -O3 with GCC.
> > +
> > +   All this analysis must allow for the possibility that the PC is in the
> > +   middle of the prologue. Data in the cache should only be set up insofar as
> > +   it has been computed.
> > +
> > +   HOWEVER. The frame_id must be created with the SP *as it will be* at the
> > +   end of the Prologue. Otherwise a recursive call, checking the frame with
> > +   the PC at the start address will end up with the same frame_id as the
> > +   caller.
> > +
> > +   A suite of "helper" routines are used, allowing reuse for
> > +   or1k_skip_prologue().
> > +
> > +   Reportedly, this is only valid for frames less than 0x7fff in size.  */
> > +
> > +static struct trad_frame_cache *
> > +or1k_frame_cache (struct frame_info *this_frame, void **prologue_cache)
> > +{
> > +  struct gdbarch *gdbarch;
> > +  struct trad_frame_cache *info;
> > +
> > +  CORE_ADDR this_pc;
> > +  CORE_ADDR this_sp;
> > +  CORE_ADDR this_sp_for_id;
> > +  int frame_size = 0;
> > +
> > +  CORE_ADDR start_addr;
> > +  CORE_ADDR end_addr;
> > +
> > +  if (frame_debug)
> > +    {
> > +      fprintf_unfiltered (gdb_stdlog,
> > +			  "or1k_frame_cache, prologue_cache = 0x%p\n",
> > +			  *prologue_cache);
> > +    }
> > +
> > +  /* Nothing to do if we already have this info.  */
> > +  if (NULL != *prologue_cache)
> > +    {
> > +      return (struct trad_frame_cache *) *prologue_cache;
> > +    }
> > +
> > +  /* Get a new prologue cache and populate it with default values.  */
> > +  info = trad_frame_cache_zalloc (this_frame);
> > +  *prologue_cache = info;
> > +
> > +  /* Find the start address of THIS function (which is a NORMAL frame, even if
> > +     the NEXT frame is the sentinel frame) and the end of its prologue.  */
> > +  this_pc = get_frame_pc (this_frame);
> > +  find_pc_partial_function (this_pc, NULL, &start_addr, NULL);
> > +
> > +  /* Get the stack pointer if we have one (if there's no process executing yet
> > +     we won't have a frame.  */
> > +  this_sp = (NULL == this_frame) ? 0 :
> > +    get_frame_register_unsigned (this_frame, OR1K_SP_REGNUM);
> > +
> > +  /* Return early if GDB couldn't find the function.  */
> > +  if (start_addr == 0)
> > +    {
> > +      if (frame_debug)
> > +	{
> > +	  fprintf_unfiltered (gdb_stdlog, "  couldn't find function\n");
> > +	}
> > +
> > +      /* JPB: 28-Apr-11. This is a temporary patch, to get round GDB crashing
> > +         right at the beginning. Build the frame ID as best we can. */
> > +      trad_frame_set_id (info, frame_id_build (this_sp, this_pc));
> > +
> > +      return info;
> > +    }
> > +
> > +
> > +  /* The default frame base of THIS frame (for ID purposes only - frame base
> > +     is an overloaded term) is its stack pointer. For now we use the value of
> > +     the SP register in THIS frame. However if the PC is in the prologue of
> > +     THIS frame, before the SP has been set up, then the value will actually
> > +     be that of the PREV frame, and we'll need to adjust it later. */
> > +  trad_frame_set_this_base (info, this_sp);
> > +  this_sp_for_id = this_sp;
> > +
> > +  /* The default is to find the PC of the PREVIOUS frame in the link register
> > +     of this frame. This may be changed if we find the link register was saved
> > +     on the stack. */
> > +  trad_frame_set_reg_realreg (info, OR1K_NPC_REGNUM, OR1K_LR_REGNUM);
> > +
> > +  /* We should only examine code that is in the prologue. This is all code up
> > +     to (but not including) end_addr. We should only populate the cache while
> > +     the address is up to (but not including) the PC or end_addr, whichever is
> > +     first. */
> > +  gdbarch = get_frame_arch (this_frame);
> > +  end_addr = or1k_skip_prologue (gdbarch, start_addr);
> > +
> > +  /* All the following analysis only occurs if we are in the prologue and have
> > +     executed the code. Check we have a sane prologue size, and if zero we
> > +     are frameless and can give up here. */
> > +  if (end_addr < start_addr)
> > +    {
> > +      throw_quit ("end addr 0x%08x is less than start addr 0x%08x\n",
> > +		  (unsigned int) end_addr, (unsigned int) start_addr);
> > +    }
> > +
> > +  if (end_addr == start_addr)
> > +    {
> > +      frame_size = 0;
> > +    }
> > +  else
> > +    {
> > +      /* have a frame. Look for the various components */
> > +      CORE_ADDR addr = start_addr;	/* Where we have got to */
> > +      uint32_t inst = or1k_fetch_instruction (gdbarch, addr);
> > +
> > +      unsigned int ra, rb, rd;	/* For instruction analysis */
> > +      int simm;
> > +
> > +      /* Look for the new stack pointer being set up. */
> > +      if (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
> > +	  && (OR1K_SP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
> > +	  && (simm < 0) && (0 == (simm % 4)))
> > +	{
> > +	  frame_size = -simm;
> > +	  addr += OR1K_INSTLEN;
> > +	  inst = or1k_fetch_instruction (gdbarch, addr);
> > +
> > +	  /* If the PC has not actually got to this point, then the frame base
> > +	     will be wrong, and we adjust it.
> > +
> > +	     If we are past this point, then we need to populate the stack
> > +	     accoringly. */
> > +	  if (this_pc <= addr)
> > +	    {
> > +	      /* Only do if executing */
> > +	      if (0 != this_sp)
> > +		{
> > +		  this_sp_for_id = this_sp + frame_size;
> > +		  trad_frame_set_this_base (info, this_sp_for_id);
> > +		}
> > +	    }
> > +	  else
> > +	    {
> > +	      /* We are past this point, so the stack pointer of the PREV
> > +	         frame is frame_size greater than the stack pointer of THIS
> > +	         frame. */
> > +	      trad_frame_set_reg_value (info, OR1K_SP_REGNUM,
> > +					this_sp + frame_size);
> > +	    }
> > +	}
> > +
> > +      /* From now on we are only populating the cache, so we stop once we get
> > +         to either the end OR the current PC. */
> > +      end_addr = (this_pc < end_addr) ? this_pc : end_addr;
> > +
> > +      /* Look for the frame pointer being manipulated. */
> > +      if ((addr < end_addr)
> > +	  && or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> > +	  && (OR1K_SP_REGNUM == ra) && (OR1K_FP_REGNUM == rb)
> > +	  && (simm >= 0) && (0 == (simm % 4)))
> > +	{
> > +	  addr += OR1K_INSTLEN;
> > +	  inst = or1k_fetch_instruction (gdbarch, addr);
> > +
> > +	  /* At this stage, we can find the frame pointer of the PREVIOUS
> > +	     frame on the stack of the current frame. */
> > +	  trad_frame_set_reg_addr (info, OR1K_FP_REGNUM, this_sp + simm);
> > +
> > +	  /* Look for the new frame pointer being set up */
> > +	  if (addr < end_addr)
> > +	    {
> > +	      gdb_assert (or1k_analyse_l_addi (inst, &rd, &ra, &simm)
> > +			  && (OR1K_FP_REGNUM == rd) && (OR1K_SP_REGNUM == ra)
> > +			  && (simm == frame_size));
> > +
> > +	      addr += OR1K_INSTLEN;
> > +	      inst = or1k_fetch_instruction (gdbarch, addr);
> > +
> > +	      /* If we have got this far, the stack pointer of the PREVIOUS
> > +	         frame is the frame pointer of THIS frame. */
> > +	      trad_frame_set_reg_realreg (info, OR1K_SP_REGNUM,
> > +					  OR1K_FP_REGNUM);
> > +	    }
> > +	}
> > +
> > +      /* Look for the link register being saved */
> > +      if ((addr < end_addr)
> > +	  && or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> > +	  && (OR1K_SP_REGNUM == ra) && (OR1K_LR_REGNUM == rb)
> > +	  && (simm >= 0) && (0 == (simm % 4)))
> > +	{
> > +	  addr += OR1K_INSTLEN;
> > +	  inst = or1k_fetch_instruction (gdbarch, addr);
> > +
> > +	  /* If the link register is saved in the THIS frame, it holds the
> > +	     value of the PC in the PREVIOUS frame. This overwrites the
> > +	     previous information about finding the PC in the link
> > +	     register.  */
> > +	  trad_frame_set_reg_addr (info, OR1K_NPC_REGNUM, this_sp + simm);
> > +	}
> > +
> > +      /* Look for arguments or callee-saved register being saved. The register
> > +         must be one of the arguments (r3-r8) or the 10 callee saved registers
> > +         (r10, r12, r14, r16, r18, r20, r22, r24, r26, r28, r30). The base
> > +         register must be the FP (for the args) or the SP (for the
> > +         callee_saved registers).  */
> > +      while (addr < end_addr)
> > +	{
> > +	  if (or1k_analyse_l_sw (inst, &simm, &ra, &rb)
> > +	      && (((OR1K_FP_REGNUM == ra) && or1k_is_arg_reg (rb))
> > +		  || ((OR1K_SP_REGNUM == ra)
> > +		      && or1k_is_callee_saved_reg (rb))) && (0 == (simm % 4)))
> > +	    {
> > +	      addr += OR1K_INSTLEN;
> > +	      inst = or1k_fetch_instruction (gdbarch, addr);
> > +
> > +	      /* The register in the PREVIOUS frame can be found at this
> > +	         location in THIS frame */
> > +	      trad_frame_set_reg_addr (info, rb, this_sp + simm);
> > +	    }
> > +	  else
> > +	    {
> > +	      break;		/* Not a register save instruction */
> > +	    }
> > +	}
> > +    }
> > +
> > +  /* Build the frame ID */
> > +  trad_frame_set_id (info, frame_id_build (this_sp_for_id, start_addr));
> > +
> > +  if (frame_debug)
> > +    {
> > +      fprintf_unfiltered (gdb_stdlog, "  this_sp_for_id = 0x%p\n",
> > +			  (void *) this_sp_for_id);
> > +      fprintf_unfiltered (gdb_stdlog, "  start_addr     = 0x%p\n",
> > +			  (void *) start_addr);
> > +    }
> > +
> > +  return info;
> > +
> > +}
> > +
> > +
> > +/* Implement the this_id function for the stub unwinder.  */
> > +
> > +static void
> > +or1k_frame_this_id (struct frame_info *this_frame,
> > +		    void **prologue_cache, struct frame_id *this_id)
> > +{
> > +  struct trad_frame_cache *info = or1k_frame_cache (this_frame,
> > +						    prologue_cache);
> > +
> > +  trad_frame_get_id (info, this_id);
> > +
> > +}
> > +
> > +
> > +/* Implement the prev_register function for the stub unwinder.  */
> > +
> > +static struct value *
> > +or1k_frame_prev_register (struct frame_info *this_frame,
> > +			  void **prologue_cache, int regnum)
> > +{
> > +  struct trad_frame_cache *info = or1k_frame_cache (this_frame,
> > +						    prologue_cache);
> > +
> > +  return trad_frame_get_register (info, this_frame, regnum);
> > +
> > +}
> > +
> > +/* Data structures for the normal prologue-analysis-based
> > +   unwinder.  */
> > +
> > +static const struct frame_unwind or1k_frame_unwind = {
> > +  .type = NORMAL_FRAME,
> > +  .stop_reason = default_frame_unwind_stop_reason,
> > +  .this_id = or1k_frame_this_id,
> > +  .prev_register = or1k_frame_prev_register,
> > +  .unwind_data = NULL,
> > +  .sniffer = default_frame_sniffer,
> > +  .dealloc_cache = NULL,
> > +  .prev_arch = NULL
> > +};
> > +
> > +/* Architecture initialization for OpenRISC 1000.  */
> > +
> > +static struct gdbarch *
> > +or1k_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> > +{
> > +  static struct frame_base or1k_frame_base;
> > +  struct gdbarch *gdbarch;
> > +  struct gdbarch_tdep *tdep;
> > +  const struct bfd_arch_info *binfo;
> > +  struct tdesc_arch_data *tdesc_data = NULL;
> > +
> > +  int i;
> > +  int reg_index = 0;
> > +  int retval;
> > +  int group;
> > +
> > +  /* Find a candidate among the list of pre-declared architectures.  */
> > +  arches = gdbarch_list_lookup_by_info (arches, &info);
> > +  if (NULL != arches)
> > +    {
> > +      return arches->gdbarch;
> > +    }
> > +
> > +  /* None found, create a new architecture from the information
> > +     provided. Can't initialize all the target dependencies until we actually
> > +     know which target we are talking to, but put in some defaults for now. */
> > +
> > +  binfo = info.bfd_arch_info;
> > +  tdep = XNEW (struct gdbarch_tdep);
> 
> XCNEW may be better suited since it zeroes out all fields?

Right, I will use it.
 
> > +  tdep->bytes_per_word = binfo->bits_per_word / binfo->bits_per_byte;
> > +  tdep->bytes_per_address = binfo->bits_per_address / binfo->bits_per_byte;
> > +  gdbarch = gdbarch_alloc (&info, tdep);
> > +
> > +  /* Target data types.  */
> > +  set_gdbarch_short_bit (gdbarch, 16);
> > +  set_gdbarch_int_bit (gdbarch, 32);
> > +  set_gdbarch_long_bit (gdbarch, 32);
> > +  set_gdbarch_long_long_bit (gdbarch, 64);
> > +  set_gdbarch_float_bit (gdbarch, 32);
> > +  set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
> > +  set_gdbarch_double_bit (gdbarch, 64);
> > +  set_gdbarch_double_format (gdbarch, floatformats_ieee_double);
> > +  set_gdbarch_long_double_bit (gdbarch, 64);
> > +  set_gdbarch_long_double_format (gdbarch, floatformats_ieee_double);
> > +  set_gdbarch_ptr_bit (gdbarch, binfo->bits_per_address);
> > +  set_gdbarch_addr_bit (gdbarch, binfo->bits_per_address);
> > +  set_gdbarch_char_signed (gdbarch, 1);
> > +
> > +  /* Information about the target architecture */
> > +  set_gdbarch_return_value (gdbarch, or1k_return_value);
> > +  set_gdbarch_breakpoint_kind_from_pc (gdbarch, or1k_breakpoint::kind_from_pc);
> > +  set_gdbarch_sw_breakpoint_from_kind (gdbarch, or1k_breakpoint::bp_from_kind);
> > +  set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
> > +
> > +  set_gdbarch_print_insn (gdbarch, print_insn_or1k);
> > +
> > +  /* Register architecture */
> > +  set_gdbarch_num_regs (gdbarch, OR1K_NUM_REGS);
> > +  set_gdbarch_num_pseudo_regs (gdbarch, OR1K_NUM_PSEUDO_REGS);
> > +  set_gdbarch_sp_regnum (gdbarch, OR1K_SP_REGNUM);
> > +  set_gdbarch_pc_regnum (gdbarch, OR1K_NPC_REGNUM);
> > +  set_gdbarch_ps_regnum (gdbarch, OR1K_SR_REGNUM);
> > +  set_gdbarch_deprecated_fp_regnum (gdbarch, OR1K_FP_REGNUM);
> > +
> > +  /* Functions to supply register information */
> > +  set_gdbarch_register_name (gdbarch, or1k_register_name);
> > +  set_gdbarch_register_type (gdbarch, or1k_register_type);
> > +  set_gdbarch_register_reggroup_p (gdbarch, or1k_register_reggroup_p);
> > +
> > +  /* Functions to analyse frames */
> > +  set_gdbarch_skip_prologue (gdbarch, or1k_skip_prologue);
> > +  set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
> > +  set_gdbarch_frame_align (gdbarch, or1k_frame_align);
> > +  set_gdbarch_frame_red_zone_size (gdbarch, OR1K_FRAME_RED_ZONE_SIZE);
> > +
> > +  /* Functions to access frame data */
> > +  set_gdbarch_unwind_pc (gdbarch, or1k_unwind_pc);
> > +  set_gdbarch_unwind_sp (gdbarch, or1k_unwind_sp);
> > +
> > +  /* Functions handling dummy frames */
> > +  set_gdbarch_call_dummy_location (gdbarch, ON_STACK);
> > +  set_gdbarch_push_dummy_code (gdbarch, or1k_push_dummy_code);
> > +  set_gdbarch_push_dummy_call (gdbarch, or1k_push_dummy_call);
> > +  set_gdbarch_dummy_id (gdbarch, or1k_dummy_id);
> > +
> > +  /* Frame unwinders. Use DWARF debug info if available, otherwise use our
> > +     own unwinder.  */
> > +  dwarf2_append_unwinders (gdbarch);
> > +  frame_unwind_append_unwinder (gdbarch, &or1k_frame_unwind);
> > +
> > +  /* Get a CGEN CPU descriptor for this architecture.  */
> > +  {
> > +
> > +    const char *mach_name = binfo->printable_name;
> > +    enum cgen_endian endian = (info.byte_order == BFD_ENDIAN_BIG
> > +			       ? CGEN_ENDIAN_BIG : CGEN_ENDIAN_LITTLE);
> > +
> > +    tdep->gdb_cgen_cpu_desc =
> > +      or1k_cgen_cpu_open (CGEN_CPU_OPEN_BFDMACH, mach_name,
> > +			  CGEN_CPU_OPEN_ENDIAN, endian, CGEN_CPU_OPEN_END);
> > +
> > +    or1k_cgen_init_asm (tdep->gdb_cgen_cpu_desc);
> > +  }
> > +
> > +  /* If this mach has a delay slot.  */
> > +  if (binfo->mach == bfd_mach_or1k)
> > +    {
> > +      set_gdbarch_single_step_through_delay
> > +	(gdbarch, or1k_single_step_through_delay);
> > +    }
> > +
> > +  /* Check any target description for validity.  */
> > +  if (tdesc_has_registers (info.target_desc))
> > +    {
> > +      const struct tdesc_feature *feature;
> > +      int valid_p;
> > +
> > +      feature = tdesc_find_feature (info.target_desc, "org.gnu.gdb.or1k.group0");
> 
> Where is this target description coming from? I don't see an xml file with
> the patch series. Is it going to be submitted? Or this something a remote
> stub/simulator sends GDB upon connection?

Currently the only target that supplies the XML description is OpenOCD.
I have some XML which I generated for testing this without OpenOCD.  I
will see if I can clean that up and add to the patch series.

> > +      if (feature == NULL)
> > +        return NULL;
> > +
> > +      tdesc_data = tdesc_data_alloc ();
> > +
> > +      valid_p = 1;
> > +
> > +      for (i = 0; i < OR1K_NUM_REGS; i++)
> > +        valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
> > +                                            or1k_reg_names[i]);
> > +
> > +      if (!valid_p)
> > +        {
> > +          tdesc_data_cleanup (tdesc_data);
> > +          return NULL;
> > +        }
> > +    }
> > +
> > +  if (tdesc_data)
> > +    {
> > +      tdesc_use_registers (gdbarch, info.target_desc, tdesc_data);
> > +
> > +      /* Target descriptions may extend into the following groups.  */
> > +      reggroup_add (gdbarch, general_reggroup);
> > +      reggroup_add (gdbarch, system_reggroup);
> > +      reggroup_add (gdbarch, float_reggroup);
> > +      reggroup_add (gdbarch, vector_reggroup);
> > +      reggroup_add (gdbarch, reggroup_new ("immu", USER_REGGROUP));
> > +      reggroup_add (gdbarch, reggroup_new ("dmmu", USER_REGGROUP));
> > +      reggroup_add (gdbarch, reggroup_new ("icache", USER_REGGROUP));
> > +      reggroup_add (gdbarch, reggroup_new ("dcache", USER_REGGROUP));
> > +      reggroup_add (gdbarch, reggroup_new ("pic", USER_REGGROUP));
> > +      reggroup_add (gdbarch, reggroup_new ("timer", USER_REGGROUP));
> > +      reggroup_add (gdbarch, reggroup_new ("power", USER_REGGROUP));
> > +      reggroup_add (gdbarch, reggroup_new ("perf", USER_REGGROUP));
> > +      reggroup_add (gdbarch, reggroup_new ("mac", USER_REGGROUP));
> > +      reggroup_add (gdbarch, reggroup_new ("debug", USER_REGGROUP));
> > +      reggroup_add (gdbarch, all_reggroup);
> > +      reggroup_add (gdbarch, save_reggroup);
> > +      reggroup_add (gdbarch, restore_reggroup);
> > +    }
> > +
> > +  return gdbarch;
> > +
> > +}
> > +
> > +/* Dump the target specific data for this architecture.  */
> > +
> > +static void
> > +or1k_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
> > +{
> > +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> > +
> > +  if (NULL == tdep)
> > +    {
> > +      return;			/* Nothing to report */
> > +    }
> > +
> > +  fprintf_unfiltered (file, "or1k_dump_tdep: %d bytes per word\n",
> > +		      tdep->bytes_per_word);
> > +  fprintf_unfiltered (file, "or1k_dump_tdep: %d bytes per address\n",
> > +		      tdep->bytes_per_address);
> > +
> > +}
> > +\f
> > +
> > +extern initialize_file_ftype _initialize_or1k_tdep; /* -Wmissing-prototypes */
> > +
> > +void
> > +_initialize_or1k_tdep (void)
> > +{
> > +  /* Register this architecture.  */
> > +  gdbarch_register (bfd_arch_or1k, or1k_gdbarch_init, or1k_dump_tdep);
> > +
> > +  /* Tell remote stub that we support XML target description.  */
> > +  register_remote_support_xml ("or1k");
> > +}
> > diff --git a/gdb/or1k-tdep.h b/gdb/or1k-tdep.h
> > new file mode 100644
> > index 0000000..43ae7c3
> > --- /dev/null
> > +++ b/gdb/or1k-tdep.h
> > @@ -0,0 +1,57 @@
> > +/* Definitions to target GDB to OpenRISC 1000 32-bit targets.
> > +   Copyright (C) 2008-2016 Free Software Foundation, Inc.
> > +   Contributed by Jeremy Bennett <jeremy.bennett@embecosm.com>
> > +
> > +   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 3 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, see <http://www.gnu.org/licenses/>.  */
> > +
> > +
> > +#ifndef OR1K_TDEP__H
> > +#define OR1K_TDEP__H
> > +
> > +#ifndef TARGET_OR1K
> > +#define TARGET_OR1K
> > +#endif
> > +
> > +#include "opcodes/or1k-desc.h"
> > +#include "opcodes/or1k-opc.h"
> > +
> > +/* General Purpose Registers */
> > +#define OR1K_ZERO_REGNUM          0
> > +#define OR1K_SP_REGNUM            1
> > +#define OR1K_FP_REGNUM            2
> > +#define OR1K_FIRST_ARG_REGNUM     3
> > +#define OR1K_LAST_ARG_REGNUM      8
> > +#define OR1K_LR_REGNUM            9
> > +#define OR1K_FIRST_SAVED_REGNUM  10
> > +#define OR1K_RV_REGNUM           11
> > +#define OR1K_PPC_REGNUM          (OR1K_MAX_GPR_REGS + 0)
> > +#define OR1K_NPC_REGNUM          (OR1K_MAX_GPR_REGS + 1)
> > +#define OR1K_SR_REGNUM           (OR1K_MAX_GPR_REGS + 2)
> > +
> > +/* Properties of the architecture. GDB mapping of registers is all the GPRs
> > +   and SPRs followed by the PPC, NPC and SR at the end. Red zone is the area
> > +   past the end of the stack reserved for exception handlers etc.  */
> > +
> > +#define OR1K_MAX_GPR_REGS            32
> > +#define OR1K_NUM_PSEUDO_REGS         0
> > +#define OR1K_NUM_REGS               (OR1K_MAX_GPR_REGS + 3)
> > +#define OR1K_STACK_ALIGN             4
> > +#define OR1K_INSTLEN                 4
> > +#define OR1K_INSTBITLEN             (OR1K_INSTLEN * 8)
> > +#define OR1K_NUM_TAP_RECORDS         8
> > +#define OR1K_FRAME_RED_ZONE_SIZE     2536
> > +
> > +#endif /* OR1K_TDEP__H */
> > 
> 
> It would be nice to have all the formatting and cosmetics polished/fixed
> before we can give it another look for correctness of the code itself. Right
> now there are quite a bit of formatting issues.
> 
> Patches 2/3 and 3/3 look fine to me.

Thank you for the review.  As this is V3 and a big part of this version
was fixing formatting issues I should have done better to make sure we
were past that.

I tried to use 'indent' and regex's to find and fix the issues.  If you
have any pointers for automating the cleanup it would be helpful.  But
for now I will be going over it manually based on your comments.

-Stafford

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support
  2016-12-25  1:31     ` Stafford Horne
@ 2016-12-26 16:10       ` Luis Machado
  2016-12-27 13:20         ` Stafford Horne
  0 siblings, 1 reply; 8+ messages in thread
From: Luis Machado @ 2016-12-26 16:10 UTC (permalink / raw)
  To: Stafford Horne; +Cc: gdb-patches, openrisc, Franck Jullien

On 12/24/2016 07:31 PM, Stafford Horne wrote:
> On Fri, Dec 23, 2016 at 03:20:52PM -0600, Luis Machado wrote:
>> On 12/22/2016 10:14 AM, Stafford Horne wrote:
>>> +
>>> +/* OpenRISC specific includes.  */
>>> +#include "or1k-tdep.h"
>>> +\f
>>> +
>>> +/* The target-dependant structure for gdbarch.  */
>>> +
>>> +struct gdbarch_tdep
>>> +{
>>> +  int bytes_per_word;
>>> +  int bytes_per_address;
>>> +  CGEN_CPU_DESC gdb_cgen_cpu_desc;
>>> +};
>>> +
>>> +/* Support functions for the architecture definition.  */
>>> +
>>> +/* Get an instruction from memory.  */
>>> +
>>> +static ULONGEST
>>> +or1k_fetch_instruction (struct gdbarch *gdbarch, CORE_ADDR addr)
>>> +{
>>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>> +  gdb_byte buf[OR1K_INSTLEN];
>>> +
>>> +  if (target_read_memory (addr, buf, OR1K_INSTLEN))
>>> +    {
>>> +      memory_error (TARGET_XFER_E_IO, addr);
>>> +    }
>>> +
>>
>> No need for curly braces for single-statement conditional blocks.
>
> Alright, I missed that part of the gnu style spec. I have been looking
> at the formatting guide [0].  Is there something better or is there a
> way to get emacs (or something else) to validate everything including
> comments and spurious newlines?
>

I'm not sure. Other emacs users in gdb land can chime in. I'm a vim user 
myself, and i use some extensions to make whitespace (trailing/leading) 
issues more obvious, as well as lines with more than 80 cols.

>>> +/* Generic function to read bits from an instruction.  */
>>> +
>>> +static int
>>> +or1k_analyse_inst (uint32_t inst, const char *format, ...)
>>> +{
>>> +  /* Break out each field in turn, validating as we go.  */
>>> +  va_list ap;
>>> +
>>> +  int i;
>>> +  int iptr = 0;			/* Instruction pointer */
>>> +
>>> +  va_start (ap, format);
>>> +
>>> +  for (i = 0; 0 != format[i];)
>>> +    {
>>> +      const char *start_ptr;
>>> +      char *end_ptr;
>>> +
>>> +      uint32_t bits;		/* Bit substring of interest */
>>> +      uint32_t width;		/* Substring width */
>>> +      uint32_t *arg_ptr;
>>> +
>>> +      switch (format[i])
>>> +	{
>>> +	case ' ':
>>> +	  i++;
>>> +	  break;		/* Formatting: ignored */
>>> +
>>> +	case '0':
>>> +	case '1':		/* Constant bit field */
>>> +	  bits = (inst >> (OR1K_INSTBITLEN - iptr - 1)) & 0x1;
>>> +
>>> +	  if ((format[i] - '0') != bits)
>>> +	    {
>>> +	      return 0;
>>> +	    }
>>
>> No need for curly braces. Possibly more occurrences of this.
>>
>> Also, the identation seems a bit off. In special for some of the comments.
>
> Are you meaning the right side comments should be aligned? I think the
> code indentation looked alright for the most part.
>

The alignment of the comments on the same line as the statement feel a 
bit too far. I'd put them closer to the statement, but that may be a 
matter of personal taste though.

I was referring to the identation of the case statements. They appear as 
if they are aligned at the same level as the for statement, so behind 
the switch. Maybe a whitespace/tab problem?

If they're correct, then nothing needs to be done there.

>>> +
>>> +	  iptr++;
>>> +	  i++;
>>> +	  break;
>>> +
>>> +	case '%':		/* Bit field */
>>> +	  i++;
>>> +	  start_ptr = &(format[i]);
>>> +	  width = strtoul (start_ptr, &end_ptr, 10);
>>> +
>>> +	  /* Check we got something, and if so skip on */
>>
>> period and two spaces after end of comment. More occurrences of this
>> throughout.
>
> I will fix this one and a few more which are sentences.  For short (label)
> comments I have left them without the period.

For short comments that is fine.

>
> Let me know if there is an issue with this.  There is not much mentioned
> about this (short comments) in the style guide [1]
>
> [1] https://www.gnu.org/prep/standards/html_node/Comments.html#Comments
>
>>> +	  if (start_ptr == end_ptr)
>>> +	    {
>>> +	      throw_quit
>>> +		("bitstring \"%s\" at offset %d has no length field.\n",
>>> +		 format, i);
>>> +	    }
>>> +
>>> +	  i += end_ptr - start_ptr;
>>> +
>>> +	  /* Look for and skip the terminating 'b'. If it's not there, we
>>> +	     still give a fatal error, because these are fixed strings that
>>> +	     just should not be wrong.  */
>>
>> Two spaces after period. More occurrences throughout.
>
> This one does have 2 spaces after period.  Am I missing something? Or do
> you mean within the comment (... 'b'._If it's ...)?
>

We require two spaces after period even within the comment block, not 
just at its end.

>>> +}
>>> +
>>> +
>>> +/* Analyse a l.addi instruction in form: l.addi  rD,rA,I. This is used
>>> +   to parse add instructions during various prologue analysis routines.  */
>>> +
>>> +static int
>>> +or1k_analyse_l_addi (uint32_t inst,
>>> +		     unsigned int *rd_ptr,
>>> +		     unsigned int *ra_ptr, int *simm_ptr)
>>> +{
>>> +  /* Instruction fields */
>>> +  uint32_t rd, ra, i;
>>> +
>>> +  if (or1k_analyse_inst (inst, "10 0111 %5b %5b %16b", &rd, &ra, &i))
>>> +    {
>>> +      /* Found it. Construct the result fields.  */
>>> +      *rd_ptr = (unsigned int) rd;
>>> +      *ra_ptr = (unsigned int) ra;
>>
>> Do we need these explicit cast or can we use types that make the casts go
>> away?
>
> I had a look, its going to be a bit of work to fix this. The return
> values ra/rd/i are derived from the input uint32_t so they retain that
> type.  It would require either changing the instruction type, or
> changing the types of rd_ptr/ra_ptr which might require casts in
> less convenient places.
>
> Will spend some more time to see what can be done.  If you have a quick
> idea let me know.
>

If rd_ptr and ra_ptr are supposed to always point to 32-bit unsigned 
values, then uint32_t sounds more appropriate. The compiler gets to 
decide what "unsigned int" means, so that may be bigger/smaller 
depending on what the compiler is doing.

In any case, this is more a suggestion to attempt to clean the code up a 
little bit, not a requirement.

>>> +  /* Check any target description for validity.  */
>>> +  if (tdesc_has_registers (info.target_desc))
>>> +    {
>>> +      const struct tdesc_feature *feature;
>>> +      int valid_p;
>>> +
>>> +      feature = tdesc_find_feature (info.target_desc, "org.gnu.gdb.or1k.group0");
>>
>> Where is this target description coming from? I don't see an xml file with
>> the patch series. Is it going to be submitted? Or this something a remote
>> stub/simulator sends GDB upon connection?
>
> Currently the only target that supplies the XML description is OpenOCD.
> I have some XML which I generated for testing this without OpenOCD.  I
> will see if I can clean that up and add to the patch series.
>

It would be useful to have a target description on gdb's side so we can 
fallback to it whenever the target fails to provide a valid target 
description. It can also serve as a base for people implementing 
debugging stubs, since they can always refer to what gdb supports.

Additionally, it can help support core files in the future, if one needs it.

>>> +/* Properties of the architecture. GDB mapping of registers is all the GPRs
>>> +   and SPRs followed by the PPC, NPC and SR at the end. Red zone is the area
>>> +   past the end of the stack reserved for exception handlers etc.  */
>>> +
>>> +#define OR1K_MAX_GPR_REGS            32
>>> +#define OR1K_NUM_PSEUDO_REGS         0
>>> +#define OR1K_NUM_REGS               (OR1K_MAX_GPR_REGS + 3)
>>> +#define OR1K_STACK_ALIGN             4
>>> +#define OR1K_INSTLEN                 4
>>> +#define OR1K_INSTBITLEN             (OR1K_INSTLEN * 8)
>>> +#define OR1K_NUM_TAP_RECORDS         8
>>> +#define OR1K_FRAME_RED_ZONE_SIZE     2536
>>> +
>>> +#endif /* OR1K_TDEP__H */
>>>
>>
>> It would be nice to have all the formatting and cosmetics polished/fixed
>> before we can give it another look for correctness of the code itself. Right
>> now there are quite a bit of formatting issues.
>>
>> Patches 2/3 and 3/3 look fine to me.
>
> Thank you for the review.  As this is V3 and a big part of this version
> was fixing formatting issues I should have done better to make sure we
> were past that.

No worries. The formatting bits take a bit of practice to get right and 
we do appreciate your patience in coping with it.

>
> I tried to use 'indent' and regex's to find and fix the issues.  If you
> have any pointers for automating the cleanup it would be helpful.  But
> for now I will be going over it manually based on your comments.

The above will definitely help with fixing things up, but i wouldn't say 
they will be 100% accurate, especially with an existing code base.

There are only a few types of issues in this patch though, so we the 
comments, newlines, curly braces and identation fixed up we can catch 
other potential offenders as it goes.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support
  2016-12-26 16:10       ` Luis Machado
@ 2016-12-27 13:20         ` Stafford Horne
  0 siblings, 0 replies; 8+ messages in thread
From: Stafford Horne @ 2016-12-27 13:20 UTC (permalink / raw)
  To: Luis Machado; +Cc: gdb-patches, openrisc, Franck Jullien

On Mon, Dec 26, 2016 at 10:10:02AM -0600, Luis Machado wrote:
> On 12/24/2016 07:31 PM, Stafford Horne wrote:
> > On Fri, Dec 23, 2016 at 03:20:52PM -0600, Luis Machado wrote:
> > > On 12/22/2016 10:14 AM, Stafford Horne wrote:
> > > > +
> > > > +/* OpenRISC specific includes.  */
> > > > +#include "or1k-tdep.h"
> > > > +\f
> > > > +
> > > > +/* The target-dependant structure for gdbarch.  */
> > > > +
> > > > +struct gdbarch_tdep
> > > > +{
> > > > +  int bytes_per_word;
> > > > +  int bytes_per_address;
> > > > +  CGEN_CPU_DESC gdb_cgen_cpu_desc;
> > > > +};
> > > > +
> > > > +/* Support functions for the architecture definition.  */
> > > > +
> > > > +/* Get an instruction from memory.  */
> > > > +
> > > > +static ULONGEST
> > > > +or1k_fetch_instruction (struct gdbarch *gdbarch, CORE_ADDR addr)
> > > > +{
> > > > +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> > > > +  gdb_byte buf[OR1K_INSTLEN];
> > > > +
> > > > +  if (target_read_memory (addr, buf, OR1K_INSTLEN))
> > > > +    {
> > > > +      memory_error (TARGET_XFER_E_IO, addr);
> > > > +    }
> > > > +
> > > 
> > > No need for curly braces for single-statement conditional blocks.
> > 
> > Alright, I missed that part of the gnu style spec. I have been looking
> > at the formatting guide [0].  Is there something better or is there a
> > way to get emacs (or something else) to validate everything including
> > comments and spurious newlines?
> > 
> 
> I'm not sure. Other emacs users in gdb land can chime in. I'm a vim user
> myself, and i use some extensions to make whitespace (trailing/leading)
> issues more obvious, as well as lines with more than 80 cols.

Alright, I'm a vim user as well.  But I installed emacs just to see what
was available, couldn't find much.  I will use what I have and can find.
 
> > > > +/* Generic function to read bits from an instruction.  */
> > > > +
> > > > +static int
> > > > +or1k_analyse_inst (uint32_t inst, const char *format, ...)
> > > > +{
> > > > +  /* Break out each field in turn, validating as we go.  */
> > > > +  va_list ap;
> > > > +
> > > > +  int i;
> > > > +  int iptr = 0;			/* Instruction pointer */
> > > > +
> > > > +  va_start (ap, format);
> > > > +
> > > > +  for (i = 0; 0 != format[i];)
> > > > +    {
> > > > +      const char *start_ptr;
> > > > +      char *end_ptr;
> > > > +
> > > > +      uint32_t bits;		/* Bit substring of interest */
> > > > +      uint32_t width;		/* Substring width */
> > > > +      uint32_t *arg_ptr;
> > > > +
> > > > +      switch (format[i])
> > > > +	{
> > > > +	case ' ':
> > > > +	  i++;
> > > > +	  break;		/* Formatting: ignored */
> > > > +
> > > > +	case '0':
> > > > +	case '1':		/* Constant bit field */
> > > > +	  bits = (inst >> (OR1K_INSTBITLEN - iptr - 1)) & 0x1;
> > > > +
> > > > +	  if ((format[i] - '0') != bits)
> > > > +	    {
> > > > +	      return 0;
> > > > +	    }
> > > 
> > > No need for curly braces. Possibly more occurrences of this.
> > > 
> > > Also, the identation seems a bit off. In special for some of the comments.
> > 
> > Are you meaning the right side comments should be aligned? I think the
> > code indentation looked alright for the most part.
> > 
> 
> The alignment of the comments on the same line as the statement feel a bit
> too far. I'd put them closer to the statement, but that may be a matter of
> personal taste though.

OK, I am going to change to have just 1 space between end of statement
and the comment. 

> I was referring to the identation of the case statements. They appear as if
> they are aligned at the same level as the for statement, so behind the
> switch. Maybe a whitespace/tab problem?
> 
> If they're correct, then nothing needs to be done there.

I think they are correct.  I did find a few other indentation issues.

> > > > +
> > > > +	  iptr++;
> > > > +	  i++;
> > > > +	  break;
> > > > +
> > > > +	case '%':		/* Bit field */
> > > > +	  i++;
> > > > +	  start_ptr = &(format[i]);
> > > > +	  width = strtoul (start_ptr, &end_ptr, 10);
> > > > +
> > > > +	  /* Check we got something, and if so skip on */
> > > 
> > > period and two spaces after end of comment. More occurrences of this
> > > throughout.
> > 
> > I will fix this one and a few more which are sentences.  For short (label)
> > comments I have left them without the period.
> 
> For short comments that is fine.
> 
> > 
> > Let me know if there is an issue with this.  There is not much mentioned
> > about this (short comments) in the style guide [1]
> > 
> > [1] https://www.gnu.org/prep/standards/html_node/Comments.html#Comments
> > 
> > > > +	  if (start_ptr == end_ptr)
> > > > +	    {
> > > > +	      throw_quit
> > > > +		("bitstring \"%s\" at offset %d has no length field.\n",
> > > > +		 format, i);
> > > > +	    }
> > > > +
> > > > +	  i += end_ptr - start_ptr;
> > > > +
> > > > +	  /* Look for and skip the terminating 'b'. If it's not there, we
> > > > +	     still give a fatal error, because these are fixed strings that
> > > > +	     just should not be wrong.  */
> > > 
> > > Two spaces after period. More occurrences throughout.
> > 
> > This one does have 2 spaces after period.  Am I missing something? Or do
> > you mean within the comment (... 'b'._If it's ...)?
> > 
> 
> We require two spaces after period even within the comment block, not just
> at its end.

Understood, those will be fixed.

> > > > +}
> > > > +
> > > > +
> > > > +/* Analyse a l.addi instruction in form: l.addi  rD,rA,I. This is used
> > > > +   to parse add instructions during various prologue analysis routines.  */
> > > > +
> > > > +static int
> > > > +or1k_analyse_l_addi (uint32_t inst,
> > > > +		     unsigned int *rd_ptr,
> > > > +		     unsigned int *ra_ptr, int *simm_ptr)
> > > > +{
> > > > +  /* Instruction fields */
> > > > +  uint32_t rd, ra, i;
> > > > +
> > > > +  if (or1k_analyse_inst (inst, "10 0111 %5b %5b %16b", &rd, &ra, &i))
> > > > +    {
> > > > +      /* Found it. Construct the result fields.  */
> > > > +      *rd_ptr = (unsigned int) rd;
> > > > +      *ra_ptr = (unsigned int) ra;
> > > 
> > > Do we need these explicit cast or can we use types that make the casts go
> > > away?
> > 
> > I had a look, its going to be a bit of work to fix this. The return
> > values ra/rd/i are derived from the input uint32_t so they retain that
> > type.  It would require either changing the instruction type, or
> > changing the types of rd_ptr/ra_ptr which might require casts in
> > less convenient places.
> > 
> > Will spend some more time to see what can be done.  If you have a quick
> > idea let me know.
> > 
> 
> If rd_ptr and ra_ptr are supposed to always point to 32-bit unsigned values,
> then uint32_t sounds more appropriate. The compiler gets to decide what
> "unsigned int" means, so that may be bigger/smaller depending on what the
> compiler is doing.
> 
> In any case, this is more a suggestion to attempt to clean the code up a
> little bit, not a requirement.

I will work on it.  Ill update in v4 the results.

> > > > +  /* Check any target description for validity.  */
> > > > +  if (tdesc_has_registers (info.target_desc))
> > > > +    {
> > > > +      const struct tdesc_feature *feature;
> > > > +      int valid_p;
> > > > +
> > > > +      feature = tdesc_find_feature (info.target_desc, "org.gnu.gdb.or1k.group0");
> > > 
> > > Where is this target description coming from? I don't see an xml file with
> > > the patch series. Is it going to be submitted? Or this something a remote
> > > stub/simulator sends GDB upon connection?
> > 
> > Currently the only target that supplies the XML description is OpenOCD.
> > I have some XML which I generated for testing this without OpenOCD.  I
> > will see if I can clean that up and add to the patch series.
> > 
> 
> It would be useful to have a target description on gdb's side so we can
> fallback to it whenever the target fails to provide a valid target
> description. It can also serve as a base for people implementing debugging
> stubs, since they can always refer to what gdb supports.
> 
> Additionally, it can help support core files in the future, if one needs it.

Understood, Ill try to get it in.  It would be good if the gdb builtin sim
can support most of the ~1000+ registers as well.

> > > > +/* Properties of the architecture. GDB mapping of registers is all the GPRs
> > > > +   and SPRs followed by the PPC, NPC and SR at the end. Red zone is the area
> > > > +   past the end of the stack reserved for exception handlers etc.  */
> > > > +
> > > > +#define OR1K_MAX_GPR_REGS            32
> > > > +#define OR1K_NUM_PSEUDO_REGS         0
> > > > +#define OR1K_NUM_REGS               (OR1K_MAX_GPR_REGS + 3)
> > > > +#define OR1K_STACK_ALIGN             4
> > > > +#define OR1K_INSTLEN                 4
> > > > +#define OR1K_INSTBITLEN             (OR1K_INSTLEN * 8)
> > > > +#define OR1K_NUM_TAP_RECORDS         8
> > > > +#define OR1K_FRAME_RED_ZONE_SIZE     2536
> > > > +
> > > > +#endif /* OR1K_TDEP__H */
> > > > 
> > > 
> > > It would be nice to have all the formatting and cosmetics polished/fixed
> > > before we can give it another look for correctness of the code itself. Right
> > > now there are quite a bit of formatting issues.
> > > 
> > > Patches 2/3 and 3/3 look fine to me.
> > 
> > Thank you for the review.  As this is V3 and a big part of this version
> > was fixing formatting issues I should have done better to make sure we
> > were past that.
> 
> No worries. The formatting bits take a bit of practice to get right and we
> do appreciate your patience in coping with it.
> 
> > 
> > I tried to use 'indent' and regex's to find and fix the issues.  If you
> > have any pointers for automating the cleanup it would be helpful.  But
> > for now I will be going over it manually based on your comments.
> 
> The above will definitely help with fixing things up, but i wouldn't say
> they will be 100% accurate, especially with an existing code base.
> 
> There are only a few types of issues in this patch though, so we the
> comments, newlines, curly braces and identation fixed up we can catch other
> potential offenders as it goes.

Thanks for reviewing again.

-Stafford

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-12-27 13:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-22 16:14 [PATCH v3 0/3] OpenRISC gdb port Stafford Horne
2016-12-22 16:14 ` [PATCH v3 1/3] gdb: Add OpenRISC or1k and or1knd target support Stafford Horne
2016-12-23 21:21   ` Luis Machado
2016-12-25  1:31     ` Stafford Horne
2016-12-26 16:10       ` Luis Machado
2016-12-27 13:20         ` Stafford Horne
2016-12-22 16:14 ` [PATCH v3 2/3] gdb: testsuite: Add or1k l.nop inscruction Stafford Horne
2016-12-22 16:14 ` [PATCH v3 3/3] Add gdb for or1k build Stafford Horne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).