public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] x86: break gas dependency on libopcodes
@ 2022-11-18  9:10 Jan Beulich
  2022-11-18  9:12 ` [PATCH v2 1/4] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes Jan Beulich
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Jan Beulich @ 2022-11-18  9:10 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

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.

1: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes
2: remove i386-opc.c
3: break gas dependency on libopcodes
4: drop sentinel from i386_optab[]

Jan

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

* [PATCH v2 1/4] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes
  2022-11-18  9:10 [PATCH v2 0/4] x86: break gas dependency on libopcodes Jan Beulich
@ 2022-11-18  9:12 ` Jan Beulich
  2022-11-18  9:13 ` [PATCH v2 2/4] x86: remove i386-opc.c Jan Beulich
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Jan Beulich @ 2022-11-18  9:12 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

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
@@ -2969,6 +2969,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
@@ -1816,7 +1816,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))
@@ -1946,7 +1946,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))
     {
@@ -2009,7 +2009,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
@@ -924,7 +924,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
@@ -990,8 +990,6 @@ typedef struct insn_template
 }
 insn_template;
 
-extern const insn_template i386_optab[];
-
 /* these are for register name --> number & type hash lookup */
 typedef struct
 {
@@ -1011,6 +1009,3 @@ typedef struct
 #define Dw2Inval (-1)
 }
 reg_entry;
-
-extern const reg_entry i386_regtab[];
-extern const unsigned int i386_regtab_size;
--- a/opcodes/i386-tbl.h
+++ b/opcodes/i386-tbl.h
@@ -20,7 +20,7 @@
 
 /* i386 opcode table.  */
 
-const insn_template i386_optab[] =
+static const insn_template i386_optab[] =
 {
   { "mov", 0xa0, 2, None,
     { 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 0, 0, 0,
@@ -59958,7 +59958,7 @@ const insn_template i386_optab[] =
 
 /* i386 register table.  */
 
-const reg_entry i386_regtab[] =
+static const reg_entry i386_regtab[] =
 {
   { "al",
     { { 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0,
@@ -61114,4 +61114,4 @@ const reg_entry i386_regtab[] =
     0, 0, { 39, 64 } },
 };
 
-const unsigned int i386_regtab_size = ARRAY_SIZE (i386_regtab);
+static const unsigned int i386_regtab_size = ARRAY_SIZE (i386_regtab);


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

* [PATCH v2 2/4] x86: remove i386-opc.c
  2022-11-18  9:10 [PATCH v2 0/4] x86: break gas dependency on libopcodes Jan Beulich
  2022-11-18  9:12 ` [PATCH v2 1/4] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes Jan Beulich
@ 2022-11-18  9:13 ` Jan Beulich
  2022-11-18  9:13 ` [PATCH v2 3/4] x86: break gas dependency on libopcodes Jan Beulich
  2022-11-18  9:14 ` [PATCH v2 4/4] x86: drop sentinel from i386_optab[] Jan Beulich
  3 siblings, 0 replies; 14+ messages in thread
From: Jan Beulich @ 2022-11-18  9:13 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

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.
---
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/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"


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

* [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-18  9:10 [PATCH v2 0/4] x86: break gas dependency on libopcodes Jan Beulich
  2022-11-18  9:12 ` [PATCH v2 1/4] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes Jan Beulich
  2022-11-18  9:13 ` [PATCH v2 2/4] x86: remove i386-opc.c Jan Beulich
@ 2022-11-18  9:13 ` Jan Beulich
  2022-11-21 16:26   ` H.J. Lu
  2022-11-18  9:14 ` [PATCH v2 4/4] x86: drop sentinel from i386_optab[] Jan Beulich
  3 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2022-11-18  9:13 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

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)
 	;;
 
       *)


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

* [PATCH v2 4/4] x86: drop sentinel from i386_optab[]
  2022-11-18  9:10 [PATCH v2 0/4] x86: break gas dependency on libopcodes Jan Beulich
                   ` (2 preceding siblings ...)
  2022-11-18  9:13 ` [PATCH v2 3/4] x86: break gas dependency on libopcodes Jan Beulich
@ 2022-11-18  9:14 ` Jan Beulich
  3 siblings, 0 replies; 14+ messages in thread
From: Jan Beulich @ 2022-11-18  9:14 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

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.
---
We may want to consider having i386-gen also produce a table of type
"templates[]", which we could then iterate through without needing to
do any allocations (and with fewer overall iterations).

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2981,31 +2981,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
@@ -1916,16 +1916,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");
 }
 


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

* Re: [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-18  9:13 ` [PATCH v2 3/4] x86: break gas dependency on libopcodes Jan Beulich
@ 2022-11-21 16:26   ` H.J. Lu
  2022-11-21 16:32     ` Jan Beulich
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2022-11-21 16:26 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Fri, Nov 18, 2022 at 1:13 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> 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)
>         ;;
>
>        *)
>

We need to add some dependencies on i386-opc.tbl and i386-reg.tbl for
--enable-maintainer-mode to check if i386-tbl.h and i386-init.h are up to
date.  It doesn't need to regenerate i386-tbl.h.  An error message should be
sufficient.

-- 
H.J.

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

* Re: [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-21 16:26   ` H.J. Lu
@ 2022-11-21 16:32     ` Jan Beulich
  2022-11-21 16:43       ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2022-11-21 16:32 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 21.11.2022 17:26, H.J. Lu wrote:
> On Fri, Nov 18, 2022 at 1:13 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> 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)
>>         ;;
>>
>>        *)
>>
> 
> We need to add some dependencies on i386-opc.tbl and i386-reg.tbl for
> --enable-maintainer-mode to check if i386-tbl.h and i386-init.h are up to
> date.  It doesn't need to regenerate i386-tbl.h.  An error message should be
> sufficient.

This isn't needed here, but in patch 2, where I'm already adjusting
existing dependencies. Since I'm not modifying any toplevel files, the
building of opcodes/ still ought to be happening before the building of
gas/, so I don't see why any further changes should be necessary. Please
clarify if you see any such reason.

Jan

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

* Re: [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-21 16:32     ` Jan Beulich
@ 2022-11-21 16:43       ` H.J. Lu
  2022-11-21 16:52         ` Jan Beulich
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2022-11-21 16:43 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Nov 21, 2022 at 8:32 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 21.11.2022 17:26, H.J. Lu wrote:
> > On Fri, Nov 18, 2022 at 1:13 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> 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)
> >>         ;;
> >>
> >>        *)
> >>
> >
> > We need to add some dependencies on i386-opc.tbl and i386-reg.tbl for
> > --enable-maintainer-mode to check if i386-tbl.h and i386-init.h are up to
> > date.  It doesn't need to regenerate i386-tbl.h.  An error message should be
> > sufficient.
>
> This isn't needed here, but in patch 2, where I'm already adjusting
> existing dependencies. Since I'm not modifying any toplevel files, the
> building of opcodes/ still ought to be happening before the building of
> gas/, so I don't see why any further changes should be necessary. Please
> clarify if you see any such reason.
>

Since gas no longer depends on libopcodes, one may change i386-opc.tbl and
run "make" in gas.   It is more reliable than the fake dependency in opcodes.

-- 
H.J.

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

* Re: [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-21 16:43       ` H.J. Lu
@ 2022-11-21 16:52         ` Jan Beulich
  2022-11-21 17:04           ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2022-11-21 16:52 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 21.11.2022 17:43, H.J. Lu wrote:
> On Mon, Nov 21, 2022 at 8:32 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 21.11.2022 17:26, H.J. Lu wrote:
>>> On Fri, Nov 18, 2022 at 1:13 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> 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)
>>>>         ;;
>>>>
>>>>        *)
>>>>
>>>
>>> We need to add some dependencies on i386-opc.tbl and i386-reg.tbl for
>>> --enable-maintainer-mode to check if i386-tbl.h and i386-init.h are up to
>>> date.  It doesn't need to regenerate i386-tbl.h.  An error message should be
>>> sufficient.
>>
>> This isn't needed here, but in patch 2, where I'm already adjusting
>> existing dependencies. Since I'm not modifying any toplevel files, the
>> building of opcodes/ still ought to be happening before the building of
>> gas/, so I don't see why any further changes should be necessary. Please
>> clarify if you see any such reason.
>>
> 
> Since gas no longer depends on libopcodes, one may change i386-opc.tbl and
> run "make" in gas.

Is running make in gas/ a supported operation?

>   It is more reliable than the fake dependency in opcodes.

If the answer to the above is yes, then I may agree here. Except that then
I don't see how this dependency is being enforced prior to this series.
IOW - aren't you asking to address an unrelated issue?

Jan

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

* Re: [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-21 16:52         ` Jan Beulich
@ 2022-11-21 17:04           ` H.J. Lu
  2022-11-22  7:15             ` Jan Beulich
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2022-11-21 17:04 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Nov 21, 2022 at 8:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 21.11.2022 17:43, H.J. Lu wrote:
> > On Mon, Nov 21, 2022 at 8:32 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 21.11.2022 17:26, H.J. Lu wrote:
> >>> On Fri, Nov 18, 2022 at 1:13 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> 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)
> >>>>         ;;
> >>>>
> >>>>        *)
> >>>>
> >>>
> >>> We need to add some dependencies on i386-opc.tbl and i386-reg.tbl for
> >>> --enable-maintainer-mode to check if i386-tbl.h and i386-init.h are up to
> >>> date.  It doesn't need to regenerate i386-tbl.h.  An error message should be
> >>> sufficient.
> >>
> >> This isn't needed here, but in patch 2, where I'm already adjusting
> >> existing dependencies. Since I'm not modifying any toplevel files, the
> >> building of opcodes/ still ought to be happening before the building of
> >> gas/, so I don't see why any further changes should be necessary. Please
> >> clarify if you see any such reason.
> >>
> >
> > Since gas no longer depends on libopcodes, one may change i386-opc.tbl and
> > run "make" in gas.
>
> Is running make in gas/ a supported operation?
>
> >   It is more reliable than the fake dependency in opcodes.
>
> If the answer to the above is yes, then I may agree here. Except that then
> I don't see how this dependency is being enforced prior to this series.
> IOW - aren't you asking to address an unrelated issue?
>

It didn't work before.  Now we have a chance.

-- 
H.J.

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

* Re: [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-21 17:04           ` H.J. Lu
@ 2022-11-22  7:15             ` Jan Beulich
  2022-11-22 16:38               ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2022-11-22  7:15 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 21.11.2022 18:04, H.J. Lu wrote:
> On Mon, Nov 21, 2022 at 8:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 21.11.2022 17:43, H.J. Lu wrote:
>>> On Mon, Nov 21, 2022 at 8:32 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 21.11.2022 17:26, H.J. Lu wrote:
>>>>> On Fri, Nov 18, 2022 at 1:13 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> 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)
>>>>>>         ;;
>>>>>>
>>>>>>        *)
>>>>>>
>>>>>
>>>>> We need to add some dependencies on i386-opc.tbl and i386-reg.tbl for
>>>>> --enable-maintainer-mode to check if i386-tbl.h and i386-init.h are up to
>>>>> date.  It doesn't need to regenerate i386-tbl.h.  An error message should be
>>>>> sufficient.
>>>>
>>>> This isn't needed here, but in patch 2, where I'm already adjusting
>>>> existing dependencies. Since I'm not modifying any toplevel files, the
>>>> building of opcodes/ still ought to be happening before the building of
>>>> gas/, so I don't see why any further changes should be necessary. Please
>>>> clarify if you see any such reason.
>>>>
>>>
>>> Since gas no longer depends on libopcodes, one may change i386-opc.tbl and
>>> run "make" in gas.
>>
>> Is running make in gas/ a supported operation?
>>
>>>   It is more reliable than the fake dependency in opcodes.
>>
>> If the answer to the above is yes, then I may agree here. Except that then
>> I don't see how this dependency is being enforced prior to this series.
>> IOW - aren't you asking to address an unrelated issue?
>>
> 
> It didn't work before.  Now we have a chance.

But then please in a separate change, on top of this series. I'm willing
to make such a patch (whether that then finds your approval is a separate
question), but I'd please like to see things unblocked here.

Jan

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

* Re: [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-22  7:15             ` Jan Beulich
@ 2022-11-22 16:38               ` H.J. Lu
  2022-11-22 17:05                 ` Jan Beulich
  0 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2022-11-22 16:38 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Mon, Nov 21, 2022 at 11:15 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 21.11.2022 18:04, H.J. Lu wrote:
> > On Mon, Nov 21, 2022 at 8:52 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 21.11.2022 17:43, H.J. Lu wrote:
> >>> On Mon, Nov 21, 2022 at 8:32 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 21.11.2022 17:26, H.J. Lu wrote:
> >>>>> On Fri, Nov 18, 2022 at 1:13 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> 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)
> >>>>>>         ;;
> >>>>>>
> >>>>>>        *)
> >>>>>>
> >>>>>
> >>>>> We need to add some dependencies on i386-opc.tbl and i386-reg.tbl for
> >>>>> --enable-maintainer-mode to check if i386-tbl.h and i386-init.h are up to
> >>>>> date.  It doesn't need to regenerate i386-tbl.h.  An error message should be
> >>>>> sufficient.
> >>>>
> >>>> This isn't needed here, but in patch 2, where I'm already adjusting
> >>>> existing dependencies. Since I'm not modifying any toplevel files, the
> >>>> building of opcodes/ still ought to be happening before the building of
> >>>> gas/, so I don't see why any further changes should be necessary. Please
> >>>> clarify if you see any such reason.
> >>>>
> >>>
> >>> Since gas no longer depends on libopcodes, one may change i386-opc.tbl and
> >>> run "make" in gas.
> >>
> >> Is running make in gas/ a supported operation?
> >>
> >>>   It is more reliable than the fake dependency in opcodes.
> >>
> >> If the answer to the above is yes, then I may agree here. Except that then
> >> I don't see how this dependency is being enforced prior to this series.
> >> IOW - aren't you asking to address an unrelated issue?
> >>
> >
> > It didn't work before.  Now we have a chance.
>
> But then please in a separate change, on top of this series. I'm willing
> to make such a patch (whether that then finds your approval is a separate
> question), but I'd please like to see things unblocked here.
>

Then please don't add the fake dependency.


-- 
H.J.

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

* Re: [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-22 16:38               ` H.J. Lu
@ 2022-11-22 17:05                 ` Jan Beulich
  2022-11-22 18:21                   ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Beulich @ 2022-11-22 17:05 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 22.11.2022 17:38, H.J. Lu wrote:
> On Mon, Nov 21, 2022 at 11:15 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 21.11.2022 18:04, H.J. Lu wrote:
>>> On Mon, Nov 21, 2022 at 8:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 21.11.2022 17:43, H.J. Lu wrote:
>>>>> On Mon, Nov 21, 2022 at 8:32 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> On 21.11.2022 17:26, H.J. Lu wrote:
>>>>>>> On Fri, Nov 18, 2022 at 1:13 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>>>
>>>>>>>> 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)
>>>>>>>>         ;;
>>>>>>>>
>>>>>>>>        *)
>>>>>>>>
>>>>>>>
>>>>>>> We need to add some dependencies on i386-opc.tbl and i386-reg.tbl for
>>>>>>> --enable-maintainer-mode to check if i386-tbl.h and i386-init.h are up to
>>>>>>> date.  It doesn't need to regenerate i386-tbl.h.  An error message should be
>>>>>>> sufficient.
>>>>>>
>>>>>> This isn't needed here, but in patch 2, where I'm already adjusting
>>>>>> existing dependencies. Since I'm not modifying any toplevel files, the
>>>>>> building of opcodes/ still ought to be happening before the building of
>>>>>> gas/, so I don't see why any further changes should be necessary. Please
>>>>>> clarify if you see any such reason.
>>>>>>
>>>>>
>>>>> Since gas no longer depends on libopcodes, one may change i386-opc.tbl and
>>>>> run "make" in gas.
>>>>
>>>> Is running make in gas/ a supported operation?
>>>>
>>>>>   It is more reliable than the fake dependency in opcodes.
>>>>
>>>> If the answer to the above is yes, then I may agree here. Except that then
>>>> I don't see how this dependency is being enforced prior to this series.
>>>> IOW - aren't you asking to address an unrelated issue?
>>>>
>>>
>>> It didn't work before.  Now we have a chance.
>>
>> But then please in a separate change, on top of this series. I'm willing
>> to make such a patch (whether that then finds your approval is a separate
>> question), but I'd please like to see things unblocked here.
> 
> Then please don't add the fake dependency.

But then the build will be broken in opcodes/. That one is needed _now_.
The new dependency in gas/ is orthogonal and hence can be added
subsequently. If that's not your understanding, then please go into
further detail.

Jan

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

* Re: [PATCH v2 3/4] x86: break gas dependency on libopcodes
  2022-11-22 17:05                 ` Jan Beulich
@ 2022-11-22 18:21                   ` H.J. Lu
  0 siblings, 0 replies; 14+ messages in thread
From: H.J. Lu @ 2022-11-22 18:21 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Tue, Nov 22, 2022 at 9:05 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 22.11.2022 17:38, H.J. Lu wrote:
> > On Mon, Nov 21, 2022 at 11:15 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 21.11.2022 18:04, H.J. Lu wrote:
> >>> On Mon, Nov 21, 2022 at 8:52 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 21.11.2022 17:43, H.J. Lu wrote:
> >>>>> On Mon, Nov 21, 2022 at 8:32 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> On 21.11.2022 17:26, H.J. Lu wrote:
> >>>>>>> On Fri, Nov 18, 2022 at 1:13 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>
> >>>>>>>> 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)
> >>>>>>>>         ;;
> >>>>>>>>
> >>>>>>>>        *)
> >>>>>>>>
> >>>>>>>
> >>>>>>> We need to add some dependencies on i386-opc.tbl and i386-reg.tbl for
> >>>>>>> --enable-maintainer-mode to check if i386-tbl.h and i386-init.h are up to
> >>>>>>> date.  It doesn't need to regenerate i386-tbl.h.  An error message should be
> >>>>>>> sufficient.
> >>>>>>
> >>>>>> This isn't needed here, but in patch 2, where I'm already adjusting
> >>>>>> existing dependencies. Since I'm not modifying any toplevel files, the
> >>>>>> building of opcodes/ still ought to be happening before the building of
> >>>>>> gas/, so I don't see why any further changes should be necessary. Please
> >>>>>> clarify if you see any such reason.
> >>>>>>
> >>>>>
> >>>>> Since gas no longer depends on libopcodes, one may change i386-opc.tbl and
> >>>>> run "make" in gas.
> >>>>
> >>>> Is running make in gas/ a supported operation?
> >>>>
> >>>>>   It is more reliable than the fake dependency in opcodes.
> >>>>
> >>>> If the answer to the above is yes, then I may agree here. Except that then
> >>>> I don't see how this dependency is being enforced prior to this series.
> >>>> IOW - aren't you asking to address an unrelated issue?
> >>>>
> >>>
> >>> It didn't work before.  Now we have a chance.
> >>
> >> But then please in a separate change, on top of this series. I'm willing
> >> to make such a patch (whether that then finds your approval is a separate
> >> question), but I'd please like to see things unblocked here.
> >
> > Then please don't add the fake dependency.
>
> But then the build will be broken in opcodes/. That one is needed _now_.
> The new dependency in gas/ is orthogonal and hence can be added
> subsequently. If that's not your understanding, then please go into
> further detail.
>

Here is a patch to include opcodes/i386-tbl.h directly:

https://sourceware.org/pipermail/binutils/2022-November/124583.html

-- 
H.J.

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

end of thread, other threads:[~2022-11-22 18:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-18  9:10 [PATCH v2 0/4] x86: break gas dependency on libopcodes Jan Beulich
2022-11-18  9:12 ` [PATCH v2 1/4] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes Jan Beulich
2022-11-18  9:13 ` [PATCH v2 2/4] x86: remove i386-opc.c Jan Beulich
2022-11-18  9:13 ` [PATCH v2 3/4] x86: break gas dependency on libopcodes Jan Beulich
2022-11-21 16:26   ` H.J. Lu
2022-11-21 16:32     ` Jan Beulich
2022-11-21 16:43       ` H.J. Lu
2022-11-21 16:52         ` Jan Beulich
2022-11-21 17:04           ` H.J. Lu
2022-11-22  7:15             ` Jan Beulich
2022-11-22 16:38               ` H.J. Lu
2022-11-22 17:05                 ` Jan Beulich
2022-11-22 18:21                   ` H.J. Lu
2022-11-18  9:14 ` [PATCH v2 4/4] x86: drop sentinel from i386_optab[] Jan Beulich

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