* [PATCH] x86: Remove libopcodes dependency
@ 2022-11-22 18:19 H.J. Lu
2022-11-23 8:36 ` Jan Beulich
2022-11-24 10:19 ` Jan Beulich
0 siblings, 2 replies; 21+ messages in thread
From: H.J. Lu @ 2022-11-22 18:19 UTC (permalink / raw)
To: binutils; +Cc: Jan Beulich
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 | 6 ++++++
gas/Makefile.in | 6 ++++++
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, 20 insertions(+), 44 deletions(-)
delete mode 100644 opcodes/i386-opc.c
diff --git a/gas/Makefile.am b/gas/Makefile.am
index 4661a718214..4deff602991 100644
--- 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
+
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 1e01f343f32..29c12852572 100644
--- a/gas/Makefile.in
+++ b/gas/Makefile.in
@@ -2060,6 +2060,12 @@ check-DEJAGNU: site.exp
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
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 f54e7fddee0..39e56d9eac7 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 d9800f8a53d..0a5d0ce4e18 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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86: Remove libopcodes dependency
2022-11-22 18:19 [PATCH] x86: Remove libopcodes dependency H.J. Lu
@ 2022-11-23 8:36 ` Jan Beulich
2022-11-28 23:43 ` H.J. Lu
2022-11-24 10:19 ` Jan Beulich
1 sibling, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2022-11-23 8:36 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
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
This recursing into a different directory (and then even using "cd" and
"make" instead of "$(MAKE) -C") is what I have specifically avoided in
my patches. This is deemed an anti-pattern by many people: If you
consider running make in just gas/ is an okay thing to do, then running
make in just opcodes/ is, too. Yet with such a rule doing so in parallel
can result in strange collisions and likely partially broken files.
Therefore with my general maintainer hat on I object to such an approach.
If you really want to generate the files from gas/, then you should do
so there, i.e. also going as far as building i386-gen there. Once
again I did consider doing to, but deemed it awkward: Even if we don't
use libopcodes.{a,so} anymore, I think the opcode table processing
would better remain in opcodes/ - we'd use that library no longer as
a binary but as a (generated) source code one. If you think differently,
I wouldn't object to you following this alternative approach.
As a formal remark: In the description I would expect to be credited at
least for recognizing the opportunity; really you've re-used some of
what I've had in my patches, irrespective of you perhaps having done
things from scratch (and having spotted/corrected an oversight of mine,
which I was about to submit v3 of my series for, but which now I will
wait with until the above is settled - sadly meaning yet further delays
for the growing pile of other work I have pending on top).
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86: Remove libopcodes dependency
2022-11-22 18:19 [PATCH] x86: Remove libopcodes dependency H.J. Lu
2022-11-23 8:36 ` Jan Beulich
@ 2022-11-24 10:19 ` Jan Beulich
2022-11-28 23:49 ` [PATCH v2] " H.J. Lu
1 sibling, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2022-11-24 10:19 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
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.
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.
Finally you're missing a dependency of the generated headers on
i386-gen.c.
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86: Remove libopcodes dependency
2022-11-23 8:36 ` Jan Beulich
@ 2022-11-28 23:43 ` H.J. Lu
2022-11-29 9:12 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2022-11-28 23:43 UTC (permalink / raw)
To: Jan Beulich; +Cc: binutils
On Wed, Nov 23, 2022 at 12:36 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
>
> This recursing into a different directory (and then even using "cd" and
> "make" instead of "$(MAKE) -C") is what I have specifically avoided in
> my patches. This is deemed an anti-pattern by many people: If you
> consider running make in just gas/ is an okay thing to do, then running
> make in just opcodes/ is, too. Yet with such a rule doing so in parallel
> can result in strange collisions and likely partially broken files.
"make" in opcodes won't regenerate these header files. As far as make
dependency is concerned,
$(MAKE) -C ../opcodes gen-i386-tbl
is like other programs.
> Therefore with my general maintainer hat on I object to such an approach.
>
> If you really want to generate the files from gas/, then you should do
> so there, i.e. also going as far as building i386-gen there. Once
> again I did consider doing to, but deemed it awkward: Even if we don't
> use libopcodes.{a,so} anymore, I think the opcode table processing
> would better remain in opcodes/ - we'd use that library no longer as
> a binary but as a (generated) source code one. If you think differently,
> I wouldn't object to you following this alternative approach.
>
> As a formal remark: In the description I would expect to be credited at
> least for recognizing the opportunity; really you've re-used some of
Will do.
> what I've had in my patches, irrespective of you perhaps having done
> things from scratch (and having spotted/corrected an oversight of mine,
> which I was about to submit v3 of my series for, but which now I will
> wait with until the above is settled - sadly meaning yet further delays
> for the growing pile of other work I have pending on top).
>
--
H.J.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2] x86: Remove libopcodes dependency
2022-11-24 10:19 ` Jan Beulich
@ 2022-11-28 23:49 ` H.J. Lu
2022-11-29 9:22 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2022-11-28 23:49 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]
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.
--
H.J.
[-- Attachment #2: v2-0001-x86-Remove-libopcodes-dependency.patch --]
[-- Type: text/x-patch, Size: 9911 bytes --]
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86: Remove libopcodes dependency
2022-11-28 23:43 ` H.J. Lu
@ 2022-11-29 9:12 ` Jan Beulich
0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2022-11-29 9:12 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On 29.11.2022 00:43, H.J. Lu wrote:
> On Wed, Nov 23, 2022 at 12:36 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
>>
>> This recursing into a different directory (and then even using "cd" and
>> "make" instead of "$(MAKE) -C") is what I have specifically avoided in
>> my patches. This is deemed an anti-pattern by many people: If you
>> consider running make in just gas/ is an okay thing to do, then running
>> make in just opcodes/ is, too. Yet with such a rule doing so in parallel
>> can result in strange collisions and likely partially broken files.
>
> "make" in opcodes won't regenerate these header files. As far as make
> dependency is concerned,
>
> $(MAKE) -C ../opcodes gen-i386-tbl
>
> is like other programs.
No, it is not. There may be auxiliary files which get touched as a side
effect. There may also be some other entity (down the road) wanting to
do something similar, which may end up conflicting. Plus you shouldn't
set a bad precedent. My objection remains. I'm actually puzzled that
you've now submitted a v2 of your patch _after_ having made me (by
remaining silent there) _once again_ waste time on submitting a new
version of a series you (apparently) mean to reject in the end. I don't
mind you wanting things done differently, but then please say so. I
did specifically wait a few days to give you time to reply one way or
another.
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-11-28 23:49 ` [PATCH v2] " H.J. Lu
@ 2022-11-29 9:22 ` Jan Beulich
2022-11-29 19:38 ` H.J. Lu
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2022-11-29 9:22 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-11-29 9:22 ` Jan Beulich
@ 2022-11-29 19:38 ` H.J. Lu
2022-11-30 0:06 ` H.J. Lu
2022-11-30 7:31 ` [PATCH v2] " Jan Beulich
0 siblings, 2 replies; 21+ messages in thread
From: H.J. Lu @ 2022-11-29 19:38 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-11-29 19:38 ` H.J. Lu
@ 2022-11-30 0:06 ` H.J. Lu
2022-11-30 6:58 ` Jan Beulich
2022-11-30 7:31 ` [PATCH v2] " Jan Beulich
1 sibling, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2022-11-30 0:06 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
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.
--
H.J.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-11-30 0:06 ` H.J. Lu
@ 2022-11-30 6:58 ` Jan Beulich
2022-11-30 22:22 ` H.J. Lu
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2022-11-30 6:58 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-11-29 19:38 ` H.J. Lu
2022-11-30 0:06 ` H.J. Lu
@ 2022-11-30 7:31 ` Jan Beulich
2022-11-30 22:15 ` H.J. Lu
1 sibling, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2022-11-30 7:31 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-11-30 7:31 ` [PATCH v2] " Jan Beulich
@ 2022-11-30 22:15 ` H.J. Lu
2022-12-01 7:21 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2022-11-30 22:15 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-11-30 6:58 ` Jan Beulich
@ 2022-11-30 22:22 ` H.J. Lu
2022-12-01 7:41 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2022-11-30 22:22 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
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.
--
H.J.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-11-30 22:15 ` H.J. Lu
@ 2022-12-01 7:21 ` Jan Beulich
2022-12-01 18:20 ` H.J. Lu
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2022-12-01 7:21 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-11-30 22:22 ` H.J. Lu
@ 2022-12-01 7:41 ` Jan Beulich
2022-12-01 18:26 ` [PATCH v3] " H.J. Lu
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2022-12-01 7:41 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-12-01 7:21 ` Jan Beulich
@ 2022-12-01 18:20 ` H.J. Lu
2022-12-02 7:10 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2022-12-01 18:20 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
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.
--
H.J.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3] x86: Remove libopcodes dependency
2022-12-01 7:41 ` Jan Beulich
@ 2022-12-01 18:26 ` H.J. Lu
2022-12-02 7:16 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2022-12-01 18:26 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
On Thu, Dec 01, 2022 at 08:41:28AM +0100, Jan Beulich wrote:
> 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
Fixed.
> 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.
---
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.
($(srcdir)/i386-tbl.h): New rule.
($(srcdir)/i386-init.h): Likewise.
(gen-i386-tbl): Likewise.
* 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 | 12 ++++++++++++
gas/Makefile.in | 12 ++++++++++++
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, 32 insertions(+), 44 deletions(-)
delete mode 100644 opcodes/i386-opc.c
diff --git a/gas/Makefile.am b/gas/Makefile.am
index dc5931252e4..bf9c7a4eb7e 100644
--- a/gas/Makefile.am
+++ b/gas/Makefile.am
@@ -448,6 +448,18 @@ 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
+
+# i386-gen will generate both headers in one go. Use a pattern rule to
+# properly express this, with the inner dash ('-') arbitrarily chosen to
+# be the stem.
+$(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..349e7614039 100644
--- a/gas/Makefile.in
+++ b/gas/Makefile.in
@@ -2064,6 +2064,18 @@ 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
+
+# i386-gen will generate both headers in one go. Use a pattern rule to
+# properly express this, with the inner dash ('-') arbitrarily chosen to
+# be the stem.
+$(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 55e2d85d04b..0211d8817fd 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 cb6ff0ebc5e..517e0eb1cfd 100644
--- a/opcodes/Makefile.am
+++ b/opcodes/Makefile.am
@@ -160,7 +160,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 \
@@ -560,10 +559,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 5524af5ccd2..9be2d279dfc 100644
--- a/opcodes/Makefile.in
+++ b/opcodes/Makefile.in
@@ -552,7 +552,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 \
@@ -945,7 +944,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@
@@ -1535,10 +1533,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 215a72d18e1..8df7645d638 100644
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -918,7 +918,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
@@ -1005,6 +1005,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 9906bf14848..effd73f8f78 100644
--- a/opcodes/po/POTFILES.in
+++ b/opcodes/po/POTFILES.in
@@ -73,7 +73,6 @@ h8300-dis.c
hppa-dis.c
i386-dis.c
i386-gen.c
-i386-opc.c
i386-opc.h
ia64-asmtab.c
ia64-asmtab.h
--
2.38.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] x86: Remove libopcodes dependency
2022-12-01 18:20 ` H.J. Lu
@ 2022-12-02 7:10 ` Jan Beulich
0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2022-12-02 7:10 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] x86: Remove libopcodes dependency
2022-12-01 18:26 ` [PATCH v3] " H.J. Lu
@ 2022-12-02 7:16 ` Jan Beulich
2022-12-02 17:19 ` H.J. Lu
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2022-12-02 7:16 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On 01.12.2022 19:26, H.J. Lu wrote:
> On Thu, Dec 01, 2022 at 08:41:28AM +0100, Jan Beulich wrote:
>> 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
>
> Fixed.
In which way? You still ...
> --- a/gas/Makefile.am
> +++ b/gas/Makefile.am
> @@ -448,6 +448,18 @@ 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
> +
> +# i386-gen will generate both headers in one go. Use a pattern rule to
> +# properly express this, with the inner dash ('-') arbitrarily chosen to
> +# be the stem.
> +$(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
... wrongly recurse from gas/ into opcodes/. In fact I can't spot any
change in regard to the cross-subdir operation that I continue to
object to. I'm afraid I'm unaware of ways to address this other than
by avoiding the recursive $(MAKE) invocation altogether.
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] x86: Remove libopcodes dependency
2022-12-02 7:16 ` Jan Beulich
@ 2022-12-02 17:19 ` H.J. Lu
2022-12-05 11:25 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: H.J. Lu @ 2022-12-02 17:19 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
On Thu, Dec 1, 2022 at 11:16 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 01.12.2022 19:26, H.J. Lu wrote:
> > On Thu, Dec 01, 2022 at 08:41:28AM +0100, Jan Beulich wrote:
> >> 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
> >
> > Fixed.
>
> In which way? You still ...
>
> > --- a/gas/Makefile.am
> > +++ b/gas/Makefile.am
> > @@ -448,6 +448,18 @@ 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
> > +
> > +# i386-gen will generate both headers in one go. Use a pattern rule to
> > +# properly express this, with the inner dash ('-') arbitrarily chosen to
> > +# be the stem.
> > +$(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
>
> ... wrongly recurse from gas/ into opcodes/. In fact I can't spot any
> change in regard to the cross-subdir operation that I continue to
> object to. I'm afraid I'm unaware of ways to address this other than
> by avoiding the recursive $(MAKE) invocation altogether.
$(srcdir)/../opcodes/i386%init.h $(srcdir)/../opcodes/i386%tbl.h
will invoke $(MAKE) only once.
--
H.J.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] x86: Remove libopcodes dependency
2022-12-02 17:19 ` H.J. Lu
@ 2022-12-05 11:25 ` Jan Beulich
0 siblings, 0 replies; 21+ messages in thread
From: Jan Beulich @ 2022-12-05 11:25 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On 02.12.2022 18:19, H.J. Lu wrote:
> On Thu, Dec 1, 2022 at 11:16 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 01.12.2022 19:26, H.J. Lu wrote:
>>> On Thu, Dec 01, 2022 at 08:41:28AM +0100, Jan Beulich wrote:
>>>> 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
>>>
>>> Fixed.
>>
>> In which way? You still ...
>>
>>> --- a/gas/Makefile.am
>>> +++ b/gas/Makefile.am
>>> @@ -448,6 +448,18 @@ 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
>>> +
>>> +# i386-gen will generate both headers in one go. Use a pattern rule to
>>> +# properly express this, with the inner dash ('-') arbitrarily chosen to
>>> +# be the stem.
>>> +$(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
>>
>> ... wrongly recurse from gas/ into opcodes/. In fact I can't spot any
>> change in regard to the cross-subdir operation that I continue to
>> object to. I'm afraid I'm unaware of ways to address this other than
>> by avoiding the recursive $(MAKE) invocation altogether.
>
> $(srcdir)/../opcodes/i386%init.h $(srcdir)/../opcodes/i386%tbl.h
> will invoke $(MAKE) only once.
But that was a separate problem I was complaining about. This does in no
way address the issue of inappropriate make recursion.
Unless I see a technically correct version from you by the end of this
week, I will commit my variant (series) early next week. It is pretty
unacceptable for you to block my (also further) work by insisting on the
use of a broken alternative patch. I've clearly indicated that I'm okay
with any solution that's better than mine, but such a solution has to be
free of (pretty obvious) flaws.
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-12-05 11:25 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-22 18:19 [PATCH] x86: Remove libopcodes dependency H.J. Lu
2022-11-23 8:36 ` Jan Beulich
2022-11-28 23:43 ` H.J. Lu
2022-11-29 9:12 ` Jan Beulich
2022-11-24 10:19 ` Jan Beulich
2022-11-28 23:49 ` [PATCH v2] " H.J. Lu
2022-11-29 9:22 ` Jan Beulich
2022-11-29 19:38 ` H.J. Lu
2022-11-30 0:06 ` H.J. Lu
2022-11-30 6:58 ` Jan Beulich
2022-11-30 22:22 ` H.J. Lu
2022-12-01 7:41 ` Jan Beulich
2022-12-01 18:26 ` [PATCH v3] " H.J. Lu
2022-12-02 7:16 ` Jan Beulich
2022-12-02 17:19 ` H.J. Lu
2022-12-05 11:25 ` Jan Beulich
2022-11-30 7:31 ` [PATCH v2] " Jan Beulich
2022-11-30 22:15 ` H.J. Lu
2022-12-01 7:21 ` Jan Beulich
2022-12-01 18:20 ` H.J. Lu
2022-12-02 7:10 ` 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).