[2/2] x86: break gas dependency on libopcodes

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

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];
>
  

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
  

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.
  

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
  

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
  

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.
  

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