public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] x86: break gas dependency on libopcodes
@ 2022-11-17 13:27 Jan Beulich
  2022-11-17 13:29 ` [PATCH 1/2] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes Jan Beulich
  2022-11-17 13:29 ` [PATCH 2/2] x86: break gas dependency on libopcodes Jan Beulich
  0 siblings, 2 replies; 10+ messages in thread
From: Jan Beulich @ 2022-11-17 13:27 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: break gas dependency on libopcodes

Jan

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

* [PATCH 1/2] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes
  2022-11-17 13:27 [PATCH 0/2] x86: break gas dependency on libopcodes Jan Beulich
@ 2022-11-17 13:29 ` Jan Beulich
  2022-11-17 13:29 ` [PATCH 2/2] x86: break gas dependency on libopcodes Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2022-11-17 13:29 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%.

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -34,6 +34,7 @@
 #include "sframe.h"
 #include "elf/x86-64.h"
 #include "opcodes/i386-init.h"
+#include "opcodes/i386-tbl.h"
 #include <limits.h>
 
 #ifndef INFER_ADDR_PREFIX
--- a/opcodes/Makefile.am
+++ b/opcodes/Makefile.am
@@ -562,10 +562,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-opc.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
@@ -1537,10 +1537,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-opc.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-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,7 +21,6 @@
 #include "sysdep.h"
 #include "libiberty.h"
 #include "i386-opc.h"
-#include "i386-tbl.h"
 
 /* To be indexed by segment register number.  */
 const unsigned char i386_seg_prefixes[] = {
--- 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
 {
@@ -1012,6 +1010,4 @@ typedef struct
 }
 reg_entry;
 
-extern const reg_entry i386_regtab[];
-extern const unsigned int i386_regtab_size;
 extern const unsigned char i386_seg_prefixes[6];


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

* [PATCH 2/2] x86: break gas dependency on libopcodes
  2022-11-17 13:27 [PATCH 0/2] x86: break gas dependency on libopcodes Jan Beulich
  2022-11-17 13:29 ` [PATCH 1/2] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes Jan Beulich
@ 2022-11-17 13:29 ` Jan Beulich
  2022-11-17 16:48   ` H.J. Lu
  1 sibling, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2022-11-17 13:29 UTC (permalink / raw)
  To: Binutils; +Cc: H.J. Lu

The only item gas still consumes from libopcodes is i386_seg_prefixes[],
which again is used by gas alone. Move it into the assembler, allowing
to remove the linking in of libopcodes.

To compensate, tie table generation in opcodes/ to the building of
i386-dis.o, despite the file not really depending on the generated data.
---
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/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -445,6 +445,16 @@ struct _i386_insn
 
 typedef struct _i386_insn i386_insn;
 
+/* To be indexed by segment register number.  */
+static const unsigned char i386_seg_prefixes[] = {
+  ES_PREFIX_OPCODE,
+  CS_PREFIX_OPCODE,
+  SS_PREFIX_OPCODE,
+  DS_PREFIX_OPCODE,
+  FS_PREFIX_OPCODE,
+  GS_PREFIX_OPCODE
+};
+
 /* Link RC type with corresponding string, that'll be looked for in
    asm.  */
 struct RC_name
--- 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)
 	;;
 
       *)
--- 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 \
@@ -564,7 +563,7 @@ $(srcdir)/i386%tbl.h $(srcdir)/i386%init
 
 # While not really dependencies, specify i386-{init,tbl}.h here as well to
 # make sure they are re-generated as necessary.
-i386-opc.lo: $(srcdir)/i386-tbl.h $(srcdir)/i386-init.h
+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@
@@ -1539,7 +1537,7 @@ $(srcdir)/i386%tbl.h $(srcdir)/i386%init
 
 # While not really dependencies, specify i386-{init,tbl}.h here as well to
 # make sure they are re-generated as necessary.
-i386-opc.lo: $(srcdir)/i386-tbl.h $(srcdir)/i386-init.h
+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,33 +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"
-
-/* To be indexed by segment register number.  */
-const unsigned char i386_seg_prefixes[] = {
-  ES_PREFIX_OPCODE,
-  CS_PREFIX_OPCODE,
-  SS_PREFIX_OPCODE,
-  DS_PREFIX_OPCODE,
-  FS_PREFIX_OPCODE,
-  GS_PREFIX_OPCODE
-};
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -1003,5 +1003,3 @@ typedef struct
 #define Dw2Inval (-1)
 }
 reg_entry;
-
-extern const unsigned char i386_seg_prefixes[6];


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

* Re: [PATCH 2/2] x86: break gas dependency on libopcodes
  2022-11-17 13:29 ` [PATCH 2/2] x86: break gas dependency on libopcodes Jan Beulich
@ 2022-11-17 16:48   ` H.J. Lu
  2022-11-17 16:53     ` Jan Beulich
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2022-11-17 16:48 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Thu, Nov 17, 2022 at 5:29 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> The only item gas still consumes from libopcodes is i386_seg_prefixes[],
> which again is used by gas alone. Move it into the assembler, allowing
> to remove the linking in of libopcodes.
>
> To compensate, tie table generation in opcodes/ to the building of
> i386-dis.o, despite the file not really depending on the generated data.
> ---
> 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/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -445,6 +445,16 @@ struct _i386_insn
>
>  typedef struct _i386_insn i386_insn;
>
> +/* To be indexed by segment register number.  */
> +static const unsigned char i386_seg_prefixes[] = {
> +  ES_PREFIX_OPCODE,
> +  CS_PREFIX_OPCODE,
> +  SS_PREFIX_OPCODE,
> +  DS_PREFIX_OPCODE,
> +  FS_PREFIX_OPCODE,
> +  GS_PREFIX_OPCODE
> +};
> +
>  /* Link RC type with corresponding string, that'll be looked for in
>     asm.  */
>  struct RC_name
> --- 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)
>         ;;

This change isn't needed to move i386_seg_prefixes to gas.

>        *)
> --- 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 \
> @@ -564,7 +563,7 @@ $(srcdir)/i386%tbl.h $(srcdir)/i386%init
>
>  # While not really dependencies, specify i386-{init,tbl}.h here as well to
>  # make sure they are re-generated as necessary.
> -i386-opc.lo: $(srcdir)/i386-tbl.h $(srcdir)/i386-init.h
> +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@
> @@ -1539,7 +1537,7 @@ $(srcdir)/i386%tbl.h $(srcdir)/i386%init
>
>  # While not really dependencies, specify i386-{init,tbl}.h here as well to
>  # make sure they are re-generated as necessary.
> -i386-opc.lo: $(srcdir)/i386-tbl.h $(srcdir)/i386-init.h
> +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,33 +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"
> -
> -/* To be indexed by segment register number.  */
> -const unsigned char i386_seg_prefixes[] = {
> -  ES_PREFIX_OPCODE,
> -  CS_PREFIX_OPCODE,
> -  SS_PREFIX_OPCODE,
> -  DS_PREFIX_OPCODE,
> -  FS_PREFIX_OPCODE,
> -  GS_PREFIX_OPCODE
> -};
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -1003,5 +1003,3 @@ typedef struct
>  #define Dw2Inval (-1)
>  }
>  reg_entry;
> -
> -extern const unsigned char i386_seg_prefixes[6];
>


-- 
H.J.

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

* Re: [PATCH 2/2] x86: break gas dependency on libopcodes
  2022-11-17 16:48   ` H.J. Lu
@ 2022-11-17 16:53     ` Jan Beulich
  2022-11-17 16:55       ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2022-11-17 16:53 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 17.11.2022 17:48, H.J. Lu wrote:
> On Thu, Nov 17, 2022 at 5:29 AM Jan Beulich <jbeulich@suse.com> wrote:
>> --- 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)
>>         ;;
> 
> This change isn't needed to move i386_seg_prefixes to gas.

Correct, but it is needed to fulfill the purpose of the patch as per
its title. Otherwise an unused NEEDED entry remains in gas'es .dynamic.

Jan

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

* Re: [PATCH 2/2] x86: break gas dependency on libopcodes
  2022-11-17 16:53     ` Jan Beulich
@ 2022-11-17 16:55       ` H.J. Lu
  2022-11-17 16:59         ` Jan Beulich
  0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2022-11-17 16:55 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Thu, Nov 17, 2022 at 8:53 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 17.11.2022 17:48, H.J. Lu wrote:
> > On Thu, Nov 17, 2022 at 5:29 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> --- 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)
> >>         ;;
> >
> > This change isn't needed to move i386_seg_prefixes to gas.
>
> Correct, but it is needed to fulfill the purpose of the patch as per
> its title. Otherwise an unused NEEDED entry remains in gas'es .dynamic.
>

This patch should be a standalone patch.

-- 
H.J.

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

* Re: [PATCH 2/2] x86: break gas dependency on libopcodes
  2022-11-17 16:55       ` H.J. Lu
@ 2022-11-17 16:59         ` Jan Beulich
  2022-11-17 17:01           ` H.J. Lu
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2022-11-17 16:59 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 17.11.2022 17:55, H.J. Lu wrote:
> On Thu, Nov 17, 2022 at 8:53 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 17.11.2022 17:48, H.J. Lu wrote:
>>> On Thu, Nov 17, 2022 at 5:29 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> --- 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)
>>>>         ;;
>>>
>>> This change isn't needed to move i386_seg_prefixes to gas.
>>
>> Correct, but it is needed to fulfill the purpose of the patch as per
>> its title. Otherwise an unused NEEDED entry remains in gas'es .dynamic.
> 
> This patch should be a standalone patch.

I'll split, but I'm nevertheless curious as to why: With it in a separate
patch, there'll be an intermediate bogus state - gas will require
libopcodes.so for no reason. Personally I'd consider such a (minor) bug.

Jan

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

* Re: [PATCH 2/2] x86: break gas dependency on libopcodes
  2022-11-17 16:59         ` Jan Beulich
@ 2022-11-17 17:01           ` H.J. Lu
  2022-11-17 17:07             ` H.J. Lu
  2022-11-18  6:53             ` Jan Beulich
  0 siblings, 2 replies; 10+ messages in thread
From: H.J. Lu @ 2022-11-17 17:01 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Thu, Nov 17, 2022 at 8:59 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 17.11.2022 17:55, H.J. Lu wrote:
> > On Thu, Nov 17, 2022 at 8:53 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 17.11.2022 17:48, H.J. Lu wrote:
> >>> On Thu, Nov 17, 2022 at 5:29 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> --- 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)
> >>>>         ;;
> >>>
> >>> This change isn't needed to move i386_seg_prefixes to gas.
> >>
> >> Correct, but it is needed to fulfill the purpose of the patch as per
> >> its title. Otherwise an unused NEEDED entry remains in gas'es .dynamic.
> >
> > This patch should be a standalone patch.
>
> I'll split, but I'm nevertheless curious as to why: With it in a separate

It should be the first patch, not the last.

> patch, there'll be an intermediate bogus state - gas will require
> libopcodes.so for no reason. Personally I'd consider such a (minor) bug.
>
> Jan



-- 
H.J.

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

* Re: [PATCH 2/2] x86: break gas dependency on libopcodes
  2022-11-17 17:01           ` H.J. Lu
@ 2022-11-17 17:07             ` H.J. Lu
  2022-11-18  6:53             ` Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2022-11-17 17:07 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Binutils

On Thu, Nov 17, 2022 at 9:01 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Nov 17, 2022 at 8:59 AM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 17.11.2022 17:55, H.J. Lu wrote:
> > > On Thu, Nov 17, 2022 at 8:53 AM Jan Beulich <jbeulich@suse.com> wrote:
> > >>
> > >> On 17.11.2022 17:48, H.J. Lu wrote:
> > >>> On Thu, Nov 17, 2022 at 5:29 AM Jan Beulich <jbeulich@suse.com> wrote:
> > >>>> --- 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)
> > >>>>         ;;
> > >>>
> > >>> This change isn't needed to move i386_seg_prefixes to gas.
> > >>
> > >> Correct, but it is needed to fulfill the purpose of the patch as per
> > >> its title. Otherwise an unused NEEDED entry remains in gas'es .dynamic.
> > >
> > > This patch should be a standalone patch.
> >
> > I'll split, but I'm nevertheless curious as to why: With it in a separate
>
> It should be the first patch, not the last.

I checked in a patch to move it.

> > patch, there'll be an intermediate bogus state - gas will require
> > libopcodes.so for no reason. Personally I'd consider such a (minor) bug.
> >
> > Jan
>
>
>
> --
> H.J.



-- 
H.J.

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

* Re: [PATCH 2/2] x86: break gas dependency on libopcodes
  2022-11-17 17:01           ` H.J. Lu
  2022-11-17 17:07             ` H.J. Lu
@ 2022-11-18  6:53             ` Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2022-11-18  6:53 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On 17.11.2022 18:01, H.J. Lu wrote:
> On Thu, Nov 17, 2022 at 8:59 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 17.11.2022 17:55, H.J. Lu wrote:
>>> On Thu, Nov 17, 2022 at 8:53 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 17.11.2022 17:48, H.J. Lu wrote:
>>>>> On Thu, Nov 17, 2022 at 5:29 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> --- 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)
>>>>>>         ;;
>>>>>
>>>>> This change isn't needed to move i386_seg_prefixes to gas.
>>>>
>>>> Correct, but it is needed to fulfill the purpose of the patch as per
>>>> its title. Otherwise an unused NEEDED entry remains in gas'es .dynamic.
>>>
>>> This patch should be a standalone patch.
>>
>> I'll split, but I'm nevertheless curious as to why: With it in a separate
> 
> It should be the first patch, not the last.

Of course it cannot be the last one; in my v2 series it would have been
patch 2, but now you've already committed a patch to do that move. You
don't, however, answer my question, because from my pov the configure
change now ought to become part of patch 1 (i.e. again the one removing
the last dependency). As said - I'll submit the updated series, but I
would _still_ like to have an answer to the "Why?" question. Not the
least also going forward, in case a somewhat similar change would turn
up again.

Jan

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

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

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-17 13:27 [PATCH 0/2] x86: break gas dependency on libopcodes Jan Beulich
2022-11-17 13:29 ` [PATCH 1/2] x86: instantiate i386_{op,reg}tab[] in gas instead of in libopcodes Jan Beulich
2022-11-17 13:29 ` [PATCH 2/2] x86: break gas dependency on libopcodes Jan Beulich
2022-11-17 16:48   ` H.J. Lu
2022-11-17 16:53     ` Jan Beulich
2022-11-17 16:55       ` H.J. Lu
2022-11-17 16:59         ` Jan Beulich
2022-11-17 17:01           ` H.J. Lu
2022-11-17 17:07             ` H.J. Lu
2022-11-18  6:53             ` 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).