[PATCH v3 0/6] x86: break gas dependency on libopcodes

[PATCH v3 0/6] x86: break gas dependency on libopcodes

[Jan Beulich]

Unlike many other architectures, x86 does not share an opcode table
between assembly and disassembly. Any consumer of libopcodes would only
ever access one of the two.

v3: Two new patches (4 and 6). Correct an omission from patch 2.

1: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes
2: remove i386-opc.c
3: break gas dependency on libopcodes
4: add generated tables dependency check to gas
5: drop sentinel from i386_optab[]
6: generate template sets data at build time

Jan

[PATCH v3 1/6] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes

[Jan Beulich]

Unlike many other architectures, x86 does not share an opcode table
between assembly and disassembly. Any consumer of libopcodes would only
ever access one of the two. Since gas is the only consumer of the
assembly data, move it there. While doing so mark respective entities
"static" in i386-gen (we may want to do away with i386_regtab_size
altogether).

This also shrinks the number of relocations to be processed for
libopcodes.so by about 30%.
---
v2: Re-base over the (premature) moving of i386_seg_prefixes[]. Move
    inclusion point of opcodes/i386-tbl.h.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2965,6 +2965,8 @@ i386_mach (void)
     as_fatal (_("unknown architecture"));
 }
 \f
+#include "opcodes/i386-tbl.h"
+
 void
 md_begin (void)
 {
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -1815,7 +1815,7 @@ process_i386_opcodes (FILE *table)
 					 xcalloc, free);
 
   fprintf (table, "\n/* i386 opcode table.  */\n\n");
-  fprintf (table, "const insn_template i386_optab[] =\n{\n");
+  fprintf (table, "static const insn_template i386_optab[] =\n{\n");
 
   /* Put everything on opcode array.  */
   while (!feof (fp))
@@ -1945,7 +1945,7 @@ process_i386_registers (FILE *table)
 	  xstrerror (errno));
 
   fprintf (table, "\n/* i386 register table.  */\n\n");
-  fprintf (table, "const reg_entry i386_regtab[] =\n{\n");
+  fprintf (table, "static const reg_entry i386_regtab[] =\n{\n");
 
   while (!feof (fp))
     {
@@ -2008,7 +2008,7 @@ process_i386_registers (FILE *table)
 
   fprintf (table, "};\n");
 
-  fprintf (table, "\nconst unsigned int i386_regtab_size = ARRAY_SIZE (i386_regtab);\n");
+  fprintf (table, "\nstatic const unsigned int i386_regtab_size = ARRAY_SIZE (i386_regtab);\n");
 }
 
 static void
--- a/opcodes/i386-opc.c
+++ b/opcodes/i386-opc.c
@@ -21,4 +21,3 @@
 #include "sysdep.h"
 #include "libiberty.h"
 #include "i386-opc.h"
-#include "i386-tbl.h"
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -921,7 +921,7 @@ typedef union i386_operand_type
 typedef struct insn_template
 {
   /* instruction name sans width suffix ("mov" for movl insns) */
-  char *name;
+  const char *name;
 
   /* Bitfield arrangement is such that individual fields can be easily
      extracted (in native builds at least) - either by at most a masking
@@ -987,8 +987,6 @@ typedef struct insn_template
 }
 insn_template;
 
-extern const insn_template i386_optab[];
-
 /* these are for register name --> number & type hash lookup */
 typedef struct
 {
@@ -1008,6 +1006,3 @@ typedef struct
 #define Dw2Inval (-1)
 }
 reg_entry;
-
-extern const reg_entry i386_regtab[];
-extern const unsigned int i386_regtab_size;

[PATCH v3 2/6] x86: remove i386-opc.c

[Jan Beulich]

Remove the now empty i386-opc.c. To compensate, tie table generation in
opcodes/ to the building of i386-dis.o, despite the file not really
depending on the generated data.
---
v3: Add missing opcodes/configure{.ac.,} change.
v2: Leftovers from earlier patch moving i386_seg_prefixes[].
---
RFC: Is there a better way to specify extra dependencies, such that
     table generation and compilation of i386-dis.c could be kept
     separate (and hence processable in parallel)?

--- a/opcodes/Makefile.am
+++ b/opcodes/Makefile.am
@@ -162,7 +162,6 @@ TARGET32_LIBOPCODES_CFILES = \
 	h8300-dis.c \
 	hppa-dis.c \
 	i386-dis.c \
-	i386-opc.c \
 	ip2k-asm.c \
 	ip2k-desc.c \
 	ip2k-dis.c \
@@ -562,10 +561,9 @@ $(srcdir)/i386%tbl.h $(srcdir)/i386%init
 		< $(srcdir)/i386-opc.tbl \
 		| ./i386-gen$(EXEEXT_FOR_BUILD) --srcdir $(srcdir)
 
-i386-opc.lo: $(srcdir)/i386-tbl.h
-# While not really a dependency, specify i386-init.h here as well to make sure
-# it is generated even if i386-tbl.h is present and up-to-date.
-i386-opc.lo: $(srcdir)/i386-init.h
+# While not really dependencies, specify i386-{init,tbl}.h here as well to
+# make sure they are re-generated as necessary.
+i386-dis.lo: $(srcdir)/i386-tbl.h $(srcdir)/i386-init.h
 
 ia64-gen$(EXEEXT_FOR_BUILD): ia64-gen.o $(BUILD_LIB_DEPS)
 	$(AM_V_CCLD)$(LINK_FOR_BUILD) ia64-gen.o $(BUILD_LIBS)
--- a/opcodes/Makefile.in
+++ b/opcodes/Makefile.in
@@ -554,7 +554,6 @@ TARGET32_LIBOPCODES_CFILES = \
 	h8300-dis.c \
 	hppa-dis.c \
 	i386-dis.c \
-	i386-opc.c \
 	ip2k-asm.c \
 	ip2k-desc.c \
 	ip2k-dis.c \
@@ -947,7 +946,6 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/h8300-dis.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/hppa-dis.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/i386-dis.Plo@am__quote@
-@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/i386-opc.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ia64-dis.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ia64-opc.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/ip2k-asm.Plo@am__quote@
@@ -1537,10 +1535,9 @@ $(srcdir)/i386%tbl.h $(srcdir)/i386%init
 		< $(srcdir)/i386-opc.tbl \
 		| ./i386-gen$(EXEEXT_FOR_BUILD) --srcdir $(srcdir)
 
-i386-opc.lo: $(srcdir)/i386-tbl.h
-# While not really a dependency, specify i386-init.h here as well to make sure
-# it is generated even if i386-tbl.h is present and up-to-date.
-i386-opc.lo: $(srcdir)/i386-init.h
+# While not really dependencies, specify i386-{init,tbl}.h here as well to
+# make sure they are re-generated as necessary.
+i386-dis.lo: $(srcdir)/i386-tbl.h $(srcdir)/i386-init.h
 
 ia64-gen$(EXEEXT_FOR_BUILD): ia64-gen.o $(BUILD_LIB_DEPS)
 	$(AM_V_CCLD)$(LINK_FOR_BUILD) ia64-gen.o $(BUILD_LIBS)
--- a/opcodes/configure
+++ b/opcodes/configure
@@ -12534,7 +12534,7 @@ if test x${all_targets} = xfalse ; then
 	bfd_h8300_arch)		ta="$ta h8300-dis.lo" ;;
 	bfd_hppa_arch)		ta="$ta hppa-dis.lo" ;;
 	bfd_i386_arch|bfd_iamcu_arch)
-				ta="$ta i386-dis.lo i386-opc.lo" ;;
+				ta="$ta i386-dis.lo" ;;
 	bfd_ia64_arch)		ta="$ta ia64-dis.lo ia64-opc.lo" ;;
 	bfd_ip2k_arch)		ta="$ta ip2k-asm.lo ip2k-desc.lo ip2k-dis.lo ip2k-ibld.lo ip2k-opc.lo" using_cgen=yes ;;
 	bfd_epiphany_arch)	ta="$ta epiphany-asm.lo epiphany-desc.lo epiphany-dis.lo epiphany-ibld.lo epiphany-opc.lo" using_cgen=yes ;;
--- a/opcodes/configure.ac
+++ b/opcodes/configure.ac
@@ -282,7 +282,7 @@ if test x${all_targets} = xfalse ; then
 	bfd_h8300_arch)		ta="$ta h8300-dis.lo" ;;
 	bfd_hppa_arch)		ta="$ta hppa-dis.lo" ;;
 	bfd_i386_arch|bfd_iamcu_arch)
-				ta="$ta i386-dis.lo i386-opc.lo" ;;
+				ta="$ta i386-dis.lo" ;;
 	bfd_ia64_arch)		ta="$ta ia64-dis.lo ia64-opc.lo" ;;
 	bfd_ip2k_arch)		ta="$ta ip2k-asm.lo ip2k-desc.lo ip2k-dis.lo ip2k-ibld.lo ip2k-opc.lo" using_cgen=yes ;;
 	bfd_epiphany_arch)	ta="$ta epiphany-asm.lo epiphany-desc.lo epiphany-dis.lo epiphany-ibld.lo epiphany-opc.lo" using_cgen=yes ;;
--- a/opcodes/i386-opc.c
+++ /dev/null
@@ -1,23 +0,0 @@
-/* Intel 80386 opcode table
-   Copyright (C) 2007-2022 Free Software Foundation, Inc.
-
-   This file is part of the GNU opcodes library.
-
-   This library 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, or (at your option)
-   any later version.
-
-   It 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.  */
-
-#include "sysdep.h"
-#include "libiberty.h"
-#include "i386-opc.h"

[PATCH v3 3/6] x86: break gas dependency on libopcodes

[Jan Beulich]

gas doesn't use anything from libopcodes anymore - suppress linking in
that library.
---
v2: New, split off from larger earlier patch.

--- a/gas/configure
+++ b/gas/configure
@@ -12263,7 +12263,7 @@ _ACEOF
 
     # Do we need the opcodes library?
     case ${cpu_type} in
-      vax | tic30)
+      vax | tic30 | i386)
 	;;
 
       *)
--- a/gas/configure.ac
+++ b/gas/configure.ac
@@ -420,7 +420,7 @@ changequote([,])dnl
 
     # Do we need the opcodes library?
     case ${cpu_type} in
-      vax | tic30)
+      vax | tic30 | i386)
 	;;
 
       *)

[PATCH v3 4/6] x86: add generated tables dependency check to gas

[Jan Beulich]

As requested by H.J., just for the sake of people potentially building
in gas/ alone, add a check that the generated files in opcodes/ are
actually up-to-date. Personally I think this should at best be a
warning, but I can see how this may not be easily noticable among other
make output (depending in particular on the verbosity level).
---
v3: New.

--- a/gas/Makefile.am
+++ b/gas/Makefile.am
@@ -446,6 +446,17 @@ development.exp: $(BFDDIR)/development.s
 	$(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
 	  | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
 
+config/tc-i386.o: $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h
+
+i386_tbl_deps = $(srcdir)/../opcodes/i386-opc.tbl \
+	$(srcdir)/../opcodes/i386-reg.tbl \
+	$(srcdir)/../opcodes/i386-gen.c $(srcdir)/../opcodes/i386-opc.h
+
+$(srcdir)/../opcodes/i386%init.h $(srcdir)/../opcodes/i386%tbl.h: @MAINT@ $(i386_tbl_deps)
+	@echo '"$@" is outdated wrt "$?"' >&2
+	@echo 'Please rebuild from the top level or in $(CURDIR)/../opcodes/' >&2
+	@false
+
 EXTRA_as_new_SOURCES += config/m68k-parse.y
 config/m68k-parse.c: $(srcdir)/config/m68k-parse.y
 	$(SHELL) $(YLWRAP) $(srcdir)/config/m68k-parse.y y.tab.c $@ -- $(YACCCOMPILE)
--- a/gas/Makefile.in
+++ b/gas/Makefile.in
@@ -902,6 +902,10 @@ EXTRA_as_new_SOURCES = $(CFILES) $(HFILE
 EXPECT = expect
 RUNTEST = runtest
 RUNTESTFLAGS = 
+i386_tbl_deps = $(srcdir)/../opcodes/i386-opc.tbl \
+	$(srcdir)/../opcodes/i386-reg.tbl \
+	$(srcdir)/../opcodes/i386-gen.c $(srcdir)/../opcodes/i386-opc.h
+
 itbl_test_SOURCES = itbl-parse.y itbl-lex.l
 itbl_test_LDADD = itbl-tops.@OBJEXT@ itbl-test.@OBJEXT@ $(GASLIBS) @LEXLIB@
 
@@ -2060,6 +2064,13 @@ check-DEJAGNU: site.exp
 development.exp: $(BFDDIR)/development.sh
 	$(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
 	  | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
+
+config/tc-i386.o: $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h
+
+$(srcdir)/../opcodes/i386%init.h $(srcdir)/../opcodes/i386%tbl.h: @MAINT@ $(i386_tbl_deps)
+	@echo '"$@" is outdated wrt "$?"' >&2
+	@echo 'Please rebuild from the top level or in $(CURDIR)/../opcodes/' >&2
+	@false
 config/m68k-parse.c: $(srcdir)/config/m68k-parse.y
 	$(SHELL) $(YLWRAP) $(srcdir)/config/m68k-parse.y y.tab.c $@ -- $(YACCCOMPILE)
 config/m68k-parse.h: config/m68k-parse.c

[PATCH v3 5/6] x86: drop sentinel from i386_optab[]

[Jan Beulich]

Now that the table is local to gas, ARRAY_SIZE() can be used to
determine the end of the table. Re-arrange the processing loop in
md_begin() accordingly, at the same time folding the two calls to
notes_alloc() into just one.
---
v2: New.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2977,31 +2977,20 @@ md_begin (void)
   op_hash = str_htab_create ();
 
   {
-    const insn_template *optab;
-    templates *core_optab;
+    const insn_template *optab = i386_optab;
+    const insn_template *end = optab + ARRAY_SIZE (i386_optab);
 
-    /* Setup for loop.  */
-    optab = i386_optab;
-    core_optab = notes_alloc (sizeof (*core_optab));
-    core_optab->start = optab;
-
-    while (1)
+    while (optab < end)
       {
-	++optab;
-	if (optab->name == NULL
-	    || strcmp (optab->name, (optab - 1)->name) != 0)
-	  {
-	    /* different name --> ship out current template list;
-	       add to hash table; & begin anew.  */
-	    core_optab->end = optab;
-	    if (str_hash_insert (op_hash, (optab - 1)->name, core_optab, 0))
-	      as_fatal (_("duplicate %s"), (optab - 1)->name);
+	templates *core_optab = notes_alloc (sizeof (*core_optab));
 
-	    if (optab->name == NULL)
-	      break;
-	    core_optab = notes_alloc (sizeof (*core_optab));
-	    core_optab->start = optab;
-	  }
+	core_optab->start = optab;
+	while (++optab < end)
+	  if (strcmp (optab->name, optab[-1].name) != 0)
+	    break;
+	core_optab->end = optab;
+	if (str_hash_insert (op_hash, optab[-1].name, core_optab, 0))
+	  as_fatal (_("duplicate %s"), optab[-1].name);
       }
   }
 
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -1915,16 +1915,6 @@ process_i386_opcodes (FILE *table)
 
   fclose (fp);
 
-  fprintf (table, "  { NULL, 0, 0, 0,\n");
-
-  process_i386_opcode_modifier (table, "0", 0, 0, NULL, -1);
-
-  process_i386_cpu_flag (table, "0", 0, ",", "    ", -1);
-
-  fprintf (table, "    { ");
-  process_i386_operand_type (table, "0", stage_opcodes, "\t  ", -1);
-  fprintf (table, " } }\n");
-
   fprintf (table, "};\n");
 }
 

[PATCH v3 6/6] x86: generate template sets data at build time

[Jan Beulich]

Speed up gas startup by avoiding runtime allocation of the instances of
type "templates". At the same time cut the memory requirement to just
very little over half (not even accounting for any overhead
notes_alloc() may incur) by reusing the "end" slot of a preceding entry
for the "start" slot of the subsequent one.
---
v3: New.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2977,21 +2977,16 @@ md_begin (void)
   op_hash = str_htab_create ();
 
   {
-    const insn_template *optab = i386_optab;
-    const insn_template *end = optab + ARRAY_SIZE (i386_optab);
+    const insn_template *const *sets = i386_op_sets;
+    const insn_template *const *end = sets + ARRAY_SIZE (i386_op_sets) - 1;
 
-    while (optab < end)
-      {
-	templates *core_optab = notes_alloc (sizeof (*core_optab));
-
-	core_optab->start = optab;
-	while (++optab < end)
-	  if (strcmp (optab->name, optab[-1].name) != 0)
-	    break;
-	core_optab->end = optab;
-	if (str_hash_insert (op_hash, optab[-1].name, core_optab, 0))
-	  as_fatal (_("duplicate %s"), optab[-1].name);
-      }
+    /* Type checks to compensate for the conversion through void * which
+       occurs during hash table insertion / lookup.  */
+    (void)(sets == &current_templates->start);
+    (void)(end == &current_templates->end);
+    for (; sets < end; ++sets)
+      if (str_hash_insert (op_hash, (*sets)->name, sets, 0))
+	as_fatal (_("duplicate %s"), (*sets)->name);
   }
 
   /* Initialize reg_hash hash table.  */
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -1800,7 +1800,7 @@ process_i386_opcodes (FILE *table)
 {
   FILE *fp;
   char buf[2048];
-  unsigned int i, j;
+  unsigned int i, j, nr;
   char *str, *p, *last, *name;
   htab_t opcode_hash_table;
   struct opcode_hash_entry **opcode_array = NULL;
@@ -1916,6 +1916,26 @@ process_i386_opcodes (FILE *table)
   fclose (fp);
 
   fprintf (table, "};\n");
+
+  /* Generate opcode sets array.  */
+  fprintf (table, "\n/* i386 opcode sets table.  */\n\n");
+  fprintf (table, "static const insn_template *i386_op_sets[] =\n{\n");
+  fprintf (table, "  i386_optab,\n");
+
+  for (nr = j = 0; j < i; j++)
+    {
+      struct opcode_hash_entry *next = opcode_array[j];
+
+      do
+	{
+	  ++nr;
+	  next = next->next;
+	}
+      while (next);
+      fprintf (table, "  i386_optab + %u,\n", nr);
+    }
+
+  fprintf (table, "};\n");
 }
 
 static void