diff mbox series

[v2] x86: Remove libopcodes dependency

Message ID CAMe9rOpbKEZ=wSgTbDLEXgzj2sbAHp1uU0WSGy0qTPL1fakM0g@mail.gmail.com
State Accepted
Headers
Series [v2] x86: Remove libopcodes dependency |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

H.J. Lu Nov. 28, 2022, 11:49 p.m. UTC 
  On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 22.11.2022 19:19, H.J. Lu wrote:
> > --- a/gas/Makefile.am
> > +++ b/gas/Makefile.am
> > @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
> >       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
> >         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
> >
> > +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
> > +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
> > +     $(srcdir)/../opcodes/i386-reg.tbl \
> > +     $(srcdir)/../opcodes/i386-opc.h
> > +     cd ../opcodes; make gen-i386-tbl
>
> I've made a patch to gas/Makefile.am as you have requested in reply to
> my series. I will want to put that through some more testing, so I will
> submit a v3 of that only a little later (and of course only unless you
> submit a v2 of your patch earlier that I would also end up being okay
> with). In the course of doing so I noticed a few more issues with your
> change:
>
> For one I don't think you can put @MAINT@ on a continued line, as the
> line continuation might then be hidden when @MAINT@ expands to #. The
> list of dependencies wants expressing via a variable, which would then
> be used immediately after @MAINT@ without any line continuation
> following.

Fixed.

> And then your rule / dependency won't be enough on a "maintainer-clean"
> tree, i.e. when the generated headers aren't there at all, and when
> config/.deps/tc-i386.Po is still empty. In that case nothing would
> trigger their generation; an explicit dependency of config/tc-i386.o on
> these headers needs adding here.

Fixed.

> Finally you're missing a dependency of the generated headers on
> i386-gen.c.

They have a dependency on i386-gen which depends on i386-gen.c.

Here is the v2 patch.

Comments

Jan Beulich Nov. 29, 2022, 9:22 a.m. UTC | #1 
On 29.11.2022 00:49, H.J. Lu wrote:
> On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 22.11.2022 19:19, H.J. Lu wrote:
>>> --- a/gas/Makefile.am
>>> +++ b/gas/Makefile.am
>>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
>>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
>>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
>>>
>>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
>>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
>>> +     $(srcdir)/../opcodes/i386-reg.tbl \
>>> +     $(srcdir)/../opcodes/i386-opc.h
>>> +     cd ../opcodes; make gen-i386-tbl
>>
>> I've made a patch to gas/Makefile.am as you have requested in reply to
>> my series. I will want to put that through some more testing, so I will
>> submit a v3 of that only a little later (and of course only unless you
>> submit a v2 of your patch earlier that I would also end up being okay
>> with). In the course of doing so I noticed a few more issues with your
>> change:
>>
>> For one I don't think you can put @MAINT@ on a continued line, as the
>> line continuation might then be hidden when @MAINT@ expands to #. The
>> list of dependencies wants expressing via a variable, which would then
>> be used immediately after @MAINT@ without any line continuation
>> following.
> 
> Fixed.

No, the same problem is still there. You either need to use a very long
line, or you need to introduce a variable holding the list of prereqs,
like I've done in my series.

>> And then your rule / dependency won't be enough on a "maintainer-clean"
>> tree, i.e. when the generated headers aren't there at all, and when
>> config/.deps/tc-i386.Po is still empty. In that case nothing would
>> trigger their generation; an explicit dependency of config/tc-i386.o on
>> these headers needs adding here.
> 
> Fixed.
> 
>> Finally you're missing a dependency of the generated headers on
>> i386-gen.c.
> 
> They have a dependency on i386-gen which depends on i386-gen.c.

In opcodes/, yes, but talk was about the rule in gas/. Yet despite
your comment above I see that you've added the missing dependency.

> Here is the v2 patch.

As said in reply to v1 - my objection to this particular use of make
recursion remains.

I also continue to be irritated by you specifically having asked me
to further split my series, when you do everything in a single patch.

One further request at least for consideration: In my v3 I've moved
the inclusion of opcodes/i386-tbl.h quite a bit further down in
tc-i386.c, as I think it's preferable that way (not introducing the
static arrays earlier than necessary).

Jan
H.J. Lu Nov. 29, 2022, 7:38 p.m. UTC | #2 
On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 29.11.2022 00:49, H.J. Lu wrote:
> > On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 22.11.2022 19:19, H.J. Lu wrote:
> >>> --- a/gas/Makefile.am
> >>> +++ b/gas/Makefile.am
> >>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
> >>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
> >>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
> >>>
> >>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
> >>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
> >>> +     $(srcdir)/../opcodes/i386-reg.tbl \
> >>> +     $(srcdir)/../opcodes/i386-opc.h
> >>> +     cd ../opcodes; make gen-i386-tbl
> >>
> >> I've made a patch to gas/Makefile.am as you have requested in reply to
> >> my series. I will want to put that through some more testing, so I will
> >> submit a v3 of that only a little later (and of course only unless you
> >> submit a v2 of your patch earlier that I would also end up being okay
> >> with). In the course of doing so I noticed a few more issues with your
> >> change:
> >>
> >> For one I don't think you can put @MAINT@ on a continued line, as the
> >> line continuation might then be hidden when @MAINT@ expands to #. The
> >> list of dependencies wants expressing via a variable, which would then
> >> be used immediately after @MAINT@ without any line continuation
> >> following.
> >
> > Fixed.
>
> No, the same problem is still there. You either need to use a very long
> line, or you need to introduce a variable holding the list of prereqs,
> like I've done in my series.

I got

$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
$(srcdir)/../opcodes/i386-opc.tbl \
        $(srcdir)/../opcodes/i386-reg.tbl \
        $(srcdir)/../opcodes/i386-opc.h \
        $(srcdir)/../opcodes/i386-gen.c
        $(MAKE) -C ../opcodes gen-i386-tbl

and

$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
$(srcdir)/../opcodes/i386-opc.tbl \
        $(srcdir)/../opcodes/i386-reg.tbl \
        $(srcdir)/../opcodes/i386-opc.h \
        $(srcdir)/../opcodes/i386-gen.c
        $(MAKE) -C ../opcodes gen-i386-tbl

I didn't run into any problems.

> >> And then your rule / dependency won't be enough on a "maintainer-clean"
> >> tree, i.e. when the generated headers aren't there at all, and when
> >> config/.deps/tc-i386.Po is still empty. In that case nothing would
> >> trigger their generation; an explicit dependency of config/tc-i386.o on
> >> these headers needs adding here.
> >
> > Fixed.
> >
> >> Finally you're missing a dependency of the generated headers on
> >> i386-gen.c.
> >
> > They have a dependency on i386-gen which depends on i386-gen.c.
>
> In opcodes/, yes, but talk was about the rule in gas/. Yet despite
> your comment above I see that you've added the missing dependency.
>
> > Here is the v2 patch.
>
> As said in reply to v1 - my objection to this particular use of make
> recursion remains.

The gen-i386-tbl rule doesn't touch any files used by libopcodes.

> I also continue to be irritated by you specifically having asked me
> to further split my series, when you do everything in a single patch.

My patch is relatively small and it does only one thing.

> One further request at least for consideration: In my v3 I've moved
> the inclusion of opcodes/i386-tbl.h quite a bit further down in
> tc-i386.c, as I think it's preferable that way (not introducing the
> static arrays earlier than necessary).

Does it make any differences?
H.J. Lu Nov. 30, 2022, 12:06 a.m. UTC | #3 
On Tue, Nov 29, 2022 at 11:38 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 29.11.2022 00:49, H.J. Lu wrote:
> > > On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
> > >> On 22.11.2022 19:19, H.J. Lu wrote:
> > >>> --- a/gas/Makefile.am
> > >>> +++ b/gas/Makefile.am
> > >>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
> > >>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
> > >>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
> > >>>
> > >>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
> > >>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
> > >>> +     $(srcdir)/../opcodes/i386-reg.tbl \
> > >>> +     $(srcdir)/../opcodes/i386-opc.h
> > >>> +     cd ../opcodes; make gen-i386-tbl
> > >>
> > >> I've made a patch to gas/Makefile.am as you have requested in reply to
> > >> my series. I will want to put that through some more testing, so I will
> > >> submit a v3 of that only a little later (and of course only unless you
> > >> submit a v2 of your patch earlier that I would also end up being okay
> > >> with). In the course of doing so I noticed a few more issues with your
> > >> change:
> > >>
> > >> For one I don't think you can put @MAINT@ on a continued line, as the
> > >> line continuation might then be hidden when @MAINT@ expands to #. The
> > >> list of dependencies wants expressing via a variable, which would then
> > >> be used immediately after @MAINT@ without any line continuation
> > >> following.
> > >
> > > Fixed.
> >
> > No, the same problem is still there. You either need to use a very long
> > line, or you need to introduce a variable holding the list of prereqs,
> > like I've done in my series.
>
> I got
>
> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
> $(srcdir)/../opcodes/i386-opc.tbl \
>         $(srcdir)/../opcodes/i386-reg.tbl \
>         $(srcdir)/../opcodes/i386-opc.h \
>         $(srcdir)/../opcodes/i386-gen.c
>         $(MAKE) -C ../opcodes gen-i386-tbl
>
> and
>
> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
> $(srcdir)/../opcodes/i386-opc.tbl \
>         $(srcdir)/../opcodes/i386-reg.tbl \
>         $(srcdir)/../opcodes/i386-opc.h \
>         $(srcdir)/../opcodes/i386-gen.c
>         $(MAKE) -C ../opcodes gen-i386-tbl
>
> I didn't run into any problems.
>
> > >> And then your rule / dependency won't be enough on a "maintainer-clean"
> > >> tree, i.e. when the generated headers aren't there at all, and when
> > >> config/.deps/tc-i386.Po is still empty. In that case nothing would
> > >> trigger their generation; an explicit dependency of config/tc-i386.o on
> > >> these headers needs adding here.
> > >
> > > Fixed.
> > >
> > >> Finally you're missing a dependency of the generated headers on
> > >> i386-gen.c.
> > >
> > > They have a dependency on i386-gen which depends on i386-gen.c.
> >
> > In opcodes/, yes, but talk was about the rule in gas/. Yet despite
> > your comment above I see that you've added the missing dependency.
> >
> > > Here is the v2 patch.
> >
> > As said in reply to v1 - my objection to this particular use of make
> > recursion remains.
>
> The gen-i386-tbl rule doesn't touch any files used by libopcodes.

There are

bfd/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
$$local_target) \
binutils/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
$$local_target) \
gas/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
$$local_target) \

> > I also continue to be irritated by you specifically having asked me
> > to further split my series, when you do everything in a single patch.
>
> My patch is relatively small and it does only one thing.
>
> > One further request at least for consideration: In my v3 I've moved
> > the inclusion of opcodes/i386-tbl.h quite a bit further down in
> > tc-i386.c, as I think it's preferable that way (not introducing the
> > static arrays earlier than necessary).
>
> Does it make any differences?
>
>
> --
> H.J.
Jan Beulich Nov. 30, 2022, 6:58 a.m. UTC | #4 
On 30.11.2022 01:06, H.J. Lu wrote:
> On Tue, Nov 29, 2022 at 11:38 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>> On 29.11.2022 00:49, H.J. Lu wrote:
>>>> On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>> On 22.11.2022 19:19, H.J. Lu wrote:
>>>>>> --- a/gas/Makefile.am
>>>>>> +++ b/gas/Makefile.am
>>>>>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
>>>>>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
>>>>>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
>>>>>>
>>>>>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
>>>>>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
>>>>>> +     $(srcdir)/../opcodes/i386-reg.tbl \
>>>>>> +     $(srcdir)/../opcodes/i386-opc.h
>>>>>> +     cd ../opcodes; make gen-i386-tbl
>>>>>
>>>>> I've made a patch to gas/Makefile.am as you have requested in reply to
>>>>> my series. I will want to put that through some more testing, so I will
>>>>> submit a v3 of that only a little later (and of course only unless you
>>>>> submit a v2 of your patch earlier that I would also end up being okay
>>>>> with). In the course of doing so I noticed a few more issues with your
>>>>> change:
>>>>>
>>>>> For one I don't think you can put @MAINT@ on a continued line, as the
>>>>> line continuation might then be hidden when @MAINT@ expands to #. The
>>>>> list of dependencies wants expressing via a variable, which would then
>>>>> be used immediately after @MAINT@ without any line continuation
>>>>> following.
>>>>
>>>> Fixed.
>>>
>>> No, the same problem is still there. You either need to use a very long
>>> line, or you need to introduce a variable holding the list of prereqs,
>>> like I've done in my series.
>>
>> I got
>>
>> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
>> $(srcdir)/../opcodes/i386-opc.tbl \
>>         $(srcdir)/../opcodes/i386-reg.tbl \
>>         $(srcdir)/../opcodes/i386-opc.h \
>>         $(srcdir)/../opcodes/i386-gen.c
>>         $(MAKE) -C ../opcodes gen-i386-tbl
>>
>> and
>>
>> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
>> $(srcdir)/../opcodes/i386-opc.tbl \
>>         $(srcdir)/../opcodes/i386-reg.tbl \
>>         $(srcdir)/../opcodes/i386-opc.h \
>>         $(srcdir)/../opcodes/i386-gen.c
>>         $(MAKE) -C ../opcodes gen-i386-tbl
>>
>> I didn't run into any problems.
>>
>>>>> And then your rule / dependency won't be enough on a "maintainer-clean"
>>>>> tree, i.e. when the generated headers aren't there at all, and when
>>>>> config/.deps/tc-i386.Po is still empty. In that case nothing would
>>>>> trigger their generation; an explicit dependency of config/tc-i386.o on
>>>>> these headers needs adding here.
>>>>
>>>> Fixed.
>>>>
>>>>> Finally you're missing a dependency of the generated headers on
>>>>> i386-gen.c.
>>>>
>>>> They have a dependency on i386-gen which depends on i386-gen.c.
>>>
>>> In opcodes/, yes, but talk was about the rule in gas/. Yet despite
>>> your comment above I see that you've added the missing dependency.
>>>
>>>> Here is the v2 patch.
>>>
>>> As said in reply to v1 - my objection to this particular use of make
>>> recursion remains.
>>
>> The gen-i386-tbl rule doesn't touch any files used by libopcodes.
> 
> There are
> 
> bfd/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
> $$local_target) \
> binutils/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
> $$local_target) \
> gas/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
> $$local_target) \

I'm glad you spotted this yourself - I was just about to point out
that my concern isn't only a theoretical one, because of the
generated makefiles we use. You don't draw any conclusion though:
Are you willing to accept my approach, specifically reworked to
account for an extra request of yours? Or are you meaning to make
another attempt yourself, avoiding the undue make recursion?

Jan
Jan Beulich Nov. 30, 2022, 7:31 a.m. UTC | #5 
On 29.11.2022 20:38, H.J. Lu wrote:
> On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 29.11.2022 00:49, H.J. Lu wrote:
>>> On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 22.11.2022 19:19, H.J. Lu wrote:
>>>>> --- a/gas/Makefile.am
>>>>> +++ b/gas/Makefile.am
>>>>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
>>>>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
>>>>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
>>>>>
>>>>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
>>>>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
>>>>> +     $(srcdir)/../opcodes/i386-reg.tbl \
>>>>> +     $(srcdir)/../opcodes/i386-opc.h
>>>>> +     cd ../opcodes; make gen-i386-tbl
>>>>
>>>> I've made a patch to gas/Makefile.am as you have requested in reply to
>>>> my series. I will want to put that through some more testing, so I will
>>>> submit a v3 of that only a little later (and of course only unless you
>>>> submit a v2 of your patch earlier that I would also end up being okay
>>>> with). In the course of doing so I noticed a few more issues with your
>>>> change:
>>>>
>>>> For one I don't think you can put @MAINT@ on a continued line, as the
>>>> line continuation might then be hidden when @MAINT@ expands to #. The
>>>> list of dependencies wants expressing via a variable, which would then
>>>> be used immediately after @MAINT@ without any line continuation
>>>> following.
>>>
>>> Fixed.
>>
>> No, the same problem is still there. You either need to use a very long
>> line, or you need to introduce a variable holding the list of prereqs,
>> like I've done in my series.
> 
> I got
> 
> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:

Note the missing line continuation here.

> $(srcdir)/../opcodes/i386-opc.tbl \
>         $(srcdir)/../opcodes/i386-reg.tbl \
>         $(srcdir)/../opcodes/i386-opc.h \
>         $(srcdir)/../opcodes/i386-gen.c
>         $(MAKE) -C ../opcodes gen-i386-tbl

I have no idea what make makes of this standalone list of filenames.
Without the line continuation these wouldn't be dependencies of the
two named generated files.

> and
> 
> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
> $(srcdir)/../opcodes/i386-opc.tbl \
>         $(srcdir)/../opcodes/i386-reg.tbl \
>         $(srcdir)/../opcodes/i386-opc.h \
>         $(srcdir)/../opcodes/i386-gen.c
>         $(MAKE) -C ../opcodes gen-i386-tbl

Again these aren't dependencies of the two named generated files.

> I didn't run into any problems.

I'm surprised.

>> One further request at least for consideration: In my v3 I've moved
>> the inclusion of opcodes/i386-tbl.h quite a bit further down in
>> tc-i386.c, as I think it's preferable that way (not introducing the
>> static arrays earlier than necessary).
> 
> Does it make any differences?

It is simply more logical to introduce definitions (not just
declarations) not among the ordinary set of #include-s. And it keeps
related things closer together.

Jan
H.J. Lu Nov. 30, 2022, 10:15 p.m. UTC | #6 
On Tue, Nov 29, 2022 at 11:31 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 29.11.2022 20:38, H.J. Lu wrote:
> > On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 29.11.2022 00:49, H.J. Lu wrote:
> >>> On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> On 22.11.2022 19:19, H.J. Lu wrote:
> >>>>> --- a/gas/Makefile.am
> >>>>> +++ b/gas/Makefile.am
> >>>>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
> >>>>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
> >>>>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
> >>>>>
> >>>>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
> >>>>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
> >>>>> +     $(srcdir)/../opcodes/i386-reg.tbl \
> >>>>> +     $(srcdir)/../opcodes/i386-opc.h
> >>>>> +     cd ../opcodes; make gen-i386-tbl
> >>>>
> >>>> I've made a patch to gas/Makefile.am as you have requested in reply to
> >>>> my series. I will want to put that through some more testing, so I will
> >>>> submit a v3 of that only a little later (and of course only unless you
> >>>> submit a v2 of your patch earlier that I would also end up being okay
> >>>> with). In the course of doing so I noticed a few more issues with your
> >>>> change:
> >>>>
> >>>> For one I don't think you can put @MAINT@ on a continued line, as the
> >>>> line continuation might then be hidden when @MAINT@ expands to #. The
> >>>> list of dependencies wants expressing via a variable, which would then
> >>>> be used immediately after @MAINT@ without any line continuation
> >>>> following.
> >>>
> >>> Fixed.
> >>
> >> No, the same problem is still there. You either need to use a very long
> >> line, or you need to introduce a variable holding the list of prereqs,
> >> like I've done in my series.
> >
> > I got
> >
> > $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
>
> Note the missing line continuation here.

They are on the same line:

$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
$(srcdir)/../opcodes/i386-opc.tbl \

or

$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
$(srcdir)/../opcodes/i386-opc.tbl \

> > $(srcdir)/../opcodes/i386-opc.tbl \
> >         $(srcdir)/../opcodes/i386-reg.tbl \
> >         $(srcdir)/../opcodes/i386-opc.h \
> >         $(srcdir)/../opcodes/i386-gen.c
> >         $(MAKE) -C ../opcodes gen-i386-tbl
>
> I have no idea what make makes of this standalone list of filenames.
> Without the line continuation these wouldn't be dependencies of the
> two named generated files.
>
> > and
> >
> > $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
> > $(srcdir)/../opcodes/i386-opc.tbl \
> >         $(srcdir)/../opcodes/i386-reg.tbl \
> >         $(srcdir)/../opcodes/i386-opc.h \
> >         $(srcdir)/../opcodes/i386-gen.c
> >         $(MAKE) -C ../opcodes gen-i386-tbl
>
> Again these aren't dependencies of the two named generated files.
>
> > I didn't run into any problems.
>
> I'm surprised.
>
> >> One further request at least for consideration: In my v3 I've moved
> >> the inclusion of opcodes/i386-tbl.h quite a bit further down in
> >> tc-i386.c, as I think it's preferable that way (not introducing the
> >> static arrays earlier than necessary).
> >
> > Does it make any differences?
>
> It is simply more logical to introduce definitions (not just
> declarations) not among the ordinary set of #include-s. And it keeps
> related things closer together.
>

I prefer keeping "#include" together.
H.J. Lu Nov. 30, 2022, 10:22 p.m. UTC | #7 
On Tue, Nov 29, 2022 at 10:58 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 30.11.2022 01:06, H.J. Lu wrote:
> > On Tue, Nov 29, 2022 at 11:38 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>
> >>> On 29.11.2022 00:49, H.J. Lu wrote:
> >>>> On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>> On 22.11.2022 19:19, H.J. Lu wrote:
> >>>>>> --- a/gas/Makefile.am
> >>>>>> +++ b/gas/Makefile.am
> >>>>>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
> >>>>>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
> >>>>>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
> >>>>>>
> >>>>>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
> >>>>>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
> >>>>>> +     $(srcdir)/../opcodes/i386-reg.tbl \
> >>>>>> +     $(srcdir)/../opcodes/i386-opc.h
> >>>>>> +     cd ../opcodes; make gen-i386-tbl
> >>>>>
> >>>>> I've made a patch to gas/Makefile.am as you have requested in reply to
> >>>>> my series. I will want to put that through some more testing, so I will
> >>>>> submit a v3 of that only a little later (and of course only unless you
> >>>>> submit a v2 of your patch earlier that I would also end up being okay
> >>>>> with). In the course of doing so I noticed a few more issues with your
> >>>>> change:
> >>>>>
> >>>>> For one I don't think you can put @MAINT@ on a continued line, as the
> >>>>> line continuation might then be hidden when @MAINT@ expands to #. The
> >>>>> list of dependencies wants expressing via a variable, which would then
> >>>>> be used immediately after @MAINT@ without any line continuation
> >>>>> following.
> >>>>
> >>>> Fixed.
> >>>
> >>> No, the same problem is still there. You either need to use a very long
> >>> line, or you need to introduce a variable holding the list of prereqs,
> >>> like I've done in my series.
> >>
> >> I got
> >>
> >> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
> >> $(srcdir)/../opcodes/i386-opc.tbl \
> >>         $(srcdir)/../opcodes/i386-reg.tbl \
> >>         $(srcdir)/../opcodes/i386-opc.h \
> >>         $(srcdir)/../opcodes/i386-gen.c
> >>         $(MAKE) -C ../opcodes gen-i386-tbl
> >>
> >> and
> >>
> >> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
> >> $(srcdir)/../opcodes/i386-opc.tbl \
> >>         $(srcdir)/../opcodes/i386-reg.tbl \
> >>         $(srcdir)/../opcodes/i386-opc.h \
> >>         $(srcdir)/../opcodes/i386-gen.c
> >>         $(MAKE) -C ../opcodes gen-i386-tbl
> >>
> >> I didn't run into any problems.
> >>
> >>>>> And then your rule / dependency won't be enough on a "maintainer-clean"
> >>>>> tree, i.e. when the generated headers aren't there at all, and when
> >>>>> config/.deps/tc-i386.Po is still empty. In that case nothing would
> >>>>> trigger their generation; an explicit dependency of config/tc-i386.o on
> >>>>> these headers needs adding here.
> >>>>
> >>>> Fixed.
> >>>>
> >>>>> Finally you're missing a dependency of the generated headers on
> >>>>> i386-gen.c.
> >>>>
> >>>> They have a dependency on i386-gen which depends on i386-gen.c.
> >>>
> >>> In opcodes/, yes, but talk was about the rule in gas/. Yet despite
> >>> your comment above I see that you've added the missing dependency.
> >>>
> >>>> Here is the v2 patch.
> >>>
> >>> As said in reply to v1 - my objection to this particular use of make
> >>> recursion remains.
> >>
> >> The gen-i386-tbl rule doesn't touch any files used by libopcodes.
> >
> > There are
> >
> > bfd/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
> > $$local_target) \
> > binutils/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
> > $$local_target) \
> > gas/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
> > $$local_target) \
>
> I'm glad you spotted this yourself - I was just about to point out
> that my concern isn't only a theoretical one, because of the
> generated makefiles we use. You don't draw any conclusion though:
> Are you willing to accept my approach, specifically reworked to
> account for an extra request of yours? Or are you meaning to make
> another attempt yourself, avoiding the undue make recursion?
>

Calling $(MAKE) in Makefile is very normal in binutils-gdb.
I don't see anything wrong with my approach.  I prefer my
approach.
Jan Beulich Dec. 1, 2022, 7:21 a.m. UTC | #8 
On 30.11.2022 23:15, H.J. Lu wrote:
> On Tue, Nov 29, 2022 at 11:31 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 29.11.2022 20:38, H.J. Lu wrote:
>>> On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>> On 29.11.2022 00:49, H.J. Lu wrote:
>>>>> On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>> On 22.11.2022 19:19, H.J. Lu wrote:
>>>>>>> --- a/gas/Makefile.am
>>>>>>> +++ b/gas/Makefile.am
>>>>>>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
>>>>>>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
>>>>>>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
>>>>>>>
>>>>>>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
>>>>>>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
>>>>>>> +     $(srcdir)/../opcodes/i386-reg.tbl \
>>>>>>> +     $(srcdir)/../opcodes/i386-opc.h
>>>>>>> +     cd ../opcodes; make gen-i386-tbl
>>>>>>
>>>>>> I've made a patch to gas/Makefile.am as you have requested in reply to
>>>>>> my series. I will want to put that through some more testing, so I will
>>>>>> submit a v3 of that only a little later (and of course only unless you
>>>>>> submit a v2 of your patch earlier that I would also end up being okay
>>>>>> with). In the course of doing so I noticed a few more issues with your
>>>>>> change:
>>>>>>
>>>>>> For one I don't think you can put @MAINT@ on a continued line, as the
>>>>>> line continuation might then be hidden when @MAINT@ expands to #. The
>>>>>> list of dependencies wants expressing via a variable, which would then
>>>>>> be used immediately after @MAINT@ without any line continuation
>>>>>> following.
>>>>>
>>>>> Fixed.
>>>>
>>>> No, the same problem is still there. You either need to use a very long
>>>> line, or you need to introduce a variable holding the list of prereqs,
>>>> like I've done in my series.
>>>
>>> I got
>>>
>>> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
>>
>> Note the missing line continuation here.
> 
> They are on the same line:
> 
> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
> $(srcdir)/../opcodes/i386-opc.tbl \

Oh, okay - just a UI presentation issue then here.

> or
> 
> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
> $(srcdir)/../opcodes/i386-opc.tbl \

Here, though, this is an issue, because the line continuation then is
past the comment character.

Jan
Jan Beulich Dec. 1, 2022, 7:41 a.m. UTC | #9 
On 30.11.2022 23:22, H.J. Lu wrote:
> On Tue, Nov 29, 2022 at 10:58 PM Jan Beulich <jbeulich@suse.com> wrote:
>> On 30.11.2022 01:06, H.J. Lu wrote:
>>> On Tue, Nov 29, 2022 at 11:38 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>> On 29.11.2022 00:49, H.J. Lu wrote:
>>>>>> On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>> On 22.11.2022 19:19, H.J. Lu wrote:
>>>>>>>> --- a/gas/Makefile.am
>>>>>>>> +++ b/gas/Makefile.am
>>>>>>>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
>>>>>>>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
>>>>>>>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
>>>>>>>>
>>>>>>>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
>>>>>>>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
>>>>>>>> +     $(srcdir)/../opcodes/i386-reg.tbl \
>>>>>>>> +     $(srcdir)/../opcodes/i386-opc.h
>>>>>>>> +     cd ../opcodes; make gen-i386-tbl
>>>>>>>
>>>>>>> I've made a patch to gas/Makefile.am as you have requested in reply to
>>>>>>> my series. I will want to put that through some more testing, so I will
>>>>>>> submit a v3 of that only a little later (and of course only unless you
>>>>>>> submit a v2 of your patch earlier that I would also end up being okay
>>>>>>> with). In the course of doing so I noticed a few more issues with your
>>>>>>> change:
>>>>>>>
>>>>>>> For one I don't think you can put @MAINT@ on a continued line, as the
>>>>>>> line continuation might then be hidden when @MAINT@ expands to #. The
>>>>>>> list of dependencies wants expressing via a variable, which would then
>>>>>>> be used immediately after @MAINT@ without any line continuation
>>>>>>> following.
>>>>>>
>>>>>> Fixed.
>>>>>
>>>>> No, the same problem is still there. You either need to use a very long
>>>>> line, or you need to introduce a variable holding the list of prereqs,
>>>>> like I've done in my series.
>>>>
>>>> I got
>>>>
>>>> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
>>>> $(srcdir)/../opcodes/i386-opc.tbl \
>>>>         $(srcdir)/../opcodes/i386-reg.tbl \
>>>>         $(srcdir)/../opcodes/i386-opc.h \
>>>>         $(srcdir)/../opcodes/i386-gen.c
>>>>         $(MAKE) -C ../opcodes gen-i386-tbl
>>>>
>>>> and
>>>>
>>>> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
>>>> $(srcdir)/../opcodes/i386-opc.tbl \
>>>>         $(srcdir)/../opcodes/i386-reg.tbl \
>>>>         $(srcdir)/../opcodes/i386-opc.h \
>>>>         $(srcdir)/../opcodes/i386-gen.c
>>>>         $(MAKE) -C ../opcodes gen-i386-tbl
>>>>
>>>> I didn't run into any problems.
>>>>
>>>>>>> And then your rule / dependency won't be enough on a "maintainer-clean"
>>>>>>> tree, i.e. when the generated headers aren't there at all, and when
>>>>>>> config/.deps/tc-i386.Po is still empty. In that case nothing would
>>>>>>> trigger their generation; an explicit dependency of config/tc-i386.o on
>>>>>>> these headers needs adding here.
>>>>>>
>>>>>> Fixed.
>>>>>>
>>>>>>> Finally you're missing a dependency of the generated headers on
>>>>>>> i386-gen.c.
>>>>>>
>>>>>> They have a dependency on i386-gen which depends on i386-gen.c.
>>>>>
>>>>> In opcodes/, yes, but talk was about the rule in gas/. Yet despite
>>>>> your comment above I see that you've added the missing dependency.
>>>>>
>>>>>> Here is the v2 patch.
>>>>>
>>>>> As said in reply to v1 - my objection to this particular use of make
>>>>> recursion remains.
>>>>
>>>> The gen-i386-tbl rule doesn't touch any files used by libopcodes.
>>>
>>> There are
>>>
>>> bfd/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
>>> $$local_target) \
>>> binutils/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
>>> $$local_target) \
>>> gas/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
>>> $$local_target) \
>>
>> I'm glad you spotted this yourself - I was just about to point out
>> that my concern isn't only a theoretical one, because of the
>> generated makefiles we use. You don't draw any conclusion though:
>> Are you willing to accept my approach, specifically reworked to
>> account for an extra request of yours? Or are you meaning to make
>> another attempt yourself, avoiding the undue make recursion?
>>
> 
> Calling $(MAKE) in Makefile is very normal in binutils-gdb.

Any further examples where this isn't done strictly top-down, or in
a properly controlled way within a single directory? I can indeed
spot many instances of $(MAKE), but none matching the pattern you
intend to introduce.

> I don't see anything wrong with my approach.  I prefer my
> approach.

I understand you prefer it, but given you've found what's wrong
yourself, I'm puzzled by "I don't see anything wrong with my
approach". Or wait - looks like I misread what you pointed out
above: That looks to be something from the top level Makefile.
Yet then again I can't spot any such in my build trees. Where is
that excerpt from? (I can spot somewhat similar patterns, but
they're used strictly to recurse _down_, just like looks to be
the case with what you've quoted.)

In any event, a practical manifestation of the issue I've said
I'm concerned of is this: If a 2nd party anywhere in the tree did
the same you do, and if the two $(MAKE) invocations then raced
with one another, then it's plain undefined what would happen to
the recursed-into Makefile when that needs re-generating for
whatever reason. Hence $(MAKE) recursion should only occur
strictly top-down, with (suitably controlled) recursion _within_
a directory possibly being okay (would need proving for every
individual instance).

Jan
H.J. Lu Dec. 1, 2022, 6:20 p.m. UTC | #10 
On Wed, Nov 30, 2022 at 11:21 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 30.11.2022 23:15, H.J. Lu wrote:
> > On Tue, Nov 29, 2022 at 11:31 PM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 29.11.2022 20:38, H.J. Lu wrote:
> >>> On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> On 29.11.2022 00:49, H.J. Lu wrote:
> >>>>> On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>> On 22.11.2022 19:19, H.J. Lu wrote:
> >>>>>>> --- a/gas/Makefile.am
> >>>>>>> +++ b/gas/Makefile.am
> >>>>>>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
> >>>>>>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
> >>>>>>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
> >>>>>>>
> >>>>>>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
> >>>>>>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
> >>>>>>> +     $(srcdir)/../opcodes/i386-reg.tbl \
> >>>>>>> +     $(srcdir)/../opcodes/i386-opc.h
> >>>>>>> +     cd ../opcodes; make gen-i386-tbl
> >>>>>>
> >>>>>> I've made a patch to gas/Makefile.am as you have requested in reply to
> >>>>>> my series. I will want to put that through some more testing, so I will
> >>>>>> submit a v3 of that only a little later (and of course only unless you
> >>>>>> submit a v2 of your patch earlier that I would also end up being okay
> >>>>>> with). In the course of doing so I noticed a few more issues with your
> >>>>>> change:
> >>>>>>
> >>>>>> For one I don't think you can put @MAINT@ on a continued line, as the
> >>>>>> line continuation might then be hidden when @MAINT@ expands to #. The
> >>>>>> list of dependencies wants expressing via a variable, which would then
> >>>>>> be used immediately after @MAINT@ without any line continuation
> >>>>>> following.
> >>>>>
> >>>>> Fixed.
> >>>>
> >>>> No, the same problem is still there. You either need to use a very long
> >>>> line, or you need to introduce a variable holding the list of prereqs,
> >>>> like I've done in my series.
> >>>
> >>> I got
> >>>
> >>> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
> >>
> >> Note the missing line continuation here.
> >
> > They are on the same line:
> >
> > $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
> > $(srcdir)/../opcodes/i386-opc.tbl \
>
> Oh, okay - just a UI presentation issue then here.
>
> > or
> >
> > $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
> > $(srcdir)/../opcodes/i386-opc.tbl \
>
> Here, though, this is an issue, because the line continuation then is
> past the comment character.
>
> Jan

It looks like

# foo \
bar

2 lines are treated as a single line.
Jan Beulich Dec. 2, 2022, 7:10 a.m. UTC | #11 
On 01.12.2022 19:20, H.J. Lu wrote:
> It looks like
> 
> # foo \
> bar
> 
> 2 lines are treated as a single line.

Hmm, I didn't think this was the case on all make versions supported
(iirc there's not even a minimum version stated anywhere, which quite
possibly is a mistake), but looking at even the oldest numbered
version in their repo (3.70.2) this has been documented behavior
already then. I'm puzzled in particular because quite recently I saw
a change specifically dealing with the supposed behavior (which I
didn't question because it was matching my recollection). I'm sorry
for the noise here.

Jan
diff mbox series

Patch

From 1eb648370f8d397ef31180d6f6777a96d81f3238 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 22 Nov 2022 10:04:20 -0800
Subject: [PATCH v2] x86: Remove libopcodes dependency

As Jan Beulich <jbeulich@suse.com> noticed, since i386-init.h and
i386-tbl.h are only used by tc-i386.c, we can remove libopcodes
dependency by including opcodes/i386-tbl.h directly.

gas/

	* Makefile.am ($(srcdir)/../opcodes/i386-init.h
	$(srcdir)/../opcodes/i386-tbl.h): New rule.
	* configure.ac (need_opcodes): Don't set for i386.
	* config/tc-i386.c: Include "opcodes/i386-tbl.h".
	* Makefile.in: Regenerated.
	* configure: Likewise.

opcodes/

	* Makefile.am (TARGET32_LIBOPCODES_CFILES): Remove i386-opc.c.
	(i386-opc.lo): Removed.
	(gen-i386-tbl): New rule.
	* configure.ac: Remove i386-opc.lo.
	* i386-opc.c: Removed.
	* i386-opc.h (insn_template): Change name to const char *.
	(i386_regtab): Removed.
	(i386_regtab_size): Likewise.
	* Makefile.in: Regenerated.
	* configure: Likewise.
	* po/POTFILES.in: Likewise.
---
 gas/Makefile.am        |  9 +++++++++
 gas/Makefile.in        |  9 +++++++++
 gas/config/tc-i386.c   |  1 +
 gas/configure          |  2 +-
 gas/configure.ac       |  2 +-
 opcodes/Makefile.am    |  6 +-----
 opcodes/Makefile.in    |  7 +------
 opcodes/configure      |  2 +-
 opcodes/configure.ac   |  2 +-
 opcodes/i386-opc.c     | 24 ------------------------
 opcodes/i386-opc.h     |  5 +----
 opcodes/po/POTFILES.in |  1 -
 12 files changed, 26 insertions(+), 44 deletions(-)
 delete mode 100644 opcodes/i386-opc.c

diff --git a/gas/Makefile.am b/gas/Makefile.am
index dc5931252e4..37d12966623 100644
--- a/gas/Makefile.am
+++ b/gas/Makefile.am
@@ -448,6 +448,15 @@  development.exp: $(BFDDIR)/development.sh
 	$(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
 	  | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
 
+config/tc-i386.@OBJEXT@: $(srcdir)/../opcodes/i386-init.h \
+	$(srcdir)/../opcodes/i386-tbl.h
+
+$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
+	$(srcdir)/../opcodes/i386-reg.tbl \
+	$(srcdir)/../opcodes/i386-opc.h \
+	$(srcdir)/../opcodes/i386-gen.c
+	$(MAKE) -C ../opcodes gen-i386-tbl
+
 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)
diff --git a/gas/Makefile.in b/gas/Makefile.in
index 8324cbe999d..3b6ae1404d9 100644
--- a/gas/Makefile.in
+++ b/gas/Makefile.in
@@ -2064,6 +2064,15 @@  check-DEJAGNU: site.exp
 development.exp: $(BFDDIR)/development.sh
 	$(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
 	  | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
+
+config/tc-i386.@OBJEXT@: $(srcdir)/../opcodes/i386-init.h \
+	$(srcdir)/../opcodes/i386-tbl.h
+
+$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
+	$(srcdir)/../opcodes/i386-reg.tbl \
+	$(srcdir)/../opcodes/i386-opc.h \
+	$(srcdir)/../opcodes/i386-gen.c
+	$(MAKE) -C ../opcodes gen-i386-tbl
 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
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 5a88d25a9c2..73277dacca3 100644
--- 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
diff --git a/gas/configure b/gas/configure
index 57c1fa3557e..f329b905d37 100755
--- a/gas/configure
+++ b/gas/configure
@@ -12263,7 +12263,7 @@  _ACEOF
 
     # Do we need the opcodes library?
     case ${cpu_type} in
-      vax | tic30)
+      i386 | vax | tic30)
 	;;
 
       *)
diff --git a/gas/configure.ac b/gas/configure.ac
index feb43399ce8..7f165c9fe64 100644
--- 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)
+      i386 | vax | tic30)
 	;;
 
       *)
diff --git a/opcodes/Makefile.am b/opcodes/Makefile.am
index 93e9002be19..b227427672f 100644
--- 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,7 @@  $(srcdir)/i386%tbl.h $(srcdir)/i386%init.h: @MAINT@ i386-gen$(EXEEXT_FOR_BUILD)
 		< $(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
+gen-i386-tbl: $(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)
diff --git a/opcodes/Makefile.in b/opcodes/Makefile.in
index fe4539d6097..8bfa2118f58 100644
--- 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,7 @@  $(srcdir)/i386%tbl.h $(srcdir)/i386%init.h: @MAINT@ i386-gen$(EXEEXT_FOR_BUILD)
 		< $(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
+gen-i386-tbl: $(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)
diff --git a/opcodes/configure b/opcodes/configure
index 08c57a33855..9dc87d6c19c 100755
--- 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 ;;
diff --git a/opcodes/configure.ac b/opcodes/configure.ac
index e998d613436..8c79deb9ffc 100644
--- 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 ;;
diff --git a/opcodes/i386-opc.c b/opcodes/i386-opc.c
deleted file mode 100644
index 729c22932b1..00000000000
--- a/opcodes/i386-opc.c
+++ /dev/null
@@ -1,24 +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"
-#include "i386-tbl.h"
diff --git a/opcodes/i386-opc.h b/opcodes/i386-opc.h
index 459268f3656..51256dcfc0e 100644
--- 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
@@ -1011,6 +1011,3 @@  typedef struct
 #define Dw2Inval (-1)
 }
 reg_entry;
-
-extern const reg_entry i386_regtab[];
-extern const unsigned int i386_regtab_size;
diff --git a/opcodes/po/POTFILES.in b/opcodes/po/POTFILES.in
index 24f57af24a3..421f67c0791 100644
--- a/opcodes/po/POTFILES.in
+++ b/opcodes/po/POTFILES.in
@@ -74,7 +74,6 @@  hppa-dis.c
 i386-dis.c
 i386-gen.c
 i386-init.h
-i386-opc.c
 i386-opc.h
 i386-tbl.h
 ia64-asmtab.c
-- 
2.38.1