public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 3/3] arc/nps400 : New cmem instructions and associated relocation
  2016-04-07 11:05 ` [PATCH " Andrew Burgess
  2016-04-07 11:05   ` [PATCH 1/3] opcodes/arc: Compute insn lengths in disassembler Andrew Burgess
  2016-04-07 11:05   ` [PATCH 2/3] bfd/arc: Rename enum entries to avoid conflicts Andrew Burgess
@ 2016-04-07 11:05   ` Andrew Burgess
  2016-04-11 12:55     ` Cupertino Miranda
  2016-04-12 11:00   ` [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function Andrew Burgess
                     ` (2 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2016-04-07 11:05 UTC (permalink / raw)
  To: binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca, Andrew Burgess

This patch adds some new arc/nps instructions.  These new instructions
require a new relocation, and this new relocation requires a specific
check of the computed relocation value.

To achieve this specific check (that the most significant 2 bytes are
a particular value), I added a new function, arc_final_relocation_checks.

I know that the Synopsys team have a particular design in mind for how
they want ARC relocations to be handled, and this new function might
not fit within their desired design.

I'm not particularly attached to the new function, I'm open to any
alternative suggestions for how to achieve the same end result if the
Synopsys team would like to make a suggestion.  In the absence of any
alternative suggestions though, I believe the code I offer below
should be acceptable for inclusion.

Thanks,
Andrew


---

Add support for arc/nps400 cmem instructions, these load and store
instructions are hard-wired to access "0x57f00000 + 16-bit-offset".

Supporting this relocation required some additions to the arc relocation
handling in the bfd library, as well as the standard changes required to
add a new relocation type.

There's a test of the new instructions in the assembler, and a test of
the relocation in the linker.

bfd/ChangeLog:

	* reloc.c: Add BFD_RELOC_ARC_NPS_CMEM16 entry.
	* bfd-in2.h: Regenerate.
	* libbfd.h: Regenerate.
	* elf32-arc.c: Add 'opcode/arc.h' include.
	(struct arc_relocation_data): Add symbol_name.
	(arc_final_relocation_checks): New function.
	(arc_do_relocation): Update ARC_RELOC_HOWTO definition to call
	arc_final_relocation_checks.
	(elf_arc_relocate_section): Setup symbol_name in reloc_data.

gas/ChangeLog:

	* testsuite/gas/arc/nps400-3.d: New file.
	* testsuite/gas/arc/nps400-3.s: New file.

include/ChangeLog:

	* elf/arc-reloc.def: Add ARC_NPS_CMEM16 reloc.
	* opcode/arc.h (NPS_CMEM_HIGH_VALUE): Define.

ld/ChangeLog:

	* testsuite/ld-arc/arc.exp: New file.
	* testsuite/ld-arc/nps-1.s: New file.
	* testsuite/ld-arc/nps-1a.d: New file.
	* testsuite/ld-arc/nps-1b.d: New file.
	* testsuite/ld-arc/nps-1b.err: New file.

opcodes/ChangeLog:

	* arc-nps400-tbl.h: Add xldb, xldw, xld, xstb, xstw, and xst
	instructions.
	* arc-opc.c (insert_nps_cmem_uimm16): New function.
	(extract_nps_cmem_uimm16): New function.
	(arc_operands): Add NPS_XLDST_UIMM16 operand.
---
 bfd/ChangeLog                    | 12 +++++++++
 bfd/bfd-in2.h                    |  1 +
 bfd/elf32-arc.c                  | 58 ++++++++++++++++++++++++++++++++++++++++
 bfd/libbfd.h                     |  1 +
 bfd/reloc.c                      |  2 ++
 gas/ChangeLog                    |  5 ++++
 gas/testsuite/gas/arc/nps400-3.d | 56 ++++++++++++++++++++++++++++++++++++++
 gas/testsuite/gas/arc/nps400-3.s | 23 ++++++++++++++++
 include/ChangeLog                |  5 ++++
 include/elf/arc-reloc.def        |  7 +++++
 include/opcode/arc.h             |  3 +++
 ld/ChangeLog                     |  8 ++++++
 ld/testsuite/ld-arc/arc.exp      | 30 +++++++++++++++++++++
 ld/testsuite/ld-arc/nps-1.s      | 10 +++++++
 ld/testsuite/ld-arc/nps-1a.d     | 16 +++++++++++
 ld/testsuite/ld-arc/nps-1b.d     |  4 +++
 ld/testsuite/ld-arc/nps-1b.err   |  1 +
 opcodes/ChangeLog                |  8 ++++++
 opcodes/arc-nps400-tbl.h         | 12 +++++++++
 opcodes/arc-opc.c                | 22 +++++++++++++++
 20 files changed, 284 insertions(+)
 create mode 100644 gas/testsuite/gas/arc/nps400-3.d
 create mode 100644 gas/testsuite/gas/arc/nps400-3.s
 create mode 100644 ld/testsuite/ld-arc/arc.exp
 create mode 100644 ld/testsuite/ld-arc/nps-1.s
 create mode 100644 ld/testsuite/ld-arc/nps-1a.d
 create mode 100644 ld/testsuite/ld-arc/nps-1b.d
 create mode 100644 ld/testsuite/ld-arc/nps-1b.err

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index f02e2aa..6532f6e 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -3721,6 +3721,7 @@ pc-relative or some form of GOT-indirect relocation.  */
   BFD_RELOC_ARC_TLS_LE_32,
   BFD_RELOC_ARC_S25W_PCREL_PLT,
   BFD_RELOC_ARC_S21H_PCREL_PLT,
+  BFD_RELOC_ARC_NPS_CMEM16,
 
 /* ADI Blackfin 16 bit immediate absolute reloc.  */
   BFD_RELOC_BFIN_16_IMM,
diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c
index 263effc..52bf5dc 100644
--- a/bfd/elf32-arc.c
+++ b/bfd/elf32-arc.c
@@ -26,6 +26,7 @@
 #include "elf/arc.h"
 #include "libiberty.h"
 #include "opcode/arc-func.h"
+#include "opcode/arc.h"
 #include "arc-plt.h"
 
 #ifdef DEBUG
@@ -722,6 +723,8 @@ struct arc_relocation_data
   bfd_signed_vma  got_symbol_vma;
 
   bfd_boolean	  should_relocate;
+
+  const char *    symbol_name;
 };
 
 static void
@@ -873,11 +876,61 @@ middle_endian_convert (bfd_vma insn, bfd_boolean do_it)
       ARC_DEBUG ("after  = 0x%08x\n", (unsigned int) insn); \
     }
 
+/* Perform final checks, and possible adjustment to the relocation value
+   before applying the relocation to the instruction.  */
+
+static inline bfd_reloc_status_type
+arc_final_relocation_checks (bfd_signed_vma *relocation,
+			     const struct arc_relocation_data reloc_data,
+			     struct bfd_link_info *info ATTRIBUTE_UNUSED)
+{
+  switch (reloc_data.howto->type)
+    {
+    case R_ARC_NPS_CMEM16:
+      if (((*relocation >> 16) & 0xffff) != NPS_CMEM_HIGH_VALUE)
+        {
+          if (reloc_data.reloc_addend == 0)
+            (*_bfd_error_handler)
+              (_("%B(%A+0x%lx): CMEM relocation to `%s' is invalid, "
+                 "16 MSB should be 0x%04x (value is 0x%lx)"),
+               reloc_data.input_section->owner,
+               reloc_data.input_section,
+               reloc_data.reloc_offset,
+               reloc_data.symbol_name,
+               NPS_CMEM_HIGH_VALUE,
+               (*relocation));
+          else
+            (*_bfd_error_handler)
+              (_("%B(%A+0x%lx): CMEM relocation to `%s+0x%lx' is invalid, "
+                 "16 MSB should be 0x%04x (value is 0x%lx)"),
+               reloc_data.input_section->owner,
+               reloc_data.input_section,
+               reloc_data.reloc_offset,
+               reloc_data.symbol_name,
+               reloc_data.reloc_addend,
+               NPS_CMEM_HIGH_VALUE,
+               (*relocation));
+          return bfd_reloc_overflow;
+        }
+      *relocation &= 0xffff;
+      break;
+
+    default:
+      break;
+    }
+
+  return bfd_reloc_ok;
+}
+
 #define ARC_RELOC_HOWTO(TYPE, VALUE, SIZE, BITSIZE, RELOC_FUNCTION, OVERFLOW, FORMULA) \
   case R_##TYPE: \
     { \
       bfd_signed_vma bitsize ATTRIBUTE_UNUSED = BITSIZE; \
+      bfd_reloc_status_type status; \
       relocation = FORMULA  ; \
+      status = arc_final_relocation_checks (&relocation, reloc_data, info); \
+      if (status != bfd_reloc_ok) \
+        return status; \
       PRINT_DEBUG_RELOC_INFO_BEFORE (#FORMULA, #TYPE); \
       insn = middle_endian_convert (insn, IS_ME (#FORMULA, abfd)); \
       insn = (* get_replace_function (abfd, TYPE)) (insn, relocation); \
@@ -1168,6 +1221,10 @@ elf_arc_relocate_section (bfd *		   output_bfd,
 
 	  reloc_data.sym_value = sym->st_value;
 	  reloc_data.sym_section = sec;
+	  reloc_data.symbol_name =
+            bfd_elf_string_from_elf_section (input_bfd,
+                                             symtab_hdr->sh_link,
+                                             sym->st_name);
 
 	  /* Mergeable section handling.  */
 	  if ((sec->flags & SEC_MERGE)
@@ -1284,6 +1341,7 @@ elf_arc_relocate_section (bfd *		   output_bfd,
 	    h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
 	  BFD_ASSERT ((h->dynindx == -1) >= (h->forced_local != 0));
+	  reloc_data.symbol_name = h->root.root.string;
 	  /* If we have encountered a definition for this symbol.  */
 	  if (h->root.type == bfd_link_hash_defined
 	      || h->root.type == bfd_link_hash_defweak)
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index d7183d3..16c0aee 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -1738,6 +1738,7 @@ static const char *const bfd_reloc_code_real_names[] = { "@@uninitialized@@",
   "BFD_RELOC_ARC_TLS_LE_32",
   "BFD_RELOC_ARC_S25W_PCREL_PLT",
   "BFD_RELOC_ARC_S21H_PCREL_PLT",
+  "BFD_RELOC_ARC_NPS_CMEM16",
   "BFD_RELOC_BFIN_16_IMM",
   "BFD_RELOC_BFIN_16_HIGH",
   "BFD_RELOC_BFIN_4_PCREL",
diff --git a/bfd/reloc.c b/bfd/reloc.c
index 0135c04..c3b713b 100644
--- a/bfd/reloc.c
+++ b/bfd/reloc.c
@@ -3668,6 +3668,8 @@ ENUMX
   BFD_RELOC_ARC_S25W_PCREL_PLT
 ENUMX
   BFD_RELOC_ARC_S21H_PCREL_PLT
+ENUMX
+  BFD_RELOC_ARC_NPS_CMEM16
 ENUMDOC
   ARC relocs.
 
diff --git a/gas/testsuite/gas/arc/nps400-3.d b/gas/testsuite/gas/arc/nps400-3.d
new file mode 100644
index 0000000..ea52554
--- /dev/null
+++ b/gas/testsuite/gas/arc/nps400-3.d
@@ -0,0 +1,56 @@
+#as: -mcpu=nps400
+#objdump: -dr
+
+.*: +file format .*arc.*
+
+Disassembly of section .text:
+
+[0-9a-f]+ <.*>:
+   0:	5988 0000           	xldb	r12,\[0x57f00000\]
+   4:	5ae8 ffff           	xldb	r23,\[0x57f0ffff\]
+   8:	5868 0000           	xldb	r3,\[0x57f00000\]
+   c:	5968 ffff           	xldb	r11,\[0x57f0ffff\]
+  10:	5a88 0000           	xldb	r20,\[0x57f00000\]
+			10: R_ARC_NPS_CMEM16	foo
+  14:	5828 0000           	xldb	r1,\[0x57f00000\]
+			14: R_ARC_NPS_CMEM16	foo\+0x20
+  18:	5989 0000           	xldw	r12,\[0x57f00000\]
+  1c:	5ae9 ffff           	xldw	r23,\[0x57f0ffff\]
+  20:	5869 0000           	xldw	r3,\[0x57f00000\]
+  24:	5969 ffff           	xldw	r11,\[0x57f0ffff\]
+  28:	5a89 0000           	xldw	r20,\[0x57f00000\]
+			28: R_ARC_NPS_CMEM16	foo
+  2c:	5829 0000           	xldw	r1,\[0x57f00000\]
+			2c: R_ARC_NPS_CMEM16	foo\+0x20
+  30:	598a 0000           	xld	r12,\[0x57f00000\]
+  34:	5aea ffff           	xld	r23,\[0x57f0ffff\]
+  38:	586a 0000           	xld	r3,\[0x57f00000\]
+  3c:	596a ffff           	xld	r11,\[0x57f0ffff\]
+  40:	5a8a 0000           	xld	r20,\[0x57f00000\]
+			40: R_ARC_NPS_CMEM16	foo
+  44:	582a 0000           	xld	r1,\[0x57f00000\]
+			44: R_ARC_NPS_CMEM16	foo\+0x20
+  48:	598c 0000           	xstb	r12,\[0x57f00000\]
+  4c:	5aec ffff           	xstb	r23,\[0x57f0ffff\]
+  50:	586c 0000           	xstb	r3,\[0x57f00000\]
+  54:	596c ffff           	xstb	r11,\[0x57f0ffff\]
+  58:	5a8c 0000           	xstb	r20,\[0x57f00000\]
+			58: R_ARC_NPS_CMEM16	foo
+  5c:	582c 0000           	xstb	r1,\[0x57f00000\]
+			5c: R_ARC_NPS_CMEM16	foo\+0x20
+  60:	598d 0000           	xstw	r12,\[0x57f00000\]
+  64:	5aed ffff           	xstw	r23,\[0x57f0ffff\]
+  68:	586d 0000           	xstw	r3,\[0x57f00000\]
+  6c:	596d ffff           	xstw	r11,\[0x57f0ffff\]
+  70:	5a8d 0000           	xstw	r20,\[0x57f00000\]
+			70: R_ARC_NPS_CMEM16	foo
+  74:	582d 0000           	xstw	r1,\[0x57f00000\]
+			74: R_ARC_NPS_CMEM16	foo\+0x20
+  78:	598e 0000           	xst	r12,\[0x57f00000\]
+  7c:	5aee ffff           	xst	r23,\[0x57f0ffff\]
+  80:	586e 0000           	xst	r3,\[0x57f00000\]
+  84:	596e ffff           	xst	r11,\[0x57f0ffff\]
+  88:	5a8e 0000           	xst	r20,\[0x57f00000\]
+			88: R_ARC_NPS_CMEM16	foo
+  8c:	582e 0000           	xst	r1,\[0x57f00000\]
+			8c: R_ARC_NPS_CMEM16	foo\+0x20
diff --git a/gas/testsuite/gas/arc/nps400-3.s b/gas/testsuite/gas/arc/nps400-3.s
new file mode 100644
index 0000000..6840223
--- /dev/null
+++ b/gas/testsuite/gas/arc/nps400-3.s
@@ -0,0 +1,23 @@
+        .macro  xldst_test mnem
+        \mnem r12, [ 0x0 ]
+        \mnem r23, [ 0xffff ]
+        \mnem r3, [ 0x57f00000 ]
+        \mnem r11, [ 0x57f0ffff ]
+        \mnem r20, [ foo ]
+        \mnem r1, [ foo + 0x20 ]
+        .endm
+
+        .text
+        ;; xldb
+        xldst_test xldb
+        ;; xldw
+        xldst_test xldw
+        ;; xld
+        xldst_test xld
+        ;; xstb
+        xldst_test xstb
+        ;; xstw
+        xldst_test xstw
+        ;; xst
+        xldst_test xst
+
diff --git a/include/elf/arc-reloc.def b/include/elf/arc-reloc.def
index 36a3516..b271e0e 100644
--- a/include/elf/arc-reloc.def
+++ b/include/elf/arc-reloc.def
@@ -490,3 +490,10 @@ ARC_RELOC_HOWTO(ARC_S21H_PCREL_PLT, 77, \
                 replace_disp21h, \
                 signed, \
                 ( ME ( ( ( ( L + A ) - P ) >> 1 ) ) ))
+
+ARC_RELOC_HOWTO(ARC_NPS_CMEM16, 78, \
+                2, \
+                16, \
+                replace_bits16, \
+                bitfield, \
+                ( S + A ))
diff --git a/include/opcode/arc.h b/include/opcode/arc.h
index bc0e1ad..2827c96 100644
--- a/include/opcode/arc.h
+++ b/include/opcode/arc.h
@@ -423,6 +423,9 @@ extern const unsigned arc_num_aux_regs;
 extern const struct arc_opcode arc_relax_opcodes[];
 extern const unsigned arc_num_relax_opcodes;
 
+/* Macro used for generating one class of NPS instructions.  */
+#define NPS_CMEM_HIGH_VALUE 0x57f0
+
 /* Macros to help generating regular pattern instructions.  */
 #define FIELDA(word) (word & 0x3F)
 #define FIELDB(word) (((word & 0x07) << 24) | (((word >> 3) & 0x07) << 12))
diff --git a/ld/testsuite/ld-arc/arc.exp b/ld/testsuite/ld-arc/arc.exp
new file mode 100644
index 0000000..0cf6228
--- /dev/null
+++ b/ld/testsuite/ld-arc/arc.exp
@@ -0,0 +1,30 @@
+# Copyright (C) 2016 Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+#
+
+if { ![istarget arc*-*-*] } {
+    return
+}
+
+set arc_test_list [lsort [glob -nocomplain $srcdir/$subdir/*.d]]
+foreach arc_test $arc_test_list {
+    verbose [file rootname $arc_test]
+    run_dump_test [file rootname $arc_test]
+}
+
diff --git a/ld/testsuite/ld-arc/nps-1.s b/ld/testsuite/ld-arc/nps-1.s
new file mode 100644
index 0000000..295fa2c
--- /dev/null
+++ b/ld/testsuite/ld-arc/nps-1.s
@@ -0,0 +1,10 @@
+        .text
+        .global __start
+__start:
+        xldb r10, [ foo ]
+        xldw r10, [ foo ]
+        xld r10, [ foo ]
+        xstb r10, [ foo ]
+        xstw r10, [ foo ]
+        xst r10, [ foo ]
+
diff --git a/ld/testsuite/ld-arc/nps-1a.d b/ld/testsuite/ld-arc/nps-1a.d
new file mode 100644
index 0000000..932842b
--- /dev/null
+++ b/ld/testsuite/ld-arc/nps-1a.d
@@ -0,0 +1,16 @@
+#source: nps-1.s
+#as: -mcpu=nps400
+#ld: -defsym=foo=0x57f03000
+#objdump: -d
+
+.*: +file format .*arc.*
+
+Disassembly of section .text:
+
+[0-9a-f]+ <.*>:
+   10074:	5948 3000           	xldb	r10,\[0x57f03000\]
+   10078:	5949 3000           	xldw	r10,\[0x57f03000\]
+   1007c:	594a 3000           	xld	r10,\[0x57f03000\]
+   10080:	594c 3000           	xstb	r10,\[0x57f03000\]
+   10084:	594d 3000           	xstw	r10,\[0x57f03000\]
+   10088:	594e 3000           	xst	r10,\[0x57f03000\]
diff --git a/ld/testsuite/ld-arc/nps-1b.d b/ld/testsuite/ld-arc/nps-1b.d
new file mode 100644
index 0000000..56c29ae
--- /dev/null
+++ b/ld/testsuite/ld-arc/nps-1b.d
@@ -0,0 +1,4 @@
+#source: nps-1.s
+#as: -mcpu=nps400
+#ld: -defsym=foo=0x56f03000
+#error_output: nps-1b.err
diff --git a/ld/testsuite/ld-arc/nps-1b.err b/ld/testsuite/ld-arc/nps-1b.err
new file mode 100644
index 0000000..a44b3c1
--- /dev/null
+++ b/ld/testsuite/ld-arc/nps-1b.err
@@ -0,0 +1 @@
+.*\.o\(\.text\+0x0\): CMEM relocation to `foo' is invalid, 16 MSB should be 0x57f0 \(value is 0x56f03000\)
diff --git a/opcodes/arc-nps400-tbl.h b/opcodes/arc-nps400-tbl.h
index dc7b066..f925ca3 100644
--- a/opcodes/arc-nps400-tbl.h
+++ b/opcodes/arc-nps400-tbl.h
@@ -123,3 +123,15 @@
 
 /* crc32<.r> 0,limm,u6		00111 110 01 110100 R 111 uuuuuu 111110 */
 { "crc32", 0x3e74703e, 0xffff703f, ARC_OPCODE_NPS400, BITOP, NONE, { ZA, LIMM, UIMM6_20 }, { C_NPS_R }},
+
+/****      Load / Store From (0x57f00000 + Offset) Instructions       ****/
+
+#define XLDST_LIKE(NAME,SUBOP2)                                          \
+  { NAME, (0x58000000 | (SUBOP2 << 16)), 0xf81f0000, ARC_OPCODE_NPS400, MEMORY, NONE, { NPS_R_DST, BRAKET, NPS_XLDST_UIMM16, BRAKETdup }, { 0 }},
+
+XLDST_LIKE("xldb", 0x8)
+XLDST_LIKE("xldw", 0x9)
+XLDST_LIKE("xld", 0xa)
+XLDST_LIKE("xstb", 0xc)
+XLDST_LIKE("xstw", 0xd)
+XLDST_LIKE("xst", 0xe)
diff --git a/opcodes/arc-opc.c b/opcodes/arc-opc.c
index f182318..8173f8a 100644
--- a/opcodes/arc-opc.c
+++ b/opcodes/arc-opc.c
@@ -838,6 +838,25 @@ extract_nps_dst_pos_and_size (unsigned insn ATTRIBUTE_UNUSED,
   return (insn & 0x1f);
 }
 
+static unsigned
+insert_nps_cmem_uimm16 (unsigned insn ATTRIBUTE_UNUSED,
+                        int value ATTRIBUTE_UNUSED,
+                        const char **errmsg ATTRIBUTE_UNUSED)
+{
+  int top = (value >> 16) & 0xffff;
+  if (top != 0x0 && top != NPS_CMEM_HIGH_VALUE)
+    *errmsg = _("invalid value for CMEM ld/st immediate");
+  insn |= (value & 0xffff);
+  return insn;
+}
+
+static int
+extract_nps_cmem_uimm16 (unsigned insn ATTRIBUTE_UNUSED,
+                         bfd_boolean * invalid ATTRIBUTE_UNUSED)
+{
+  return (NPS_CMEM_HIGH_VALUE << 16) | (insn & 0xffff);
+}
+
 /* Include the generic extract/insert functions.  Order is important
    as some of the functions present in the .h may be disabled via
    defines.  */
@@ -1442,6 +1461,9 @@ const struct arc_operand arc_operands[] =
 
 #define NPS_RFLT_UIMM6		(NPS_UIMM16 + 1)
   { 6, 6, 0, ARC_OPERAND_UNSIGNED | ARC_OPERAND_NCHK, insert_nps_rflt_uimm6, extract_nps_rflt_uimm6 },
+
+#define NPS_XLDST_UIMM16	(NPS_RFLT_UIMM6 + 1)
+  { 16, 0, BFD_RELOC_ARC_NPS_CMEM16, ARC_OPERAND_UNSIGNED | ARC_OPERAND_NCHK, insert_nps_cmem_uimm16, extract_nps_cmem_uimm16 },
 };
 
 const unsigned arc_num_operands = ARRAY_SIZE (arc_operands);
-- 
2.5.1

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

* [PATCH 0/3] ARC: Add another group of nps instructions
@ 2016-04-07 11:05 ` Andrew Burgess
  2016-04-07 11:05   ` [PATCH 1/3] opcodes/arc: Compute insn lengths in disassembler Andrew Burgess
                     ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Andrew Burgess @ 2016-04-07 11:05 UTC (permalink / raw)
  To: binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca, Andrew Burgess

This patch series adds more nps instructions to the arc architecture.
The first two patches are preparation, resolving some issues that I
ran into on the way.

Patch #1 makes the disassembler a little easier to maintain, at the
expense of a small start up cost.  Personally I think the trade off is
worth it as we otherwise risk having an ever-growing mini-decoder in
the disassembler that we have to maintain.

Patch #2 should be fairly obvious I think, there was a name conflict
in the enum values used.

Patch #3 should be fairly straight forward except for one issue which
the team at Synopsys might not be happy with, I've discussed this more
in the patch itself.

Thanks,
Andrew

---

Andrew Burgess (3):
  opcodes/arc: Compute insn lengths in disassembler
  bfd/arc: Rename enum entries to avoid conflicts
  arc/nps400 : New cmem instructions and associated relocation

 bfd/ChangeLog                    | 20 +++++++++
 bfd/bfd-in2.h                    |  1 +
 bfd/elf32-arc.c                  | 92 ++++++++++++++++++++++++++++++++--------
 bfd/libbfd.h                     |  1 +
 bfd/reloc.c                      |  2 +
 gas/ChangeLog                    |  5 +++
 gas/testsuite/gas/arc/nps400-3.d | 56 ++++++++++++++++++++++++
 gas/testsuite/gas/arc/nps400-3.s | 23 ++++++++++
 include/ChangeLog                |  5 +++
 include/elf/arc-reloc.def        |  7 +++
 include/opcode/arc.h             |  3 ++
 ld/ChangeLog                     |  8 ++++
 ld/testsuite/ld-arc/arc.exp      | 30 +++++++++++++
 ld/testsuite/ld-arc/nps-1.s      | 10 +++++
 ld/testsuite/ld-arc/nps-1a.d     | 16 +++++++
 ld/testsuite/ld-arc/nps-1b.d     |  4 ++
 ld/testsuite/ld-arc/nps-1b.err   |  1 +
 opcodes/ChangeLog                | 13 ++++++
 opcodes/arc-dis.c                | 69 +++++++++++++++++++++++++-----
 opcodes/arc-nps400-tbl.h         | 12 ++++++
 opcodes/arc-opc.c                | 22 ++++++++++
 21 files changed, 372 insertions(+), 28 deletions(-)
 create mode 100644 gas/testsuite/gas/arc/nps400-3.d
 create mode 100644 gas/testsuite/gas/arc/nps400-3.s
 create mode 100644 ld/testsuite/ld-arc/arc.exp
 create mode 100644 ld/testsuite/ld-arc/nps-1.s
 create mode 100644 ld/testsuite/ld-arc/nps-1a.d
 create mode 100644 ld/testsuite/ld-arc/nps-1b.d
 create mode 100644 ld/testsuite/ld-arc/nps-1b.err

-- 
2.5.1

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

* [PATCH 2/3] bfd/arc: Rename enum entries to avoid conflicts
  2016-04-07 11:05 ` [PATCH " Andrew Burgess
  2016-04-07 11:05   ` [PATCH 1/3] opcodes/arc: Compute insn lengths in disassembler Andrew Burgess
@ 2016-04-07 11:05   ` Andrew Burgess
  2016-04-07 11:20     ` Claudiu Zissulescu
  2016-04-07 11:05   ` [PATCH 3/3] arc/nps400 : New cmem instructions and associated relocation Andrew Burgess
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2016-04-07 11:05 UTC (permalink / raw)
  To: binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca, Andrew Burgess

In bfd/elf32-arc.c an enum is created that contains entries with generic
names like 'NONE' and 'OFF'.  This has been fine for now, but I had a
need to include opcode/arc.h into bfd/elf32-arc.c.  Unfortunately
opcode/arc.h includes a different enum with identical generic names.

Given that changing the enum in the header file could mean wide-ranging
changes, while changing the enum in the .c file is limited to only
changing the one file, I've added a prefix to the enum in the .c file.

This commit does not add the new include, that will come later.  There
should be no functional change with this commit.

bfd/ChangeLog:

	* elf32-arc.c (tls_got_entries): Add 'TLS_GOT_' prefix to all
	entries.
	(elf_arc_relocate_section): Update enum uses.
	(elf_arc_check_relocs): Likewise.
	(elf_arc_finish_dynamic_symbol): Likewise.
---
 bfd/ChangeLog   |  8 ++++++++
 bfd/elf32-arc.c | 34 +++++++++++++++++-----------------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c
index 323f65d..263effc 100644
--- a/bfd/elf32-arc.c
+++ b/bfd/elf32-arc.c
@@ -127,10 +127,10 @@ enum tls_type_e
 
 enum tls_got_entries
 {
-  NONE = 0,
-  MOD,
-  OFF,
-  MOD_AND_OFF
+  TLS_GOT_NONE = 0,
+  TLS_GOT_MOD,
+  TLS_GOT_OFF,
+  TLS_GOT_MOD_AND_OFF
 };
 
 struct got_entry
@@ -1401,7 +1401,7 @@ elf_arc_relocate_section (bfd *		   output_bfd,
 			  bfd_put_32 (output_bfd,
 				      sym_value - sec_vma,
 				      htab->sgot->contents + entry->offset
-				      + (entry->existing_entries == MOD_AND_OFF ? 4 : 0));
+				      + (entry->existing_entries ==TLS_GOT_MOD_AND_OFF ? 4 : 0));
 
 			  ARC_DEBUG ("arc_info: FIXED -> %s value = 0x%x "
 				     "@ 0x%x, for symbol %s\n",
@@ -1409,7 +1409,7 @@ elf_arc_relocate_section (bfd *		   output_bfd,
 				      "GOT_TLS_IE"),
 				     sym_value - sec_vma,
 				     htab->sgot->contents + entry->offset
-				     + (entry->existing_entries == MOD_AND_OFF ? 4 : 0),
+				     + (entry->existing_entries == TLS_GOT_MOD_AND_OFF ? 4 : 0),
 				     h->root.root.string);
 
 			  entry->processed = TRUE;
@@ -1806,7 +1806,7 @@ elf_arc_check_relocs (bfd *		         abfd,
 						  bfd_link_pic (info),
 						  NULL);
 		  new_got_entry_to_list (&(local_got_ents[r_symndx]),
-					 GOT_NORMAL, offset, NONE);
+					 GOT_NORMAL, offset, TLS_GOT_NONE);
 		}
 	    }
 	  else
@@ -1818,7 +1818,7 @@ elf_arc_check_relocs (bfd *		         abfd,
 		  bfd_vma offset =
 		    ADD_SYMBOL_REF_SEC_AND_RELOC (got, TRUE, h);
 		  new_got_entry_to_list (&h->got.glist,
-					 GOT_NORMAL, offset, NONE);
+					 GOT_NORMAL, offset, TLS_GOT_NONE);
 		}
 	    }
 	}
@@ -1847,7 +1847,7 @@ elf_arc_check_relocs (bfd *		         abfd,
 
 	  if (type != GOT_UNKNOWN && !symbol_has_entry_of_type (*list, type))
 	    {
-	      enum tls_got_entries entries = NONE;
+	      enum tls_got_entries entries = TLS_GOT_NONE;
 	      bfd_vma offset =
 		ADD_SYMBOL_REF_SEC_AND_RELOC (got, TRUE, h);
 
@@ -1855,11 +1855,11 @@ elf_arc_check_relocs (bfd *		         abfd,
 		{
 		  bfd_vma ATTRIBUTE_UNUSED notneeded =
 		    ADD_SYMBOL_REF_SEC_AND_RELOC (got, TRUE, h);
-		  entries = MOD_AND_OFF;
+		  entries = TLS_GOT_MOD_AND_OFF;
 		}
 
-	      if (entries == NONE)
-		entries = OFF;
+	      if (entries == TLS_GOT_NONE)
+		entries = TLS_GOT_OFF;
 
 	      new_got_entry_to_list (list, type, offset, entries);
 	    }
@@ -2256,16 +2256,16 @@ elf_arc_finish_dynamic_symbol (bfd * output_bfd,
 		}
 	      list->created_dyn_relocation = TRUE;
 	    }
-	  else if (list->existing_entries != NONE)
+	  else if (list->existing_entries != TLS_GOT_NONE)
 	    {
 	      struct elf_link_hash_table *htab = elf_hash_table (info);
 	      enum tls_got_entries e = list->existing_entries;
 
 	      BFD_ASSERT (list->type != GOT_TLS_GD
-			  || list->existing_entries == MOD_AND_OFF);
+			  || list->existing_entries == TLS_GOT_MOD_AND_OFF);
 
 	      bfd_vma dynindx = h->dynindx == -1 ? 0 : h->dynindx;
-	      if (e == MOD_AND_OFF || e == MOD)
+	      if (e == TLS_GOT_MOD_AND_OFF || e == TLS_GOT_MOD)
 		{
 		  ADD_RELA (output_bfd, got, got_offset, dynindx,
 			    R_ARC_TLS_DTPMOD, 0);
@@ -2277,7 +2277,7 @@ GOT_OFFSET = 0x%x, GOT_VMA = 0x%x, INDEX = %d, ADDEND = 0x%x\n",
 			     + htab->sgot->output_offset + got_offset,
 			     dynindx, 0);
 		}
-	      if (e == MOD_AND_OFF || e == OFF)
+	      if (e == TLS_GOT_MOD_AND_OFF || e == TLS_GOT_OFF)
 		{
 		  bfd_vma addend = 0;
 		  if (list->type == GOT_TLS_IE)
@@ -2285,7 +2285,7 @@ GOT_OFFSET = 0x%x, GOT_VMA = 0x%x, INDEX = %d, ADDEND = 0x%x\n",
 					 htab->sgot->contents + got_offset);
 
 		  ADD_RELA (output_bfd, got,
-			    got_offset + (e == MOD_AND_OFF ? 4 : 0),
+			    got_offset + (e == TLS_GOT_MOD_AND_OFF ? 4 : 0),
 			    dynindx,
 			    (list->type == GOT_TLS_IE ?
 			     R_ARC_TLS_TPOFF : R_ARC_TLS_DTPOFF),
-- 
2.5.1

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

* [PATCH 1/3] opcodes/arc: Compute insn lengths in disassembler
  2016-04-07 11:05 ` [PATCH " Andrew Burgess
@ 2016-04-07 11:05   ` Andrew Burgess
  2016-04-07 11:27     ` Claudiu Zissulescu
  2016-04-12  9:27     ` Claudiu Zissulescu
  2016-04-07 11:05   ` [PATCH 2/3] bfd/arc: Rename enum entries to avoid conflicts Andrew Burgess
                     ` (4 subsequent siblings)
  5 siblings, 2 replies; 20+ messages in thread
From: Andrew Burgess @ 2016-04-07 11:05 UTC (permalink / raw)
  To: binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca, Andrew Burgess

Change the mechanism by which the instruction length is figured out in
the arc disassembler.  Previously we maintained a micro-decoder that
contained enough knowledge to figure out the instruction length.  The
problem is that as instructions are added for different arc
architectures, more and more special cases need to be added to this
micro disassembler.

This commit changes to a table based lookup for the instruction length,
the table maps from the major opcode of the instruction to the
instruction length.  The table is filled at run-time, the first time
that the table is needed.

The table is self-checking while being built, it only contains the
instructions for the current architecture being disassembled, and if
different instructions with the same major opcode have different
lengths, this will cause an error while the table is being filled.

opcodes/ChangeLog:

	* arc-dis.c (arc_insn_length): New function.
	(print_insn_arc): Use arc_insn_length.
---
 opcodes/ChangeLog |  5 ++++
 opcodes/arc-dis.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
index 5f8fa42..f7cff5f 100644
--- a/opcodes/arc-dis.c
+++ b/opcodes/arc-dis.c
@@ -102,6 +102,53 @@ special_flag_p (const char *opname,
   return 0;
 }
 
+/* Calculate the instruction length for an instruction starting with MSB
+   and LSB, the most and least significant byte.  The ISA_MASK is used to
+   filter the instructions considered to only those that are part of the
+   current architecture.
+
+   The instruction lengths are calculated from the ARC_OPCODE table, and
+   cached for later use.  */
+
+static int
+arc_insn_length (unsigned isa_mask, bfd_byte msb,
+                 bfd_byte lsb ATTRIBUTE_UNUSED)
+{
+  static int tbl [32]; /* 0x00 -> 0x1f */
+  static int tbl_init = 0;
+#define MAJOR_OPCODE_VALUE(BYTE) (BYTE >> 3)
+
+  if (!tbl_init)
+    {
+      unsigned i;
+
+      tbl_init = 1;
+      memset (tbl, 0, sizeof (tbl));
+      for (i = 0; i < arc_num_opcodes; i++)
+        {
+          const struct arc_opcode *opcode;
+          bfd_byte b;
+          int is_short, len;
+          unsigned value;
+
+          opcode = &arc_opcodes[i];
+          if (!(opcode->cpu & isa_mask))
+            continue;
+
+          value = opcode->opcode;
+          is_short = ARC_SHORT (opcode->mask);
+          b = MAJOR_OPCODE_VALUE (value >> (is_short ? 8 : 24));
+          len = (is_short ? 2 : 4);
+          assert (tbl [b] == 0 || tbl [b] == len);
+          tbl [b] = len;
+        }
+    }
+
+  return tbl [MAJOR_OPCODE_VALUE (msb)];
+
+#undef MAJOR_OPCODE_VALUE
+}
+
 /* Disassemble ARC instructions.  */
 
 static int
@@ -218,20 +265,19 @@ print_insn_arc (bfd_vma memaddr,
       return size;
     }
 
-  if (   (((buffer[lowbyte] & 0xf8) > 0x38)
-       && ((buffer[lowbyte] & 0xf8) != 0x48))
-      || ((info->mach == bfd_mach_arc_arcv2)
-	  && ((buffer[lowbyte] & 0xF8) == 0x48)) /* FIXME! ugly.  */
-      )
+  insnLen = arc_insn_length (isa_mask, buffer[lowbyte], buffer[highbyte]);
+  switch (insnLen)
     {
-      /* This is a short instruction.  */
-      insnLen = 2;
+    case 2:
       insn[0] = (buffer[lowbyte] << 8) | buffer[highbyte];
-    }
-  else
-    {
-      insnLen = 4;
+      break;
 
+    default:
+      /* An unknown instruction is treated as being length 4.  This is
+         possibly not the best solution, but matches the behaviour that was
+         in place before the table based instruction length look-up was
+         introduced.  */
+    case 4:
       /* This is a long instruction: Read the remaning 2 bytes.  */
       status = (*info->read_memory_func) (memaddr + 2, &buffer[2], 2, info);
       if (status != 0)
@@ -240,6 +286,7 @@ print_insn_arc (bfd_vma memaddr,
 	  return -1;
 	}
       insn[0] = ARRANGE_ENDIAN (info, buffer);
+      break;
     }
 
   /* Set some defaults for the insn info.  */
-- 
2.5.1

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

* RE: [PATCH 2/3] bfd/arc: Rename enum entries to avoid conflicts
  2016-04-07 11:05   ` [PATCH 2/3] bfd/arc: Rename enum entries to avoid conflicts Andrew Burgess
@ 2016-04-07 11:20     ` Claudiu Zissulescu
  0 siblings, 0 replies; 20+ messages in thread
From: Claudiu Zissulescu @ 2016-04-07 11:20 UTC (permalink / raw)
  To: Andrew Burgess, binutils; +Cc: Cupertino.Miranda, noamca

Hi,
 
> bfd/ChangeLog:
> 
> 	* elf32-arc.c (tls_got_entries): Add 'TLS_GOT_' prefix to all
> 	entries.
> 	(elf_arc_relocate_section): Update enum uses.
> 	(elf_arc_check_relocs): Likewise.
> 	(elf_arc_finish_dynamic_symbol): Likewise.

This looks alright to me. 

Best,
Claudiu

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

* RE: [PATCH 1/3] opcodes/arc: Compute insn lengths in disassembler
  2016-04-07 11:05   ` [PATCH 1/3] opcodes/arc: Compute insn lengths in disassembler Andrew Burgess
@ 2016-04-07 11:27     ` Claudiu Zissulescu
  2016-04-12  9:27     ` Claudiu Zissulescu
  1 sibling, 0 replies; 20+ messages in thread
From: Claudiu Zissulescu @ 2016-04-07 11:27 UTC (permalink / raw)
  To: Andrew Burgess, binutils; +Cc: Cupertino.Miranda, noamca

Hi,

> +static int
> +arc_insn_length (unsigned isa_mask, bfd_byte msb,
> +                 bfd_byte lsb ATTRIBUTE_UNUSED)
> +{
> +  static int tbl [32]; /* 0x00 -> 0x1f */
> +  static int tbl_init = 0;

Please use bfd_boolean instead of int for tbl_init.

Otherwise, it seems sane to me.

Best,
Claudiu

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

* Re: [PATCH 3/3] arc/nps400 : New cmem instructions and associated relocation
  2016-04-07 11:05   ` [PATCH 3/3] arc/nps400 : New cmem instructions and associated relocation Andrew Burgess
@ 2016-04-11 12:55     ` Cupertino Miranda
  0 siblings, 0 replies; 20+ messages in thread
From: Cupertino Miranda @ 2016-04-11 12:55 UTC (permalink / raw)
  To: Andrew Burgess, binutils; +Cc: Claudiu.Zissulescu, Francois Bedard, noamca

[-- Attachment #1: Type: text/plain, Size: 20357 bytes --]

Hi Andrew,

Like you mention in the email, indeed we are a little keen to respect
our initial design decisions regarding how the relocations are handled. ;-)

After reviewing your patch, IMHO the following things would be nicer.

- Your new function should not be allowed to change the relocation
value. Moreover, I believe that the masking you do is not necessary as
replace_bits16 function, used for the new relocation in arc-reloc.def,
already performs the masking on the 16 less significant bits.

- I would prefer the checks to be done next to the existing relocation
overflow detection, inside the do_relocation function.

You can find all this suggestions in the attached patch with the
refereed changes.
Please be aware the changes in the patch might not generate the exact
same stdout/stderr messages, so the tests must be changed.

Best regards,
Cupertino

On 04/07/2016 01:05 PM, Andrew Burgess wrote:
> This patch adds some new arc/nps instructions.  These new instructions
> require a new relocation, and this new relocation requires a specific
> check of the computed relocation value.
>
> To achieve this specific check (that the most significant 2 bytes are
> a particular value), I added a new function, arc_final_relocation_checks.
>
> I know that the Synopsys team have a particular design in mind for how
> they want ARC relocations to be handled, and this new function might
> not fit within their desired design.
>
> I'm not particularly attached to the new function, I'm open to any
> alternative suggestions for how to achieve the same end result if the
> Synopsys team would like to make a suggestion.  In the absence of any
> alternative suggestions though, I believe the code I offer below
> should be acceptable for inclusion.
>
> Thanks,
> Andrew
>
>
> ---
>
> Add support for arc/nps400 cmem instructions, these load and store
> instructions are hard-wired to access "0x57f00000 + 16-bit-offset".
>
> Supporting this relocation required some additions to the arc relocation
> handling in the bfd library, as well as the standard changes required to
> add a new relocation type.
>
> There's a test of the new instructions in the assembler, and a test of
> the relocation in the linker.
>
> bfd/ChangeLog:
>
> 	* reloc.c: Add BFD_RELOC_ARC_NPS_CMEM16 entry.
> 	* bfd-in2.h: Regenerate.
> 	* libbfd.h: Regenerate.
> 	* elf32-arc.c: Add 'opcode/arc.h' include.
> 	(struct arc_relocation_data): Add symbol_name.
> 	(arc_final_relocation_checks): New function.
> 	(arc_do_relocation): Update ARC_RELOC_HOWTO definition to call
> 	arc_final_relocation_checks.
> 	(elf_arc_relocate_section): Setup symbol_name in reloc_data.
>
> gas/ChangeLog:
>
> 	* testsuite/gas/arc/nps400-3.d: New file.
> 	* testsuite/gas/arc/nps400-3.s: New file.
>
> include/ChangeLog:
>
> 	* elf/arc-reloc.def: Add ARC_NPS_CMEM16 reloc.
> 	* opcode/arc.h (NPS_CMEM_HIGH_VALUE): Define.
>
> ld/ChangeLog:
>
> 	* testsuite/ld-arc/arc.exp: New file.
> 	* testsuite/ld-arc/nps-1.s: New file.
> 	* testsuite/ld-arc/nps-1a.d: New file.
> 	* testsuite/ld-arc/nps-1b.d: New file.
> 	* testsuite/ld-arc/nps-1b.err: New file.
>
> opcodes/ChangeLog:
>
> 	* arc-nps400-tbl.h: Add xldb, xldw, xld, xstb, xstw, and xst
> 	instructions.
> 	* arc-opc.c (insert_nps_cmem_uimm16): New function.
> 	(extract_nps_cmem_uimm16): New function.
> 	(arc_operands): Add NPS_XLDST_UIMM16 operand.
> ---
>  bfd/ChangeLog                    | 12 +++++++++
>  bfd/bfd-in2.h                    |  1 +
>  bfd/elf32-arc.c                  | 58 ++++++++++++++++++++++++++++++++++++++++
>  bfd/libbfd.h                     |  1 +
>  bfd/reloc.c                      |  2 ++
>  gas/ChangeLog                    |  5 ++++
>  gas/testsuite/gas/arc/nps400-3.d | 56 ++++++++++++++++++++++++++++++++++++++
>  gas/testsuite/gas/arc/nps400-3.s | 23 ++++++++++++++++
>  include/ChangeLog                |  5 ++++
>  include/elf/arc-reloc.def        |  7 +++++
>  include/opcode/arc.h             |  3 +++
>  ld/ChangeLog                     |  8 ++++++
>  ld/testsuite/ld-arc/arc.exp      | 30 +++++++++++++++++++++
>  ld/testsuite/ld-arc/nps-1.s      | 10 +++++++
>  ld/testsuite/ld-arc/nps-1a.d     | 16 +++++++++++
>  ld/testsuite/ld-arc/nps-1b.d     |  4 +++
>  ld/testsuite/ld-arc/nps-1b.err   |  1 +
>  opcodes/ChangeLog                |  8 ++++++
>  opcodes/arc-nps400-tbl.h         | 12 +++++++++
>  opcodes/arc-opc.c                | 22 +++++++++++++++
>  20 files changed, 284 insertions(+)
>  create mode 100644 gas/testsuite/gas/arc/nps400-3.d
>  create mode 100644 gas/testsuite/gas/arc/nps400-3.s
>  create mode 100644 ld/testsuite/ld-arc/arc.exp
>  create mode 100644 ld/testsuite/ld-arc/nps-1.s
>  create mode 100644 ld/testsuite/ld-arc/nps-1a.d
>  create mode 100644 ld/testsuite/ld-arc/nps-1b.d
>  create mode 100644 ld/testsuite/ld-arc/nps-1b.err
>
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index f02e2aa..6532f6e 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -3721,6 +3721,7 @@ pc-relative or some form of GOT-indirect relocation.  */
>    BFD_RELOC_ARC_TLS_LE_32,
>    BFD_RELOC_ARC_S25W_PCREL_PLT,
>    BFD_RELOC_ARC_S21H_PCREL_PLT,
> +  BFD_RELOC_ARC_NPS_CMEM16,
>  
>  /* ADI Blackfin 16 bit immediate absolute reloc.  */
>    BFD_RELOC_BFIN_16_IMM,
> diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c
> index 263effc..52bf5dc 100644
> --- a/bfd/elf32-arc.c
> +++ b/bfd/elf32-arc.c
> @@ -26,6 +26,7 @@
>  #include "elf/arc.h"
>  #include "libiberty.h"
>  #include "opcode/arc-func.h"
> +#include "opcode/arc.h"
>  #include "arc-plt.h"
>  
>  #ifdef DEBUG
> @@ -722,6 +723,8 @@ struct arc_relocation_data
>    bfd_signed_vma  got_symbol_vma;
>  
>    bfd_boolean	  should_relocate;
> +
> +  const char *    symbol_name;
>  };
>  
>  static void
> @@ -873,11 +876,61 @@ middle_endian_convert (bfd_vma insn, bfd_boolean do_it)
>        ARC_DEBUG ("after  = 0x%08x\n", (unsigned int) insn); \
>      }
>  
> +/* Perform final checks, and possible adjustment to the relocation value
> +   before applying the relocation to the instruction.  */
> +
> +static inline bfd_reloc_status_type
> +arc_final_relocation_checks (bfd_signed_vma *relocation,
> +			     const struct arc_relocation_data reloc_data,
> +			     struct bfd_link_info *info ATTRIBUTE_UNUSED)
> +{
> +  switch (reloc_data.howto->type)
> +    {
> +    case R_ARC_NPS_CMEM16:
> +      if (((*relocation >> 16) & 0xffff) != NPS_CMEM_HIGH_VALUE)
> +        {
> +          if (reloc_data.reloc_addend == 0)
> +            (*_bfd_error_handler)
> +              (_("%B(%A+0x%lx): CMEM relocation to `%s' is invalid, "
> +                 "16 MSB should be 0x%04x (value is 0x%lx)"),
> +               reloc_data.input_section->owner,
> +               reloc_data.input_section,
> +               reloc_data.reloc_offset,
> +               reloc_data.symbol_name,
> +               NPS_CMEM_HIGH_VALUE,
> +               (*relocation));
> +          else
> +            (*_bfd_error_handler)
> +              (_("%B(%A+0x%lx): CMEM relocation to `%s+0x%lx' is invalid, "
> +                 "16 MSB should be 0x%04x (value is 0x%lx)"),
> +               reloc_data.input_section->owner,
> +               reloc_data.input_section,
> +               reloc_data.reloc_offset,
> +               reloc_data.symbol_name,
> +               reloc_data.reloc_addend,
> +               NPS_CMEM_HIGH_VALUE,
> +               (*relocation));
> +          return bfd_reloc_overflow;
> +        }
> +      *relocation &= 0xffff;
> +      break;
> +
> +    default:
> +      break;
> +    }
> +
> +  return bfd_reloc_ok;
> +}
> +
>  #define ARC_RELOC_HOWTO(TYPE, VALUE, SIZE, BITSIZE, RELOC_FUNCTION, OVERFLOW, FORMULA) \
>    case R_##TYPE: \
>      { \
>        bfd_signed_vma bitsize ATTRIBUTE_UNUSED = BITSIZE; \
> +      bfd_reloc_status_type status; \
>        relocation = FORMULA  ; \
> +      status = arc_final_relocation_checks (&relocation, reloc_data, info); \
> +      if (status != bfd_reloc_ok) \
> +        return status; \
>        PRINT_DEBUG_RELOC_INFO_BEFORE (#FORMULA, #TYPE); \
>        insn = middle_endian_convert (insn, IS_ME (#FORMULA, abfd)); \
>        insn = (* get_replace_function (abfd, TYPE)) (insn, relocation); \
> @@ -1168,6 +1221,10 @@ elf_arc_relocate_section (bfd *		   output_bfd,
>  
>  	  reloc_data.sym_value = sym->st_value;
>  	  reloc_data.sym_section = sec;
> +	  reloc_data.symbol_name =
> +            bfd_elf_string_from_elf_section (input_bfd,
> +                                             symtab_hdr->sh_link,
> +                                             sym->st_name);
>  
>  	  /* Mergeable section handling.  */
>  	  if ((sec->flags & SEC_MERGE)
> @@ -1284,6 +1341,7 @@ elf_arc_relocate_section (bfd *		   output_bfd,
>  	    h = (struct elf_link_hash_entry *) h->root.u.i.link;
>  
>  	  BFD_ASSERT ((h->dynindx == -1) >= (h->forced_local != 0));
> +	  reloc_data.symbol_name = h->root.root.string;
>  	  /* If we have encountered a definition for this symbol.  */
>  	  if (h->root.type == bfd_link_hash_defined
>  	      || h->root.type == bfd_link_hash_defweak)
> diff --git a/bfd/libbfd.h b/bfd/libbfd.h
> index d7183d3..16c0aee 100644
> --- a/bfd/libbfd.h
> +++ b/bfd/libbfd.h
> @@ -1738,6 +1738,7 @@ static const char *const bfd_reloc_code_real_names[] = { "@@uninitialized@@",
>    "BFD_RELOC_ARC_TLS_LE_32",
>    "BFD_RELOC_ARC_S25W_PCREL_PLT",
>    "BFD_RELOC_ARC_S21H_PCREL_PLT",
> +  "BFD_RELOC_ARC_NPS_CMEM16",
>    "BFD_RELOC_BFIN_16_IMM",
>    "BFD_RELOC_BFIN_16_HIGH",
>    "BFD_RELOC_BFIN_4_PCREL",
> diff --git a/bfd/reloc.c b/bfd/reloc.c
> index 0135c04..c3b713b 100644
> --- a/bfd/reloc.c
> +++ b/bfd/reloc.c
> @@ -3668,6 +3668,8 @@ ENUMX
>    BFD_RELOC_ARC_S25W_PCREL_PLT
>  ENUMX
>    BFD_RELOC_ARC_S21H_PCREL_PLT
> +ENUMX
> +  BFD_RELOC_ARC_NPS_CMEM16
>  ENUMDOC
>    ARC relocs.
>  
> diff --git a/gas/testsuite/gas/arc/nps400-3.d b/gas/testsuite/gas/arc/nps400-3.d
> new file mode 100644
> index 0000000..ea52554
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/nps400-3.d
> @@ -0,0 +1,56 @@
> +#as: -mcpu=nps400
> +#objdump: -dr
> +
> +.*: +file format .*arc.*
> +
> +Disassembly of section .text:
> +
> +[0-9a-f]+ <.*>:
> +   0:	5988 0000           	xldb	r12,\[0x57f00000\]
> +   4:	5ae8 ffff           	xldb	r23,\[0x57f0ffff\]
> +   8:	5868 0000           	xldb	r3,\[0x57f00000\]
> +   c:	5968 ffff           	xldb	r11,\[0x57f0ffff\]
> +  10:	5a88 0000           	xldb	r20,\[0x57f00000\]
> +			10: R_ARC_NPS_CMEM16	foo
> +  14:	5828 0000           	xldb	r1,\[0x57f00000\]
> +			14: R_ARC_NPS_CMEM16	foo\+0x20
> +  18:	5989 0000           	xldw	r12,\[0x57f00000\]
> +  1c:	5ae9 ffff           	xldw	r23,\[0x57f0ffff\]
> +  20:	5869 0000           	xldw	r3,\[0x57f00000\]
> +  24:	5969 ffff           	xldw	r11,\[0x57f0ffff\]
> +  28:	5a89 0000           	xldw	r20,\[0x57f00000\]
> +			28: R_ARC_NPS_CMEM16	foo
> +  2c:	5829 0000           	xldw	r1,\[0x57f00000\]
> +			2c: R_ARC_NPS_CMEM16	foo\+0x20
> +  30:	598a 0000           	xld	r12,\[0x57f00000\]
> +  34:	5aea ffff           	xld	r23,\[0x57f0ffff\]
> +  38:	586a 0000           	xld	r3,\[0x57f00000\]
> +  3c:	596a ffff           	xld	r11,\[0x57f0ffff\]
> +  40:	5a8a 0000           	xld	r20,\[0x57f00000\]
> +			40: R_ARC_NPS_CMEM16	foo
> +  44:	582a 0000           	xld	r1,\[0x57f00000\]
> +			44: R_ARC_NPS_CMEM16	foo\+0x20
> +  48:	598c 0000           	xstb	r12,\[0x57f00000\]
> +  4c:	5aec ffff           	xstb	r23,\[0x57f0ffff\]
> +  50:	586c 0000           	xstb	r3,\[0x57f00000\]
> +  54:	596c ffff           	xstb	r11,\[0x57f0ffff\]
> +  58:	5a8c 0000           	xstb	r20,\[0x57f00000\]
> +			58: R_ARC_NPS_CMEM16	foo
> +  5c:	582c 0000           	xstb	r1,\[0x57f00000\]
> +			5c: R_ARC_NPS_CMEM16	foo\+0x20
> +  60:	598d 0000           	xstw	r12,\[0x57f00000\]
> +  64:	5aed ffff           	xstw	r23,\[0x57f0ffff\]
> +  68:	586d 0000           	xstw	r3,\[0x57f00000\]
> +  6c:	596d ffff           	xstw	r11,\[0x57f0ffff\]
> +  70:	5a8d 0000           	xstw	r20,\[0x57f00000\]
> +			70: R_ARC_NPS_CMEM16	foo
> +  74:	582d 0000           	xstw	r1,\[0x57f00000\]
> +			74: R_ARC_NPS_CMEM16	foo\+0x20
> +  78:	598e 0000           	xst	r12,\[0x57f00000\]
> +  7c:	5aee ffff           	xst	r23,\[0x57f0ffff\]
> +  80:	586e 0000           	xst	r3,\[0x57f00000\]
> +  84:	596e ffff           	xst	r11,\[0x57f0ffff\]
> +  88:	5a8e 0000           	xst	r20,\[0x57f00000\]
> +			88: R_ARC_NPS_CMEM16	foo
> +  8c:	582e 0000           	xst	r1,\[0x57f00000\]
> +			8c: R_ARC_NPS_CMEM16	foo\+0x20
> diff --git a/gas/testsuite/gas/arc/nps400-3.s b/gas/testsuite/gas/arc/nps400-3.s
> new file mode 100644
> index 0000000..6840223
> --- /dev/null
> +++ b/gas/testsuite/gas/arc/nps400-3.s
> @@ -0,0 +1,23 @@
> +        .macro  xldst_test mnem
> +        \mnem r12, [ 0x0 ]
> +        \mnem r23, [ 0xffff ]
> +        \mnem r3, [ 0x57f00000 ]
> +        \mnem r11, [ 0x57f0ffff ]
> +        \mnem r20, [ foo ]
> +        \mnem r1, [ foo + 0x20 ]
> +        .endm
> +
> +        .text
> +        ;; xldb
> +        xldst_test xldb
> +        ;; xldw
> +        xldst_test xldw
> +        ;; xld
> +        xldst_test xld
> +        ;; xstb
> +        xldst_test xstb
> +        ;; xstw
> +        xldst_test xstw
> +        ;; xst
> +        xldst_test xst
> +
> diff --git a/include/elf/arc-reloc.def b/include/elf/arc-reloc.def
> index 36a3516..b271e0e 100644
> --- a/include/elf/arc-reloc.def
> +++ b/include/elf/arc-reloc.def
> @@ -490,3 +490,10 @@ ARC_RELOC_HOWTO(ARC_S21H_PCREL_PLT, 77, \
>                  replace_disp21h, \
>                  signed, \
>                  ( ME ( ( ( ( L + A ) - P ) >> 1 ) ) ))
> +
> +ARC_RELOC_HOWTO(ARC_NPS_CMEM16, 78, \
> +                2, \
> +                16, \
> +                replace_bits16, \
> +                bitfield, \
> +                ( S + A ))
> diff --git a/include/opcode/arc.h b/include/opcode/arc.h
> index bc0e1ad..2827c96 100644
> --- a/include/opcode/arc.h
> +++ b/include/opcode/arc.h
> @@ -423,6 +423,9 @@ extern const unsigned arc_num_aux_regs;
>  extern const struct arc_opcode arc_relax_opcodes[];
>  extern const unsigned arc_num_relax_opcodes;
>  
> +/* Macro used for generating one class of NPS instructions.  */
> +#define NPS_CMEM_HIGH_VALUE 0x57f0
> +
>  /* Macros to help generating regular pattern instructions.  */
>  #define FIELDA(word) (word & 0x3F)
>  #define FIELDB(word) (((word & 0x07) << 24) | (((word >> 3) & 0x07) << 12))
> diff --git a/ld/testsuite/ld-arc/arc.exp b/ld/testsuite/ld-arc/arc.exp
> new file mode 100644
> index 0000000..0cf6228
> --- /dev/null
> +++ b/ld/testsuite/ld-arc/arc.exp
> @@ -0,0 +1,30 @@
> +# Copyright (C) 2016 Free Software Foundation, Inc.
> +#
> +# This file is part of the GNU Binutils.
> +#
> +# 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, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
> +# MA 02110-1301, USA.
> +#
> +
> +if { ![istarget arc*-*-*] } {
> +    return
> +}
> +
> +set arc_test_list [lsort [glob -nocomplain $srcdir/$subdir/*.d]]
> +foreach arc_test $arc_test_list {
> +    verbose [file rootname $arc_test]
> +    run_dump_test [file rootname $arc_test]
> +}
> +
> diff --git a/ld/testsuite/ld-arc/nps-1.s b/ld/testsuite/ld-arc/nps-1.s
> new file mode 100644
> index 0000000..295fa2c
> --- /dev/null
> +++ b/ld/testsuite/ld-arc/nps-1.s
> @@ -0,0 +1,10 @@
> +        .text
> +        .global __start
> +__start:
> +        xldb r10, [ foo ]
> +        xldw r10, [ foo ]
> +        xld r10, [ foo ]
> +        xstb r10, [ foo ]
> +        xstw r10, [ foo ]
> +        xst r10, [ foo ]
> +
> diff --git a/ld/testsuite/ld-arc/nps-1a.d b/ld/testsuite/ld-arc/nps-1a.d
> new file mode 100644
> index 0000000..932842b
> --- /dev/null
> +++ b/ld/testsuite/ld-arc/nps-1a.d
> @@ -0,0 +1,16 @@
> +#source: nps-1.s
> +#as: -mcpu=nps400
> +#ld: -defsym=foo=0x57f03000
> +#objdump: -d
> +
> +.*: +file format .*arc.*
> +
> +Disassembly of section .text:
> +
> +[0-9a-f]+ <.*>:
> +   10074:	5948 3000           	xldb	r10,\[0x57f03000\]
> +   10078:	5949 3000           	xldw	r10,\[0x57f03000\]
> +   1007c:	594a 3000           	xld	r10,\[0x57f03000\]
> +   10080:	594c 3000           	xstb	r10,\[0x57f03000\]
> +   10084:	594d 3000           	xstw	r10,\[0x57f03000\]
> +   10088:	594e 3000           	xst	r10,\[0x57f03000\]
> diff --git a/ld/testsuite/ld-arc/nps-1b.d b/ld/testsuite/ld-arc/nps-1b.d
> new file mode 100644
> index 0000000..56c29ae
> --- /dev/null
> +++ b/ld/testsuite/ld-arc/nps-1b.d
> @@ -0,0 +1,4 @@
> +#source: nps-1.s
> +#as: -mcpu=nps400
> +#ld: -defsym=foo=0x56f03000
> +#error_output: nps-1b.err
> diff --git a/ld/testsuite/ld-arc/nps-1b.err b/ld/testsuite/ld-arc/nps-1b.err
> new file mode 100644
> index 0000000..a44b3c1
> --- /dev/null
> +++ b/ld/testsuite/ld-arc/nps-1b.err
> @@ -0,0 +1 @@
> +.*\.o\(\.text\+0x0\): CMEM relocation to `foo' is invalid, 16 MSB should be 0x57f0 \(value is 0x56f03000\)
> diff --git a/opcodes/arc-nps400-tbl.h b/opcodes/arc-nps400-tbl.h
> index dc7b066..f925ca3 100644
> --- a/opcodes/arc-nps400-tbl.h
> +++ b/opcodes/arc-nps400-tbl.h
> @@ -123,3 +123,15 @@
>  
>  /* crc32<.r> 0,limm,u6		00111 110 01 110100 R 111 uuuuuu 111110 */
>  { "crc32", 0x3e74703e, 0xffff703f, ARC_OPCODE_NPS400, BITOP, NONE, { ZA, LIMM, UIMM6_20 }, { C_NPS_R }},
> +
> +/****      Load / Store From (0x57f00000 + Offset) Instructions       ****/
> +
> +#define XLDST_LIKE(NAME,SUBOP2)                                          \
> +  { NAME, (0x58000000 | (SUBOP2 << 16)), 0xf81f0000, ARC_OPCODE_NPS400, MEMORY, NONE, { NPS_R_DST, BRAKET, NPS_XLDST_UIMM16, BRAKETdup }, { 0 }},
> +
> +XLDST_LIKE("xldb", 0x8)
> +XLDST_LIKE("xldw", 0x9)
> +XLDST_LIKE("xld", 0xa)
> +XLDST_LIKE("xstb", 0xc)
> +XLDST_LIKE("xstw", 0xd)
> +XLDST_LIKE("xst", 0xe)
> diff --git a/opcodes/arc-opc.c b/opcodes/arc-opc.c
> index f182318..8173f8a 100644
> --- a/opcodes/arc-opc.c
> +++ b/opcodes/arc-opc.c
> @@ -838,6 +838,25 @@ extract_nps_dst_pos_and_size (unsigned insn ATTRIBUTE_UNUSED,
>    return (insn & 0x1f);
>  }
>  
> +static unsigned
> +insert_nps_cmem_uimm16 (unsigned insn ATTRIBUTE_UNUSED,
> +                        int value ATTRIBUTE_UNUSED,
> +                        const char **errmsg ATTRIBUTE_UNUSED)
> +{
> +  int top = (value >> 16) & 0xffff;
> +  if (top != 0x0 && top != NPS_CMEM_HIGH_VALUE)
> +    *errmsg = _("invalid value for CMEM ld/st immediate");
> +  insn |= (value & 0xffff);
> +  return insn;
> +}
> +
> +static int
> +extract_nps_cmem_uimm16 (unsigned insn ATTRIBUTE_UNUSED,
> +                         bfd_boolean * invalid ATTRIBUTE_UNUSED)
> +{
> +  return (NPS_CMEM_HIGH_VALUE << 16) | (insn & 0xffff);
> +}
> +
>  /* Include the generic extract/insert functions.  Order is important
>     as some of the functions present in the .h may be disabled via
>     defines.  */
> @@ -1442,6 +1461,9 @@ const struct arc_operand arc_operands[] =
>  
>  #define NPS_RFLT_UIMM6		(NPS_UIMM16 + 1)
>    { 6, 6, 0, ARC_OPERAND_UNSIGNED | ARC_OPERAND_NCHK, insert_nps_rflt_uimm6, extract_nps_rflt_uimm6 },
> +
> +#define NPS_XLDST_UIMM16	(NPS_RFLT_UIMM6 + 1)
> +  { 16, 0, BFD_RELOC_ARC_NPS_CMEM16, ARC_OPERAND_UNSIGNED | ARC_OPERAND_NCHK, insert_nps_cmem_uimm16, extract_nps_cmem_uimm16 },
>  };
>  
>  const unsigned arc_num_operands = ARRAY_SIZE (arc_operands);


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: changes.patch --]
[-- Type: text/x-patch; name="changes.patch", Size: 4503 bytes --]

diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c
index 52bf5dc..0d06634 100644
--- a/bfd/elf32-arc.c
+++ b/bfd/elf32-arc.c
@@ -791,6 +791,51 @@ middle_endian_convert (bfd_vma insn, bfd_boolean do_it)
   return insn;
 }
 
+/* Perform final checks, and possible adjustment to the relocation value
+   before applying the relocation to the instruction.  */
+
+static inline bfd_reloc_status_type
+arc_extra_overflow_checks (bfd_signed_vma relocation,
+			     const struct arc_relocation_data reloc_data,
+			     struct bfd_link_info *info ATTRIBUTE_UNUSED)
+{
+  switch (reloc_data.howto->type)
+    {
+    case R_ARC_NPS_CMEM16:
+      if (((relocation >> 16) & 0xffff) != NPS_CMEM_HIGH_VALUE)
+        {
+          if (reloc_data.reloc_addend == 0)
+            (*_bfd_error_handler)
+              (_("%B(%A+0x%lx): CMEM relocation to `%s' is invalid, "
+                 "16 MSB should be 0x%04x (value is 0x%lx)"),
+               reloc_data.input_section->owner,
+               reloc_data.input_section,
+               reloc_data.reloc_offset,
+               reloc_data.symbol_name,
+               NPS_CMEM_HIGH_VALUE,
+               (relocation));
+          else
+            (*_bfd_error_handler)
+              (_("%B(%A+0x%lx): CMEM relocation to `%s+0x%lx' is invalid, "
+                 "16 MSB should be 0x%04x (value is 0x%lx)"),
+               reloc_data.input_section->owner,
+               reloc_data.input_section,
+               reloc_data.reloc_offset,
+               reloc_data.symbol_name,
+               reloc_data.reloc_addend,
+               NPS_CMEM_HIGH_VALUE,
+               (relocation));
+          return bfd_reloc_overflow;
+        }
+      break;
+
+    default:
+      break;
+    }
+
+  return bfd_reloc_ok;
+}
+
 #define ME(reloc) (reloc)
 
 #define IS_ME(FORMULA,BFD) ((strstr (FORMULA, "ME") != NULL) \
@@ -876,61 +921,12 @@ middle_endian_convert (bfd_vma insn, bfd_boolean do_it)
       ARC_DEBUG ("after  = 0x%08x\n", (unsigned int) insn); \
     }
 
-/* Perform final checks, and possible adjustment to the relocation value
-   before applying the relocation to the instruction.  */
-
-static inline bfd_reloc_status_type
-arc_final_relocation_checks (bfd_signed_vma *relocation,
-			     const struct arc_relocation_data reloc_data,
-			     struct bfd_link_info *info ATTRIBUTE_UNUSED)
-{
-  switch (reloc_data.howto->type)
-    {
-    case R_ARC_NPS_CMEM16:
-      if (((*relocation >> 16) & 0xffff) != NPS_CMEM_HIGH_VALUE)
-        {
-          if (reloc_data.reloc_addend == 0)
-            (*_bfd_error_handler)
-              (_("%B(%A+0x%lx): CMEM relocation to `%s' is invalid, "
-                 "16 MSB should be 0x%04x (value is 0x%lx)"),
-               reloc_data.input_section->owner,
-               reloc_data.input_section,
-               reloc_data.reloc_offset,
-               reloc_data.symbol_name,
-               NPS_CMEM_HIGH_VALUE,
-               (*relocation));
-          else
-            (*_bfd_error_handler)
-              (_("%B(%A+0x%lx): CMEM relocation to `%s+0x%lx' is invalid, "
-                 "16 MSB should be 0x%04x (value is 0x%lx)"),
-               reloc_data.input_section->owner,
-               reloc_data.input_section,
-               reloc_data.reloc_offset,
-               reloc_data.symbol_name,
-               reloc_data.reloc_addend,
-               NPS_CMEM_HIGH_VALUE,
-               (*relocation));
-          return bfd_reloc_overflow;
-        }
-      *relocation &= 0xffff;
-      break;
-
-    default:
-      break;
-    }
-
-  return bfd_reloc_ok;
-}
 
 #define ARC_RELOC_HOWTO(TYPE, VALUE, SIZE, BITSIZE, RELOC_FUNCTION, OVERFLOW, FORMULA) \
   case R_##TYPE: \
     { \
       bfd_signed_vma bitsize ATTRIBUTE_UNUSED = BITSIZE; \
-      bfd_reloc_status_type status; \
       relocation = FORMULA  ; \
-      status = arc_final_relocation_checks (&relocation, reloc_data, info); \
-      if (status != bfd_reloc_ok) \
-        return status; \
       PRINT_DEBUG_RELOC_INFO_BEFORE (#FORMULA, #TYPE); \
       insn = middle_endian_convert (insn, IS_ME (#FORMULA, abfd)); \
       insn = (* get_replace_function (abfd, TYPE)) (insn, relocation); \
@@ -997,6 +993,9 @@ arc_do_relocation (bfd_byte * contents,
 				 bfd_arch_bits_per_address (abfd),
 				 relocation);
 
+      if(flag == bfd_reloc_ok)
+	flag = arc_extra_overflow_checks (relocation, reloc_data, info);
+
 #undef  DEBUG_ARC_RELOC
 #define DEBUG_ARC_RELOC(A) debug_arc_reloc (A)
       if (flag != bfd_reloc_ok)

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

* RE: [PATCH 1/3] opcodes/arc: Compute insn lengths in disassembler
  2016-04-07 11:05   ` [PATCH 1/3] opcodes/arc: Compute insn lengths in disassembler Andrew Burgess
  2016-04-07 11:27     ` Claudiu Zissulescu
@ 2016-04-12  9:27     ` Claudiu Zissulescu
  1 sibling, 0 replies; 20+ messages in thread
From: Claudiu Zissulescu @ 2016-04-12  9:27 UTC (permalink / raw)
  To: Andrew Burgess, binutils; +Cc: Cupertino.Miranda, noamca

Hi Andrew,
 
> The table is self-checking while being built, it only contains the
> instructions for the current architecture being disassembled, and if
> different instructions with the same major opcode have different
> lengths, this will cause an error while the table is being filled.

I still have some questions regarding your newly proposed way of decoding the instructions. What bothers me is the fact you relay on the existing encodings, but what is happening when we have extension instructions. For ARCompact the extension instructions may  start from major 0x05 and end somewhere at 0x0B, For ARCv2 the extension instructions start from 0x05 and ends at 0x07. The official docs are stipulating those ones are 32-bit. Though, I've seen custom instructions defined as short instructions(16-bit). How your proposed patch is addressing the extension instructions (let us not take into account the short ones)?

As far as I can see, we can actually say that all major larger than 0x0B for ARCompact are 16-bit, and all major larger than 0x07 for ARCv2 are 16-bit. Though some sanity testing needs to be done.

Name conventions:
MAJOR		Notes
0x04 		ARC 32-bit basecase instructions;
0x05 - 0x06 	ARC 32-bit extension instructions;
0x07		User 32-bit extension instructions;
0x08		ARCompact: User 32-bit extension instructions; ARCv2: 16-bit instructions
0x09		ARCompact: Market-specific extension instructions; ARCv2: 16-bit instructions
0x0A		ARCompact: Market-specific extension instructions; ARCv2: 16-bit instructions
0x0B		ARCompact: Market-specific extension instructions; ARCv2: 16-bit instructions

Thanks,
Claudiu

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

* [PATCHv2 3/3] arc/nps400 : New cmem instructions and associated relocation
  2016-04-07 11:05 ` [PATCH " Andrew Burgess
                     ` (4 preceding siblings ...)
  2016-04-12 11:00   ` [PATCHv2 2/3] bfd/arc: Rename enum entries to avoid conflicts Andrew Burgess
@ 2016-04-12 11:00   ` Andrew Burgess
  2016-04-13 13:37     ` Nick Clifton
  5 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2016-04-12 11:00 UTC (permalink / raw)
  To: binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca, Andrew Burgess

Add support for arc/nps400 cmem instructions, these load and store
instructions are hard-wired to access "0x57f00000 + 16-bit-offset".

Supporting this relocation required some additions to the arc relocation
handling in the bfd library, as well as the standard changes required to
add a new relocation type.

There's a test of the new instructions in the assembler, and a test of
the relocation in the linker.

bfd/ChangeLog:

	* reloc.c: Add BFD_RELOC_ARC_NPS_CMEM16 entry.
	* bfd-in2.h: Regenerate.
	* libbfd.h: Regenerate.
	* elf32-arc.c: Add 'opcode/arc.h' include.
	(struct arc_relocation_data): Add symbol_name.
	(arc_special_overflow_checks): New function.
	(arc_do_relocation): Use arc_special_overflow_checks, reindent as
	required, add an extra comment.
	(elf_arc_relocate_section): Setup symbol_name in reloc_data.

gas/ChangeLog:

	* testsuite/gas/arc/nps400-3.d: New file.
	* testsuite/gas/arc/nps400-3.s: New file.

include/ChangeLog:

	* elf/arc-reloc.def: Add ARC_NPS_CMEM16 reloc.
	* opcode/arc.h (NPS_CMEM_HIGH_VALUE): Define.

ld/ChangeLog:

	* testsuite/ld-arc/arc.exp: New file.
	* testsuite/ld-arc/nps-1.s: New file.
	* testsuite/ld-arc/nps-1a.d: New file.
	* testsuite/ld-arc/nps-1b.d: New file.
	* testsuite/ld-arc/nps-1b.err: New file.

opcodes/ChangeLog:

	* arc-nps400-tbl.h: Add xldb, xldw, xld, xstb, xstw, and xst
	instructions.
	* arc-opc.c (insert_nps_cmem_uimm16): New function.
	(extract_nps_cmem_uimm16): New function.
	(arc_operands): Add NPS_XLDST_UIMM16 operand.
---
 bfd/ChangeLog                    | 12 ++++++
 bfd/bfd-in2.h                    |  1 +
 bfd/elf32-arc.c                  | 93 ++++++++++++++++++++++++++++++++--------
 bfd/libbfd.h                     |  1 +
 bfd/reloc.c                      |  2 +
 gas/ChangeLog                    |  5 +++
 gas/testsuite/gas/arc/nps400-3.d | 56 ++++++++++++++++++++++++
 gas/testsuite/gas/arc/nps400-3.s | 23 ++++++++++
 include/ChangeLog                |  5 +++
 include/elf/arc-reloc.def        |  7 +++
 include/opcode/arc.h             |  3 ++
 ld/ChangeLog                     |  8 ++++
 ld/testsuite/ld-arc/arc.exp      | 30 +++++++++++++
 ld/testsuite/ld-arc/nps-1.s      | 10 +++++
 ld/testsuite/ld-arc/nps-1a.d     | 16 +++++++
 ld/testsuite/ld-arc/nps-1b.d     |  4 ++
 ld/testsuite/ld-arc/nps-1b.err   |  1 +
 opcodes/ChangeLog                |  8 ++++
 opcodes/arc-nps400-tbl.h         | 12 ++++++
 opcodes/arc-opc.c                | 22 ++++++++++
 20 files changed, 300 insertions(+), 19 deletions(-)
 create mode 100644 gas/testsuite/gas/arc/nps400-3.d
 create mode 100644 gas/testsuite/gas/arc/nps400-3.s
 create mode 100644 ld/testsuite/ld-arc/arc.exp
 create mode 100644 ld/testsuite/ld-arc/nps-1.s
 create mode 100644 ld/testsuite/ld-arc/nps-1a.d
 create mode 100644 ld/testsuite/ld-arc/nps-1b.d
 create mode 100644 ld/testsuite/ld-arc/nps-1b.err

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index f02e2aa..6532f6e 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -3721,6 +3721,7 @@ pc-relative or some form of GOT-indirect relocation.  */
   BFD_RELOC_ARC_TLS_LE_32,
   BFD_RELOC_ARC_S25W_PCREL_PLT,
   BFD_RELOC_ARC_S21H_PCREL_PLT,
+  BFD_RELOC_ARC_NPS_CMEM16,
 
 /* ADI Blackfin 16 bit immediate absolute reloc.  */
   BFD_RELOC_BFIN_16_IMM,
diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c
index 263effc..ee72b9a 100644
--- a/bfd/elf32-arc.c
+++ b/bfd/elf32-arc.c
@@ -26,6 +26,7 @@
 #include "elf/arc.h"
 #include "libiberty.h"
 #include "opcode/arc-func.h"
+#include "opcode/arc.h"
 #include "arc-plt.h"
 
 #ifdef DEBUG
@@ -722,6 +723,8 @@ struct arc_relocation_data
   bfd_signed_vma  got_symbol_vma;
 
   bfd_boolean	  should_relocate;
+
+  const char *    symbol_name;
 };
 
 static void
@@ -788,6 +791,52 @@ middle_endian_convert (bfd_vma insn, bfd_boolean do_it)
   return insn;
 }
 
+/* This function is called for relocations that are otherwise marked as NOT
+   requiring overflow checks.  In here we perform non-standard checks of
+   the relocation value.  */
+
+static inline bfd_reloc_status_type
+arc_special_overflow_checks (const struct arc_relocation_data reloc_data,
+                             bfd_signed_vma relocation,
+			     struct bfd_link_info *info ATTRIBUTE_UNUSED)
+{
+  switch (reloc_data.howto->type)
+    {
+    case R_ARC_NPS_CMEM16:
+      if (((relocation >> 16) & 0xffff) != NPS_CMEM_HIGH_VALUE)
+        {
+          if (reloc_data.reloc_addend == 0)
+            (*_bfd_error_handler)
+              (_("%B(%A+0x%lx): CMEM relocation to `%s' is invalid, "
+                 "16 MSB should be 0x%04x (value is 0x%lx)"),
+               reloc_data.input_section->owner,
+               reloc_data.input_section,
+               reloc_data.reloc_offset,
+               reloc_data.symbol_name,
+               NPS_CMEM_HIGH_VALUE,
+               (relocation));
+          else
+            (*_bfd_error_handler)
+              (_("%B(%A+0x%lx): CMEM relocation to `%s+0x%lx' is invalid, "
+                 "16 MSB should be 0x%04x (value is 0x%lx)"),
+               reloc_data.input_section->owner,
+               reloc_data.input_section,
+               reloc_data.reloc_offset,
+               reloc_data.symbol_name,
+               reloc_data.reloc_addend,
+               NPS_CMEM_HIGH_VALUE,
+               (relocation));
+          return bfd_reloc_overflow;
+        }
+      break;
+
+    default:
+      break;
+    }
+
+  return bfd_reloc_ok;
+}
+
 #define ME(reloc) (reloc)
 
 #define IS_ME(FORMULA,BFD) ((strstr (FORMULA, "ME") != NULL) \
@@ -896,6 +945,7 @@ arc_do_relocation (bfd_byte * contents,
   bfd_vma orig_insn ATTRIBUTE_UNUSED;
   bfd * abfd = reloc_data.input_section->owner;
   struct elf_link_hash_table *htab ATTRIBUTE_UNUSED = elf_hash_table (info);
+  bfd_reloc_status_type flag;
 
   if (reloc_data.should_relocate == FALSE)
     return bfd_reloc_ok;
@@ -936,34 +986,34 @@ arc_do_relocation (bfd_byte * contents,
 
   /* Check for relocation overflow.  */
   if (reloc_data.howto->complain_on_overflow != complain_overflow_dont)
-    {
-      bfd_reloc_status_type flag;
-      flag = bfd_check_overflow (reloc_data.howto->complain_on_overflow,
-				 reloc_data.howto->bitsize,
-				 reloc_data.howto->rightshift,
-				 bfd_arch_bits_per_address (abfd),
-				 relocation);
+    flag = bfd_check_overflow (reloc_data.howto->complain_on_overflow,
+                               reloc_data.howto->bitsize,
+                               reloc_data.howto->rightshift,
+                               bfd_arch_bits_per_address (abfd),
+                               relocation);
+  else
+    flag = arc_special_overflow_checks (reloc_data, relocation, info);
 
 #undef  DEBUG_ARC_RELOC
 #define DEBUG_ARC_RELOC(A) debug_arc_reloc (A)
-      if (flag != bfd_reloc_ok)
-	{
-	  PR_DEBUG ( "Relocation overflows !!!!\n");
+  if (flag != bfd_reloc_ok)
+    {
+      PR_DEBUG ( "Relocation overflows !!!!\n");
 
-	  DEBUG_ARC_RELOC (reloc_data);
+      DEBUG_ARC_RELOC (reloc_data);
 
-	  PR_DEBUG (
-		  "Relocation value = signed -> %d, unsigned -> %u"
-		  ", hex -> (0x%08x)\n",
-		  (int) relocation,
-		  (unsigned int) relocation,
-		  (unsigned int) relocation);
-	  return flag;
-	}
+      PR_DEBUG (
+                "Relocation value = signed -> %d, unsigned -> %u"
+                ", hex -> (0x%08x)\n",
+                (int) relocation,
+                (unsigned int) relocation,
+                (unsigned int) relocation);
+      return flag;
     }
 #undef  DEBUG_ARC_RELOC
 #define DEBUG_ARC_RELOC(A)
 
+  /* Write updated instruction back to memory.  */
   switch (reloc_data.howto->size)
     {
       case 2:
@@ -1168,6 +1218,10 @@ elf_arc_relocate_section (bfd *		   output_bfd,
 
 	  reloc_data.sym_value = sym->st_value;
 	  reloc_data.sym_section = sec;
+	  reloc_data.symbol_name =
+            bfd_elf_string_from_elf_section (input_bfd,
+                                             symtab_hdr->sh_link,
+                                             sym->st_name);
 
 	  /* Mergeable section handling.  */
 	  if ((sec->flags & SEC_MERGE)
@@ -1284,6 +1338,7 @@ elf_arc_relocate_section (bfd *		   output_bfd,
 	    h = (struct elf_link_hash_entry *) h->root.u.i.link;
 
 	  BFD_ASSERT ((h->dynindx == -1) >= (h->forced_local != 0));
+	  reloc_data.symbol_name = h->root.root.string;
 	  /* If we have encountered a definition for this symbol.  */
 	  if (h->root.type == bfd_link_hash_defined
 	      || h->root.type == bfd_link_hash_defweak)
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index d7183d3..16c0aee 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -1738,6 +1738,7 @@ static const char *const bfd_reloc_code_real_names[] = { "@@uninitialized@@",
   "BFD_RELOC_ARC_TLS_LE_32",
   "BFD_RELOC_ARC_S25W_PCREL_PLT",
   "BFD_RELOC_ARC_S21H_PCREL_PLT",
+  "BFD_RELOC_ARC_NPS_CMEM16",
   "BFD_RELOC_BFIN_16_IMM",
   "BFD_RELOC_BFIN_16_HIGH",
   "BFD_RELOC_BFIN_4_PCREL",
diff --git a/bfd/reloc.c b/bfd/reloc.c
index 0135c04..c3b713b 100644
--- a/bfd/reloc.c
+++ b/bfd/reloc.c
@@ -3668,6 +3668,8 @@ ENUMX
   BFD_RELOC_ARC_S25W_PCREL_PLT
 ENUMX
   BFD_RELOC_ARC_S21H_PCREL_PLT
+ENUMX
+  BFD_RELOC_ARC_NPS_CMEM16
 ENUMDOC
   ARC relocs.
 
diff --git a/gas/testsuite/gas/arc/nps400-3.d b/gas/testsuite/gas/arc/nps400-3.d
new file mode 100644
index 0000000..ea52554
--- /dev/null
+++ b/gas/testsuite/gas/arc/nps400-3.d
@@ -0,0 +1,56 @@
+#as: -mcpu=nps400
+#objdump: -dr
+
+.*: +file format .*arc.*
+
+Disassembly of section .text:
+
+[0-9a-f]+ <.*>:
+   0:	5988 0000           	xldb	r12,\[0x57f00000\]
+   4:	5ae8 ffff           	xldb	r23,\[0x57f0ffff\]
+   8:	5868 0000           	xldb	r3,\[0x57f00000\]
+   c:	5968 ffff           	xldb	r11,\[0x57f0ffff\]
+  10:	5a88 0000           	xldb	r20,\[0x57f00000\]
+			10: R_ARC_NPS_CMEM16	foo
+  14:	5828 0000           	xldb	r1,\[0x57f00000\]
+			14: R_ARC_NPS_CMEM16	foo\+0x20
+  18:	5989 0000           	xldw	r12,\[0x57f00000\]
+  1c:	5ae9 ffff           	xldw	r23,\[0x57f0ffff\]
+  20:	5869 0000           	xldw	r3,\[0x57f00000\]
+  24:	5969 ffff           	xldw	r11,\[0x57f0ffff\]
+  28:	5a89 0000           	xldw	r20,\[0x57f00000\]
+			28: R_ARC_NPS_CMEM16	foo
+  2c:	5829 0000           	xldw	r1,\[0x57f00000\]
+			2c: R_ARC_NPS_CMEM16	foo\+0x20
+  30:	598a 0000           	xld	r12,\[0x57f00000\]
+  34:	5aea ffff           	xld	r23,\[0x57f0ffff\]
+  38:	586a 0000           	xld	r3,\[0x57f00000\]
+  3c:	596a ffff           	xld	r11,\[0x57f0ffff\]
+  40:	5a8a 0000           	xld	r20,\[0x57f00000\]
+			40: R_ARC_NPS_CMEM16	foo
+  44:	582a 0000           	xld	r1,\[0x57f00000\]
+			44: R_ARC_NPS_CMEM16	foo\+0x20
+  48:	598c 0000           	xstb	r12,\[0x57f00000\]
+  4c:	5aec ffff           	xstb	r23,\[0x57f0ffff\]
+  50:	586c 0000           	xstb	r3,\[0x57f00000\]
+  54:	596c ffff           	xstb	r11,\[0x57f0ffff\]
+  58:	5a8c 0000           	xstb	r20,\[0x57f00000\]
+			58: R_ARC_NPS_CMEM16	foo
+  5c:	582c 0000           	xstb	r1,\[0x57f00000\]
+			5c: R_ARC_NPS_CMEM16	foo\+0x20
+  60:	598d 0000           	xstw	r12,\[0x57f00000\]
+  64:	5aed ffff           	xstw	r23,\[0x57f0ffff\]
+  68:	586d 0000           	xstw	r3,\[0x57f00000\]
+  6c:	596d ffff           	xstw	r11,\[0x57f0ffff\]
+  70:	5a8d 0000           	xstw	r20,\[0x57f00000\]
+			70: R_ARC_NPS_CMEM16	foo
+  74:	582d 0000           	xstw	r1,\[0x57f00000\]
+			74: R_ARC_NPS_CMEM16	foo\+0x20
+  78:	598e 0000           	xst	r12,\[0x57f00000\]
+  7c:	5aee ffff           	xst	r23,\[0x57f0ffff\]
+  80:	586e 0000           	xst	r3,\[0x57f00000\]
+  84:	596e ffff           	xst	r11,\[0x57f0ffff\]
+  88:	5a8e 0000           	xst	r20,\[0x57f00000\]
+			88: R_ARC_NPS_CMEM16	foo
+  8c:	582e 0000           	xst	r1,\[0x57f00000\]
+			8c: R_ARC_NPS_CMEM16	foo\+0x20
diff --git a/gas/testsuite/gas/arc/nps400-3.s b/gas/testsuite/gas/arc/nps400-3.s
new file mode 100644
index 0000000..6840223
--- /dev/null
+++ b/gas/testsuite/gas/arc/nps400-3.s
@@ -0,0 +1,23 @@
+        .macro  xldst_test mnem
+        \mnem r12, [ 0x0 ]
+        \mnem r23, [ 0xffff ]
+        \mnem r3, [ 0x57f00000 ]
+        \mnem r11, [ 0x57f0ffff ]
+        \mnem r20, [ foo ]
+        \mnem r1, [ foo + 0x20 ]
+        .endm
+
+        .text
+        ;; xldb
+        xldst_test xldb
+        ;; xldw
+        xldst_test xldw
+        ;; xld
+        xldst_test xld
+        ;; xstb
+        xldst_test xstb
+        ;; xstw
+        xldst_test xstw
+        ;; xst
+        xldst_test xst
+
diff --git a/include/elf/arc-reloc.def b/include/elf/arc-reloc.def
index 36a3516..10703d2 100644
--- a/include/elf/arc-reloc.def
+++ b/include/elf/arc-reloc.def
@@ -490,3 +490,10 @@ ARC_RELOC_HOWTO(ARC_S21H_PCREL_PLT, 77, \
                 replace_disp21h, \
                 signed, \
                 ( ME ( ( ( ( L + A ) - P ) >> 1 ) ) ))
+
+ARC_RELOC_HOWTO(ARC_NPS_CMEM16, 78, \
+                2, \
+                16, \
+                replace_bits16, \
+                dont, \
+                ( S + A ))
diff --git a/include/opcode/arc.h b/include/opcode/arc.h
index 7cd78e4..2811877 100644
--- a/include/opcode/arc.h
+++ b/include/opcode/arc.h
@@ -437,6 +437,9 @@ extern const unsigned arc_num_aux_regs;
 extern const struct arc_opcode arc_relax_opcodes[];
 extern const unsigned arc_num_relax_opcodes;
 
+/* Macro used for generating one class of NPS instructions.  */
+#define NPS_CMEM_HIGH_VALUE 0x57f0
+
 /* Macros to help generating regular pattern instructions.  */
 #define FIELDA(word) (word & 0x3F)
 #define FIELDB(word) (((word & 0x07) << 24) | (((word >> 3) & 0x07) << 12))
diff --git a/ld/testsuite/ld-arc/arc.exp b/ld/testsuite/ld-arc/arc.exp
new file mode 100644
index 0000000..0cf6228
--- /dev/null
+++ b/ld/testsuite/ld-arc/arc.exp
@@ -0,0 +1,30 @@
+# Copyright (C) 2016 Free Software Foundation, Inc.
+#
+# This file is part of the GNU Binutils.
+#
+# 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+#
+
+if { ![istarget arc*-*-*] } {
+    return
+}
+
+set arc_test_list [lsort [glob -nocomplain $srcdir/$subdir/*.d]]
+foreach arc_test $arc_test_list {
+    verbose [file rootname $arc_test]
+    run_dump_test [file rootname $arc_test]
+}
+
diff --git a/ld/testsuite/ld-arc/nps-1.s b/ld/testsuite/ld-arc/nps-1.s
new file mode 100644
index 0000000..295fa2c
--- /dev/null
+++ b/ld/testsuite/ld-arc/nps-1.s
@@ -0,0 +1,10 @@
+        .text
+        .global __start
+__start:
+        xldb r10, [ foo ]
+        xldw r10, [ foo ]
+        xld r10, [ foo ]
+        xstb r10, [ foo ]
+        xstw r10, [ foo ]
+        xst r10, [ foo ]
+
diff --git a/ld/testsuite/ld-arc/nps-1a.d b/ld/testsuite/ld-arc/nps-1a.d
new file mode 100644
index 0000000..120c71c
--- /dev/null
+++ b/ld/testsuite/ld-arc/nps-1a.d
@@ -0,0 +1,16 @@
+#source: nps-1.s
+#as: -mcpu=nps400
+#ld: -defsym=foo=0x57f03000
+#objdump: -d
+
+.*: +file format .*arc.*
+
+Disassembly of section .text:
+
+[0-9a-f]+ <.*>:
+ *[0-9a-f]+:	5948 3000           	xldb	r10,\[0x57f03000\]
+ *[0-9a-f]+:	5949 3000           	xldw	r10,\[0x57f03000\]
+ *[0-9a-f]+:	594a 3000           	xld	r10,\[0x57f03000\]
+ *[0-9a-f]+:	594c 3000           	xstb	r10,\[0x57f03000\]
+ *[0-9a-f]+:	594d 3000           	xstw	r10,\[0x57f03000\]
+ *[0-9a-f]+:	594e 3000           	xst	r10,\[0x57f03000\]
diff --git a/ld/testsuite/ld-arc/nps-1b.d b/ld/testsuite/ld-arc/nps-1b.d
new file mode 100644
index 0000000..56c29ae
--- /dev/null
+++ b/ld/testsuite/ld-arc/nps-1b.d
@@ -0,0 +1,4 @@
+#source: nps-1.s
+#as: -mcpu=nps400
+#ld: -defsym=foo=0x56f03000
+#error_output: nps-1b.err
diff --git a/ld/testsuite/ld-arc/nps-1b.err b/ld/testsuite/ld-arc/nps-1b.err
new file mode 100644
index 0000000..a44b3c1
--- /dev/null
+++ b/ld/testsuite/ld-arc/nps-1b.err
@@ -0,0 +1 @@
+.*\.o\(\.text\+0x0\): CMEM relocation to `foo' is invalid, 16 MSB should be 0x57f0 \(value is 0x56f03000\)
diff --git a/opcodes/arc-nps400-tbl.h b/opcodes/arc-nps400-tbl.h
index 58d479f..832d2ff 100644
--- a/opcodes/arc-nps400-tbl.h
+++ b/opcodes/arc-nps400-tbl.h
@@ -140,3 +140,15 @@
 
 /* hwscd.restore 0,C */
 { "hwschd", 0x3e6f7003, 0xfffff03f, ARC_OPCODE_NPS400, CONTROL, NONE, { ZA, RC }, { C_NPS_HWS_RESTORE }},
+
+/****      Load / Store From (0x57f00000 + Offset) Instructions       ****/
+
+#define XLDST_LIKE(NAME,SUBOP2)                                          \
+  { NAME, (0x58000000 | (SUBOP2 << 16)), 0xf81f0000, ARC_OPCODE_NPS400, MEMORY, NONE, { NPS_R_DST, BRAKET, NPS_XLDST_UIMM16, BRAKETdup }, { 0 }},
+
+XLDST_LIKE("xldb", 0x8)
+XLDST_LIKE("xldw", 0x9)
+XLDST_LIKE("xld", 0xa)
+XLDST_LIKE("xstb", 0xc)
+XLDST_LIKE("xstw", 0xd)
+XLDST_LIKE("xst", 0xe)
diff --git a/opcodes/arc-opc.c b/opcodes/arc-opc.c
index 5603ded..2ce8853 100644
--- a/opcodes/arc-opc.c
+++ b/opcodes/arc-opc.c
@@ -838,6 +838,25 @@ extract_nps_dst_pos_and_size (unsigned insn ATTRIBUTE_UNUSED,
   return (insn & 0x1f);
 }
 
+static unsigned
+insert_nps_cmem_uimm16 (unsigned insn ATTRIBUTE_UNUSED,
+                        int value ATTRIBUTE_UNUSED,
+                        const char **errmsg ATTRIBUTE_UNUSED)
+{
+  int top = (value >> 16) & 0xffff;
+  if (top != 0x0 && top != NPS_CMEM_HIGH_VALUE)
+    *errmsg = _("invalid value for CMEM ld/st immediate");
+  insn |= (value & 0xffff);
+  return insn;
+}
+
+static int
+extract_nps_cmem_uimm16 (unsigned insn ATTRIBUTE_UNUSED,
+                         bfd_boolean * invalid ATTRIBUTE_UNUSED)
+{
+  return (NPS_CMEM_HIGH_VALUE << 16) | (insn & 0xffff);
+}
+
 /* Include the generic extract/insert functions.  Order is important
    as some of the functions present in the .h may be disabled via
    defines.  */
@@ -1498,6 +1517,9 @@ const struct arc_operand arc_operands[] =
 
 #define NPS_RFLT_UIMM6		(NPS_UIMM16 + 1)
   { 6, 6, 0, ARC_OPERAND_UNSIGNED | ARC_OPERAND_NCHK, insert_nps_rflt_uimm6, extract_nps_rflt_uimm6 },
+
+#define NPS_XLDST_UIMM16	(NPS_RFLT_UIMM6 + 1)
+  { 16, 0, BFD_RELOC_ARC_NPS_CMEM16, ARC_OPERAND_UNSIGNED | ARC_OPERAND_NCHK, insert_nps_cmem_uimm16, extract_nps_cmem_uimm16 },
 };
 
 const unsigned arc_num_operands = ARRAY_SIZE (arc_operands);
-- 
2.5.1

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

* [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
  2016-04-07 11:05 ` [PATCH " Andrew Burgess
                     ` (2 preceding siblings ...)
  2016-04-07 11:05   ` [PATCH 3/3] arc/nps400 : New cmem instructions and associated relocation Andrew Burgess
@ 2016-04-12 11:00   ` Andrew Burgess
  2016-04-12 12:27     ` Claudiu Zissulescu
  2016-04-13 13:32     ` Nick Clifton
  2016-04-12 11:00   ` [PATCHv2 2/3] bfd/arc: Rename enum entries to avoid conflicts Andrew Burgess
  2016-04-12 11:00   ` [PATCHv2 3/3] arc/nps400 : New cmem instructions and associated relocation Andrew Burgess
  5 siblings, 2 replies; 20+ messages in thread
From: Andrew Burgess @ 2016-04-12 11:00 UTC (permalink / raw)
  To: binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca, Andrew Burgess

Move the logic that calculates the instruction length out to a new
function.  Restructure the code to make it simpler.

opcodes/ChangeLog:

	* arc-dis.c (arc_insn_length): New function.
	(print_insn_arc): Use arc_insn_length.
---
 opcodes/ChangeLog |  5 +++++
 opcodes/arc-dis.c | 57 ++++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
index eb8bd67..458ba47 100644
--- a/opcodes/arc-dis.c
+++ b/opcodes/arc-dis.c
@@ -309,6 +309,41 @@ get_auxreg (const struct arc_opcode *opcode,
     }
   return NULL;
 }
+
+/* Calculate the instruction length for an instruction starting with MSB
+   and LSB, the most and least significant byte.  The ISA_MASK is used to
+   filter the instructions considered to only those that are part of the
+   current architecture.
+
+   The instruction lengths are calculated from the ARC_OPCODE table, and
+   cached for later use.  */
+
+static int
+arc_insn_length (bfd_byte msb, bfd_byte lsb ATTRIBUTE_UNUSED,
+                 struct disassemble_info *info)
+{
+  bfd_byte major_opcode = msb >> 3;
+  int len = 0;
+
+  switch (info->mach)
+    {
+    case bfd_mach_arc_nps400:
+    case bfd_mach_arc_arc700:
+    case bfd_mach_arc_arc600:
+      len = (major_opcode > 0xb) ? 2 : 4;
+      break;
+
+    case bfd_mach_arc_arcv2:
+      len = (major_opcode > 0x7) ? 2 : 4;
+      break;
+
+    default:
+      abort ();
+    }
+
+  return len;
+}
+
 /* Disassemble ARC instructions.  */
 
 static int
@@ -422,20 +457,19 @@ print_insn_arc (bfd_vma memaddr,
       return size;
     }
 
-  if ((((buffer[lowbyte] & 0xf8) > 0x38)
-       && ((buffer[lowbyte] & 0xf8) != 0x48))
-      || ((info->mach == bfd_mach_arc_arcv2)
-	  && ((buffer[lowbyte] & 0xF8) == 0x48)) /* FIXME! ugly.  */
-      )
+  insnLen = arc_insn_length (buffer[lowbyte], buffer[highbyte], info);
+  switch (insnLen)
     {
-      /* This is a short instruction.  */
-      insnLen = 2;
+    case 2:
       insn[0] = (buffer[lowbyte] << 8) | buffer[highbyte];
-    }
-  else
-    {
-      insnLen = 4;
+      break;
 
+    default:
+      /* An unknown instruction is treated as being length 4.  This is
+         possibly not the best solution, but matches the behaviour that was
+         in place before the table based instruction length look-up was
+         introduced.  */
+    case 4:
       /* This is a long instruction: Read the remaning 2 bytes.  */
       status = (*info->read_memory_func) (memaddr + 2, &buffer[2], 2, info);
       if (status != 0)
@@ -444,6 +478,7 @@ print_insn_arc (bfd_vma memaddr,
 	  return -1;
 	}
       insn[0] = ARRANGE_ENDIAN (info, buffer);
+      break;
     }
 
   /* Set some defaults for the insn info.  */
-- 
2.5.1

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

* [PATCHv2 0/3] ARC: Add another group of nps instructions
  2016-04-07 11:05 ` [PATCH " Andrew Burgess
@ 2016-04-12 11:00 Andrew Burgess
  2016-04-07 11:05 ` [PATCH " Andrew Burgess
  5 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2016-04-12 11:00 UTC (permalink / raw)
  To: binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca, Andrew Burgess

This revision incorporates the feedback from Synopsys.  The changes are:

 Patch #1: Complete rewrite, much simpler now based on Claudiu's
 observations.

 Patch #2: Unchanged.

 Patch #3: Incorporates Cupertino's feedback about prefered code flow.
 Some small adjustments were required to Cupertino's suggestion,
 however, I believe what I have now is inline with his intentions.

---

Andrew Burgess (3):
  opcodes/arc: Move instruction length logic to new function
  bfd/arc: Rename enum entries to avoid conflicts
  arc/nps400 : New cmem instructions and associated relocation

 bfd/ChangeLog                    |  20 ++++++
 bfd/bfd-in2.h                    |   1 +
 bfd/elf32-arc.c                  | 127 ++++++++++++++++++++++++++++-----------
 bfd/libbfd.h                     |   1 +
 bfd/reloc.c                      |   2 +
 gas/ChangeLog                    |   5 ++
 gas/testsuite/gas/arc/nps400-3.d |  56 +++++++++++++++++
 gas/testsuite/gas/arc/nps400-3.s |  23 +++++++
 include/ChangeLog                |   5 ++
 include/elf/arc-reloc.def        |   7 +++
 include/opcode/arc.h             |   3 +
 ld/ChangeLog                     |   8 +++
 ld/testsuite/ld-arc/arc.exp      |  30 +++++++++
 ld/testsuite/ld-arc/nps-1.s      |  10 +++
 ld/testsuite/ld-arc/nps-1a.d     |  16 +++++
 ld/testsuite/ld-arc/nps-1b.d     |   4 ++
 ld/testsuite/ld-arc/nps-1b.err   |   1 +
 opcodes/ChangeLog                |  13 ++++
 opcodes/arc-dis.c                |  57 ++++++++++++++----
 opcodes/arc-nps400-tbl.h         |  12 ++++
 opcodes/arc-opc.c                |  22 +++++++
 21 files changed, 376 insertions(+), 47 deletions(-)
 create mode 100644 gas/testsuite/gas/arc/nps400-3.d
 create mode 100644 gas/testsuite/gas/arc/nps400-3.s
 create mode 100644 ld/testsuite/ld-arc/arc.exp
 create mode 100644 ld/testsuite/ld-arc/nps-1.s
 create mode 100644 ld/testsuite/ld-arc/nps-1a.d
 create mode 100644 ld/testsuite/ld-arc/nps-1b.d
 create mode 100644 ld/testsuite/ld-arc/nps-1b.err

-- 
2.5.1

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

* [PATCHv2 2/3] bfd/arc: Rename enum entries to avoid conflicts
  2016-04-07 11:05 ` [PATCH " Andrew Burgess
                     ` (3 preceding siblings ...)
  2016-04-12 11:00   ` [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function Andrew Burgess
@ 2016-04-12 11:00   ` Andrew Burgess
  2016-04-13 13:34     ` Nick Clifton
  2016-04-12 11:00   ` [PATCHv2 3/3] arc/nps400 : New cmem instructions and associated relocation Andrew Burgess
  5 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2016-04-12 11:00 UTC (permalink / raw)
  To: binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca, Andrew Burgess

In bfd/elf32-arc.c an enum is created that contains entries with generic
names like 'NONE' and 'OFF'.  This has been fine for now, but I had a
need to include opcode/arc.h into bfd/elf32-arc.c.  Unfortunately
opcode/arc.h includes a different enum with identical generic names.

Given that changing the enum in the header file could mean wide-ranging
changes, while changing the enum in the .c file is limited to only
changing the one file, I've added a prefix to the enum in the .c file.

This commit does not add the new include, that will come later.  There
should be no functional change with this commit.

bfd/ChangeLog:

	* elf32-arc.c (tls_got_entries): Add 'TLS_GOT_' prefix to all
	entries.
	(elf_arc_relocate_section): Update enum uses.
	(elf_arc_check_relocs): Likewise.
	(elf_arc_finish_dynamic_symbol): Likewise.
---
 bfd/ChangeLog   |  8 ++++++++
 bfd/elf32-arc.c | 34 +++++++++++++++++-----------------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c
index 323f65d..263effc 100644
--- a/bfd/elf32-arc.c
+++ b/bfd/elf32-arc.c
@@ -127,10 +127,10 @@ enum tls_type_e
 
 enum tls_got_entries
 {
-  NONE = 0,
-  MOD,
-  OFF,
-  MOD_AND_OFF
+  TLS_GOT_NONE = 0,
+  TLS_GOT_MOD,
+  TLS_GOT_OFF,
+  TLS_GOT_MOD_AND_OFF
 };
 
 struct got_entry
@@ -1401,7 +1401,7 @@ elf_arc_relocate_section (bfd *		   output_bfd,
 			  bfd_put_32 (output_bfd,
 				      sym_value - sec_vma,
 				      htab->sgot->contents + entry->offset
-				      + (entry->existing_entries == MOD_AND_OFF ? 4 : 0));
+				      + (entry->existing_entries ==TLS_GOT_MOD_AND_OFF ? 4 : 0));
 
 			  ARC_DEBUG ("arc_info: FIXED -> %s value = 0x%x "
 				     "@ 0x%x, for symbol %s\n",
@@ -1409,7 +1409,7 @@ elf_arc_relocate_section (bfd *		   output_bfd,
 				      "GOT_TLS_IE"),
 				     sym_value - sec_vma,
 				     htab->sgot->contents + entry->offset
-				     + (entry->existing_entries == MOD_AND_OFF ? 4 : 0),
+				     + (entry->existing_entries == TLS_GOT_MOD_AND_OFF ? 4 : 0),
 				     h->root.root.string);
 
 			  entry->processed = TRUE;
@@ -1806,7 +1806,7 @@ elf_arc_check_relocs (bfd *		         abfd,
 						  bfd_link_pic (info),
 						  NULL);
 		  new_got_entry_to_list (&(local_got_ents[r_symndx]),
-					 GOT_NORMAL, offset, NONE);
+					 GOT_NORMAL, offset, TLS_GOT_NONE);
 		}
 	    }
 	  else
@@ -1818,7 +1818,7 @@ elf_arc_check_relocs (bfd *		         abfd,
 		  bfd_vma offset =
 		    ADD_SYMBOL_REF_SEC_AND_RELOC (got, TRUE, h);
 		  new_got_entry_to_list (&h->got.glist,
-					 GOT_NORMAL, offset, NONE);
+					 GOT_NORMAL, offset, TLS_GOT_NONE);
 		}
 	    }
 	}
@@ -1847,7 +1847,7 @@ elf_arc_check_relocs (bfd *		         abfd,
 
 	  if (type != GOT_UNKNOWN && !symbol_has_entry_of_type (*list, type))
 	    {
-	      enum tls_got_entries entries = NONE;
+	      enum tls_got_entries entries = TLS_GOT_NONE;
 	      bfd_vma offset =
 		ADD_SYMBOL_REF_SEC_AND_RELOC (got, TRUE, h);
 
@@ -1855,11 +1855,11 @@ elf_arc_check_relocs (bfd *		         abfd,
 		{
 		  bfd_vma ATTRIBUTE_UNUSED notneeded =
 		    ADD_SYMBOL_REF_SEC_AND_RELOC (got, TRUE, h);
-		  entries = MOD_AND_OFF;
+		  entries = TLS_GOT_MOD_AND_OFF;
 		}
 
-	      if (entries == NONE)
-		entries = OFF;
+	      if (entries == TLS_GOT_NONE)
+		entries = TLS_GOT_OFF;
 
 	      new_got_entry_to_list (list, type, offset, entries);
 	    }
@@ -2256,16 +2256,16 @@ elf_arc_finish_dynamic_symbol (bfd * output_bfd,
 		}
 	      list->created_dyn_relocation = TRUE;
 	    }
-	  else if (list->existing_entries != NONE)
+	  else if (list->existing_entries != TLS_GOT_NONE)
 	    {
 	      struct elf_link_hash_table *htab = elf_hash_table (info);
 	      enum tls_got_entries e = list->existing_entries;
 
 	      BFD_ASSERT (list->type != GOT_TLS_GD
-			  || list->existing_entries == MOD_AND_OFF);
+			  || list->existing_entries == TLS_GOT_MOD_AND_OFF);
 
 	      bfd_vma dynindx = h->dynindx == -1 ? 0 : h->dynindx;
-	      if (e == MOD_AND_OFF || e == MOD)
+	      if (e == TLS_GOT_MOD_AND_OFF || e == TLS_GOT_MOD)
 		{
 		  ADD_RELA (output_bfd, got, got_offset, dynindx,
 			    R_ARC_TLS_DTPMOD, 0);
@@ -2277,7 +2277,7 @@ GOT_OFFSET = 0x%x, GOT_VMA = 0x%x, INDEX = %d, ADDEND = 0x%x\n",
 			     + htab->sgot->output_offset + got_offset,
 			     dynindx, 0);
 		}
-	      if (e == MOD_AND_OFF || e == OFF)
+	      if (e == TLS_GOT_MOD_AND_OFF || e == TLS_GOT_OFF)
 		{
 		  bfd_vma addend = 0;
 		  if (list->type == GOT_TLS_IE)
@@ -2285,7 +2285,7 @@ GOT_OFFSET = 0x%x, GOT_VMA = 0x%x, INDEX = %d, ADDEND = 0x%x\n",
 					 htab->sgot->contents + got_offset);
 
 		  ADD_RELA (output_bfd, got,
-			    got_offset + (e == MOD_AND_OFF ? 4 : 0),
+			    got_offset + (e == TLS_GOT_MOD_AND_OFF ? 4 : 0),
 			    dynindx,
 			    (list->type == GOT_TLS_IE ?
 			     R_ARC_TLS_TPOFF : R_ARC_TLS_DTPOFF),
-- 
2.5.1

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

* RE: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
  2016-04-12 11:00   ` [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function Andrew Burgess
@ 2016-04-12 12:27     ` Claudiu Zissulescu
  2016-04-13 13:32     ` Nick Clifton
  1 sibling, 0 replies; 20+ messages in thread
From: Claudiu Zissulescu @ 2016-04-12 12:27 UTC (permalink / raw)
  To: Andrew Burgess, binutils; +Cc: Cupertino.Miranda, noamca

Hi Andrew,

> +    case bfd_mach_arc_nps400:
> +    case bfd_mach_arc_arc700:
> +    case bfd_mach_arc_arc600:
> +      len = (major_opcode > 0xb) ? 2 : 4;
> +      break;

Ok, this solves the hole between 0x07 and 0x0b existing in your former proposal. I think this implementation is safer than the older proposal and nicer than original code.

Cheers,
Claudiu

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

* Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
  2016-04-12 11:00   ` [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function Andrew Burgess
  2016-04-12 12:27     ` Claudiu Zissulescu
@ 2016-04-13 13:32     ` Nick Clifton
  2016-04-13 14:41       ` Andrew Burgess
  1 sibling, 1 reply; 20+ messages in thread
From: Nick Clifton @ 2016-04-13 13:32 UTC (permalink / raw)
  To: Andrew Burgess, binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca

Hi Andrew,

> opcodes/ChangeLog:
> 
> 	* arc-dis.c (arc_insn_length): New function.
> 	(print_insn_arc): Use arc_insn_length.

Approved - please apply, but ...

> +static int
> +arc_insn_length (bfd_byte msb, bfd_byte lsb ATTRIBUTE_UNUSED,
> +                 struct disassemble_info *info)

Would this function ever return a negative value ?  I assume not, so
it would make sense for its return type to be "unsigned int".

Also will the LSB parameter ever be used ?  I assume that it is there
for a future extension to this function which might need it, but if
that is only theoretical, then maybe the parameter can be dropped
entirely and the code simplified.

> +  switch (info->mach)
> +    {
> +    case bfd_mach_arc_nps400:
> +    case bfd_mach_arc_arc700:
> +    case bfd_mach_arc_arc600:
> +      len = (major_opcode > 0xb) ? 2 : 4;
> +      break;
> +
> +    case bfd_mach_arc_arcv2:
> +      len = (major_opcode > 0x7) ? 2 : 4;
> +      break;
> +
> +    default:
> +      abort ();
> +    }
> +
> +  return len;

Since len is always returned without any further processing you
could save a bit of space in the function by moving the return 
instruction into the switch statement.  eg:

  case bfd_mach_arc_arc600:
    return (major_opcode > 0xb) ? 2 : 4;

Cheers
  Nick

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

* Re: [PATCHv2 2/3] bfd/arc: Rename enum entries to avoid conflicts
  2016-04-12 11:00   ` [PATCHv2 2/3] bfd/arc: Rename enum entries to avoid conflicts Andrew Burgess
@ 2016-04-13 13:34     ` Nick Clifton
  0 siblings, 0 replies; 20+ messages in thread
From: Nick Clifton @ 2016-04-13 13:34 UTC (permalink / raw)
  To: Andrew Burgess, binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca

Hi Andrew,

> bfd/ChangeLog:
> 
> 	* elf32-arc.c (tls_got_entries): Add 'TLS_GOT_' prefix to all
> 	entries.
> 	(elf_arc_relocate_section): Update enum uses.
> 	(elf_arc_check_relocs): Likewise.
> 	(elf_arc_finish_dynamic_symbol): Likewise.

Approved - please apply.  But ...

> -				      + (entry->existing_entries == MOD_AND_OFF ? 4 : 0));
> +				      + (entry->existing_entries ==TLS_GOT_MOD_AND_OFF ? 4 : 0));

You need a space between the == and the TLS_GOT_MOD_AND_OFF.

Cheers
  Nick

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

* Re: [PATCHv2 3/3] arc/nps400 : New cmem instructions and associated relocation
  2016-04-12 11:00   ` [PATCHv2 3/3] arc/nps400 : New cmem instructions and associated relocation Andrew Burgess
@ 2016-04-13 13:37     ` Nick Clifton
  0 siblings, 0 replies; 20+ messages in thread
From: Nick Clifton @ 2016-04-13 13:37 UTC (permalink / raw)
  To: Andrew Burgess, binutils; +Cc: Claudiu.Zissulescu, Cupertino.Miranda, noamca

Hi Andrew,

> bfd/ChangeLog:
> 
> 	* reloc.c: Add BFD_RELOC_ARC_NPS_CMEM16 entry.
> 	* bfd-in2.h: Regenerate.
> 	* libbfd.h: Regenerate.
> 	* elf32-arc.c: Add 'opcode/arc.h' include.
> 	(struct arc_relocation_data): Add symbol_name.
> 	(arc_special_overflow_checks): New function.
> 	(arc_do_relocation): Use arc_special_overflow_checks, reindent as
> 	required, add an extra comment.
> 	(elf_arc_relocate_section): Setup symbol_name in reloc_data.
> 
> gas/ChangeLog:
> 
> 	* testsuite/gas/arc/nps400-3.d: New file.
> 	* testsuite/gas/arc/nps400-3.s: New file.
> 
> include/ChangeLog:
> 
> 	* elf/arc-reloc.def: Add ARC_NPS_CMEM16 reloc.
> 	* opcode/arc.h (NPS_CMEM_HIGH_VALUE): Define.
> 
> ld/ChangeLog:
> 
> 	* testsuite/ld-arc/arc.exp: New file.
> 	* testsuite/ld-arc/nps-1.s: New file.
> 	* testsuite/ld-arc/nps-1a.d: New file.
> 	* testsuite/ld-arc/nps-1b.d: New file.
> 	* testsuite/ld-arc/nps-1b.err: New file.
> 
> opcodes/ChangeLog:
> 
> 	* arc-nps400-tbl.h: Add xldb, xldw, xld, xstb, xstw, and xst
> 	instructions.
> 	* arc-opc.c (insert_nps_cmem_uimm16): New function.
> 	(extract_nps_cmem_uimm16): New function.
> 	(arc_operands): Add NPS_XLDST_UIMM16 operand.
 
Approved - please apply.

Cheers
  Nick

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

* Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
  2016-04-13 13:32     ` Nick Clifton
@ 2016-04-13 14:41       ` Andrew Burgess
  2016-04-13 15:30         ` Nick Clifton
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2016-04-13 14:41 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils, Claudiu.Zissulescu, Cupertino.Miranda, noamca

* Nick Clifton <nickc@redhat.com> [2016-04-13 14:32:38 +0100]:

> Hi Andrew,
> 
> > opcodes/ChangeLog:
> > 
> > 	* arc-dis.c (arc_insn_length): New function.
> > 	(print_insn_arc): Use arc_insn_length.
> 
> Approved - please apply, but ...
> 
> > +static int
> > +arc_insn_length (bfd_byte msb, bfd_byte lsb ATTRIBUTE_UNUSED,
> > +                 struct disassemble_info *info)
> 
> Would this function ever return a negative value ?  I assume not, so
> it would make sense for its return type to be "unsigned int".

I'm not sure is the answer.  I agree that arc_insn_length will never
return a negative, however, the return value from arc_insn_length is
used to prime a variable that is then the return value for
print_insn_arc, which is also defined to return 'int', and is part of
the disassembler API, and does return a negative value if there's an
error.

It was this relationship that originally lead me to make
arc_insn_length return an 'int'.

Given that it will only ever return small positive integers there
should be no problem making it return an unsigned value then casting
to int in print_insn_arc - would this be preferred?

Thanks,
Andrew

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

* Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
  2016-04-13 14:41       ` Andrew Burgess
@ 2016-04-13 15:30         ` Nick Clifton
  2016-04-14 12:40           ` Andrew Burgess
  0 siblings, 1 reply; 20+ messages in thread
From: Nick Clifton @ 2016-04-13 15:30 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils, Claudiu.Zissulescu, Cupertino.Miranda, noamca

Hi Andrew,

>> Would this function ever return a negative value ?  I assume not, so
>> it would make sense for its return type to be "unsigned int".
> 
> I'm not sure is the answer.  I agree that arc_insn_length will never
> return a negative, however, the return value from arc_insn_length is
> used to prime a variable that is then the return value for
> print_insn_arc, which is also defined to return 'int', and is part of
> the disassembler API, and does return a negative value if there's an
> error.
> 
> It was this relationship that originally lead me to make
> arc_insn_length return an 'int'.
> 
> Given that it will only ever return small positive integers there
> should be no problem making it return an unsigned value then casting
> to int in print_insn_arc - would this be preferred?

Yes.  Or (better IMHO) just explicitly set the return value for 
print_insn_arc based upon testing the return value from arc_insn_length.
Ie something like:

  len = arc_insn_length (...);
  if (len == 0)
    return -1;

(I am assuming here that a returned length of zero should never happen
with a valid instruction, and can therefore be used as an error return
value).

Cheers
  Nick

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

* Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
  2016-04-13 15:30         ` Nick Clifton
@ 2016-04-14 12:40           ` Andrew Burgess
  2016-04-14 14:45             ` Nick Clifton
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2016-04-14 12:40 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils, Claudiu.Zissulescu, Cupertino.Miranda, noamca

* Nick Clifton <nickc@redhat.com> [2016-04-13 16:30:38 +0100]:

> Hi Andrew,
> 
> >> Would this function ever return a negative value ?  I assume not, so
> >> it would make sense for its return type to be "unsigned int".
> > 
> > I'm not sure is the answer.  I agree that arc_insn_length will never
> > return a negative, however, the return value from arc_insn_length is
> > used to prime a variable that is then the return value for
> > print_insn_arc, which is also defined to return 'int', and is part of
> > the disassembler API, and does return a negative value if there's an
> > error.
> > 
> > It was this relationship that originally lead me to make
> > arc_insn_length return an 'int'.
> > 
> > Given that it will only ever return small positive integers there
> > should be no problem making it return an unsigned value then casting
> > to int in print_insn_arc - would this be preferred?
> 
> Yes.  Or (better IMHO) just explicitly set the return value for 
> print_insn_arc based upon testing the return value from arc_insn_length.
> Ie something like:
> 
>   len = arc_insn_length (...);
>   if (len == 0)
>     return -1;
> 
> (I am assuming here that a returned length of zero should never happen
> with a valid instruction, and can therefore be used as an error return
> value).

Just so I know I've done what you imagined, here's the patch that I
think addresses your point.  Is this OK?  It's mostly the same except
for type changes associated with insnLen.

Thanks,
Andrew

---

Move the logic that calculates the instruction length out to a new
function.  Restructure the code to make it simpler.

opcodes/ChangeLog:

	* arc-dis.c (arc_insn_length): New function.
	(print_insn_arc): Use arc_insn_length, change insnLen to unsigned.
	(find_format): Change insnLen parameter to unsigned.
---
 opcodes/ChangeLog |  6 ++++++
 opcodes/arc-dis.c | 57 ++++++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
index eb8bd67..7757ef6 100644
--- a/opcodes/arc-dis.c
+++ b/opcodes/arc-dis.c
@@ -105,7 +105,7 @@ special_flag_p (const char *opname,
 /* Find proper format for the given opcode.  */
 static const struct arc_opcode *
 find_format (const struct arc_opcode *arc_table,
-	     unsigned *insn, int insnLen,
+	     unsigned *insn, unsigned int insnLen,
 	     unsigned isa_mask)
 {
   unsigned int i = 0;
@@ -309,6 +309,37 @@ get_auxreg (const struct arc_opcode *opcode,
     }
   return NULL;
 }
+
+/* Calculate the instruction length for an instruction starting with MSB
+   and LSB, the most and least significant byte.  The ISA_MASK is used to
+   filter the instructions considered to only those that are part of the
+   current architecture.
+
+   The instruction lengths are calculated from the ARC_OPCODE table, and
+   cached for later use.  */
+
+static unsigned int
+arc_insn_length (bfd_byte msb, struct disassemble_info *info)
+{
+  bfd_byte major_opcode = msb >> 3;
+
+  switch (info->mach)
+    {
+    case bfd_mach_arc_nps400:
+    case bfd_mach_arc_arc700:
+    case bfd_mach_arc_arc600:
+      return (major_opcode > 0xb) ? 2 : 4;
+      break;
+
+    case bfd_mach_arc_arcv2:
+      return (major_opcode > 0x7) ? 2 : 4;
+      break;
+
+    default:
+      abort ();
+    }
+}
+
 /* Disassemble ARC instructions.  */
 
 static int
@@ -318,7 +349,7 @@ print_insn_arc (bfd_vma memaddr,
   bfd_byte buffer[4];
   unsigned int lowbyte, highbyte;
   int status;
-  int insnLen = 0;
+  unsigned int insnLen;
   unsigned insn[2] = { 0, 0 };
   unsigned isa_mask;
   const unsigned char *opidx;
@@ -422,20 +453,19 @@ print_insn_arc (bfd_vma memaddr,
       return size;
     }
 
-  if ((((buffer[lowbyte] & 0xf8) > 0x38)
-       && ((buffer[lowbyte] & 0xf8) != 0x48))
-      || ((info->mach == bfd_mach_arc_arcv2)
-	  && ((buffer[lowbyte] & 0xF8) == 0x48)) /* FIXME! ugly.  */
-      )
+  insnLen = arc_insn_length (buffer[lowbyte], info);
+  switch (insnLen)
     {
-      /* This is a short instruction.  */
-      insnLen = 2;
+    case 2:
       insn[0] = (buffer[lowbyte] << 8) | buffer[highbyte];
-    }
-  else
-    {
-      insnLen = 4;
+      break;
 
+    default:
+      /* An unknown instruction is treated as being length 4.  This is
+         possibly not the best solution, but matches the behaviour that was
+         in place before the table based instruction length look-up was
+         introduced.  */
+    case 4:
       /* This is a long instruction: Read the remaning 2 bytes.  */
       status = (*info->read_memory_func) (memaddr + 2, &buffer[2], 2, info);
       if (status != 0)
@@ -444,6 +474,7 @@ print_insn_arc (bfd_vma memaddr,
 	  return -1;
 	}
       insn[0] = ARRANGE_ENDIAN (info, buffer);
+      break;
     }
 
   /* Set some defaults for the insn info.  */
-- 
2.5.1

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

* Re: [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function
  2016-04-14 12:40           ` Andrew Burgess
@ 2016-04-14 14:45             ` Nick Clifton
  0 siblings, 0 replies; 20+ messages in thread
From: Nick Clifton @ 2016-04-14 14:45 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils, Claudiu.Zissulescu, Cupertino.Miranda, noamca

Hi Andrew,

> opcodes/ChangeLog:
> 
> 	* arc-dis.c (arc_insn_length): New function.
> 	(print_insn_arc): Use arc_insn_length, change insnLen to unsigned.
> 	(find_format): Change insnLen parameter to unsigned.
 
Looks great - please commit.

Cheers
  Nick

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

end of thread, other threads:[~2016-04-14 14:45 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-12 11:00 [PATCHv2 0/3] ARC: Add another group of nps instructions Andrew Burgess
2016-04-07 11:05 ` [PATCH " Andrew Burgess
2016-04-07 11:05   ` [PATCH 1/3] opcodes/arc: Compute insn lengths in disassembler Andrew Burgess
2016-04-07 11:27     ` Claudiu Zissulescu
2016-04-12  9:27     ` Claudiu Zissulescu
2016-04-07 11:05   ` [PATCH 2/3] bfd/arc: Rename enum entries to avoid conflicts Andrew Burgess
2016-04-07 11:20     ` Claudiu Zissulescu
2016-04-07 11:05   ` [PATCH 3/3] arc/nps400 : New cmem instructions and associated relocation Andrew Burgess
2016-04-11 12:55     ` Cupertino Miranda
2016-04-12 11:00   ` [PATCHv2 1/3] opcodes/arc: Move instruction length logic to new function Andrew Burgess
2016-04-12 12:27     ` Claudiu Zissulescu
2016-04-13 13:32     ` Nick Clifton
2016-04-13 14:41       ` Andrew Burgess
2016-04-13 15:30         ` Nick Clifton
2016-04-14 12:40           ` Andrew Burgess
2016-04-14 14:45             ` Nick Clifton
2016-04-12 11:00   ` [PATCHv2 2/3] bfd/arc: Rename enum entries to avoid conflicts Andrew Burgess
2016-04-13 13:34     ` Nick Clifton
2016-04-12 11:00   ` [PATCHv2 3/3] arc/nps400 : New cmem instructions and associated relocation Andrew Burgess
2016-04-13 13:37     ` Nick Clifton

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).