From: Mark Wielaard <mark@klomp.org>
To: Yonggang Luo <luoyonggang@gmail.com>, elfutils-devel@sourceware.org
Subject: Re: [PATCH v2 05/16] libcpu: Remove the need of NMNES by using enum
Date: Thu, 23 Feb 2023 12:50:53 +0100 [thread overview]
Message-ID: <fa21ee76f41a49be901a76b68fde8d4b6f2de1ca.camel@klomp.org> (raw)
In-Reply-To: <20b6e6b3798e69e410d16f9764f5573ce1dfb154.camel@klomp.org>
[-- Attachment #1: Type: text/plain, Size: 4895 bytes --]
Hi,
On Wed, 2022-12-21 at 18:56 +0100, Mark Wielaard wrote:
> On Sun, 2022-12-18 at 00:52 +0800, Yonggang Luo via Elfutils-devel
> wrote:
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > ---
> > libcpu/Makefile.am | 2 +-
> > libcpu/i386_disasm.c | 14 +-------------
> > libcpu/i386_mne.h | 36 ++++++++++++++++++++++++++++++++++++
> > libcpu/i386_parse.y | 9 +++------
> > 4 files changed, 41 insertions(+), 20 deletions(-)
> > create mode 100644 libcpu/i386_mne.h
> >
> > diff --git a/libcpu/Makefile.am b/libcpu/Makefile.am
> > index 57d0a164..259ed838 100644
> > --- a/libcpu/Makefile.am
> > +++ b/libcpu/Makefile.am
> > @@ -92,7 +92,7 @@ libeu = ../lib/libeu.a
> > i386_lex_CFLAGS = -Wno-unused-label -Wno-unused-function -Wno-sign-
> > compare \
> > -Wno-implicit-fallthrough
> > i386_parse.o: i386_parse.c i386.mnemonics
> > -i386_parse_CFLAGS = -DNMNES="`wc -l < i386.mnemonics`"
> > +i386_parse_CFLAGS =
> > i386_lex.o: i386_parse.h
> > i386_gendis_LDADD = $(libeu) -lm $(obstack_LIBS)
>
> The new i386_mne.h file should be added to noinst_HEADERS (or it won't
> be included in a make dist, so make distcheck fails).
>
I added this. And did a make distcheck to check things work as
intended.
> > diff --git a/libcpu/i386_disasm.c b/libcpu/i386_disasm.c
> > index 599d1654..c34f03d6 100644
> > --- a/libcpu/i386_disasm.c
> > +++ b/libcpu/i386_disasm.c
> > @@ -46,10 +46,7 @@
> > #define MACHINE_ENCODING LITTLE_ENDIAN
> > #include "memory-access.h"
> >
> > -
> > -#ifndef MNEFILE
> > -# define MNEFILE "i386.mnemonics"
> > -#endif
> > +#include "i386_mne.h"
> >
> > #define MNESTRFIELD(line) MNESTRFIELD1 (line)
> > #define MNESTRFIELD1(line) str##line
> > @@ -71,15 +68,6 @@ static const union mnestr_t
> > }
> > };
> >
> > -/* The index can be stored in the instrtab. */
> > -enum
> > - {
> > -#define MNE(name) MNE_##name,
> > -#include MNEFILE
> > -#undef MNE
> > - MNE_INVALID
> > - };
> > -
> > static const unsigned short int mneidx[] =
> > {
> > #define MNE(name) \
>
> OK.
>
> > diff --git a/libcpu/i386_mne.h b/libcpu/i386_mne.h
> > new file mode 100644
> > index 00000000..41dacf61
> > --- /dev/null
> > +++ b/libcpu/i386_mne.h
> > @@ -0,0 +1,36 @@
> > +/* Compute hash value for given string according to ELF standard.
> > + Copyright (C) 1995-2015 Free Software Foundation, Inc.
> > + This file is part of the GNU C Library.
> > +
> > + The GNU C Library is free software; you can redistribute it
> > and/or
> > + modify it under the terms of the GNU Lesser General Public
> > + License as published by the Free Software Foundation; either
> > + version 2.1 of the License, or (at your option) any later
> > version.
> > +
> > + The GNU C Library 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
> > + Lesser General Public License for more details.
> > +
> > + You should have received a copy of the GNU Lesser General Public
> > + License along with the GNU C Library; if not, see
> > + <http://www.gnu.org/licenses/>. */
>
> That looks like the wrong header.
> Just copy the one from i386_disasm.c
I fixed the header.
> > +#ifndef _I386_MNE_H
> > +#define _I386_MNE_H 1
> > +
> > +#ifndef MNEFILE
> > +# define MNEFILE "i386.mnemonics"
> > +#endif
> > +
> > +/* The index can be stored in the instrtab. */
> > +enum
> > + {
> > +#define MNE(name) MNE_##name,
> > +#include MNEFILE
> > +#undef MNE
> > + MNE_INVALID,
> > + MNE_COUNT = MNE_INVALID,
> > + };
> > +
> > +#endif /* i386_mne.h */
>
> OK.
>
> > diff --git a/libcpu/i386_parse.y b/libcpu/i386_parse.y
> > index d2236d59..459684c6 100644
> > --- a/libcpu/i386_parse.y
> > +++ b/libcpu/i386_parse.y
> > @@ -46,6 +46,8 @@
> > #include <libeu.h>
> > #include <system.h>
> >
> > +#include "i386_mne.h"
> > +
> > #define obstack_chunk_alloc xmalloc
> > #define obstack_chunk_free free
> >
> > @@ -1107,11 +1109,6 @@ print_op_fct (const void *nodep, VISIT value,
> > }
> > }
> >
> > -
> > -#if NMNES < 2
> > -# error "bogus NMNES value"
> > -#endif
> > -
> > static void
> > instrtable_out (void)
> > {
> > @@ -1123,7 +1120,7 @@ instrtable_out (void)
> > fprintf (outfile, "#define MNEMONIC_BITS %zu\n",
> > best_mnemonic_bits);
> > #else
> > fprintf (outfile, "#define MNEMONIC_BITS %ld\n",
> > - lrint (ceil (log2 (NMNES))));
> > + lrint (ceil (log2 (MNE_COUNT))));
> > #endif
> > fprintf (outfile, "#define SUFFIX_BITS %d\n", nbitsuf);
> > for (int i = 0; i < 3; ++i)
>
> OK.
Pushed with those changes and a ChangeLog entry as attached.
Cheers,
Mark
[-- Attachment #2: 0001-libcpu-Remove-the-need-of-NMNES-by-using-enum.patch --]
[-- Type: text/x-patch, Size: 4994 bytes --]
From 4961f9ae2f11795022166698aa15a15f48ec8c5b Mon Sep 17 00:00:00 2001
From: Yonggang Luo <luoyonggang@gmail.com>
Date: Sun, 18 Dec 2022 00:52:02 +0800
Subject: [PATCH] libcpu: Remove the need of NMNES by using enum
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Signed-off-by: Mark Wielaard <mark@klomp.org>
---
libcpu/ChangeLog | 9 +++++++++
libcpu/Makefile.am | 3 +--
libcpu/i386_disasm.c | 14 +-------------
libcpu/i386_mne.h | 46 ++++++++++++++++++++++++++++++++++++++++++++
libcpu/i386_parse.y | 9 +++------
5 files changed, 60 insertions(+), 21 deletions(-)
create mode 100644 libcpu/i386_mne.h
diff --git a/libcpu/ChangeLog b/libcpu/ChangeLog
index 6d4b717a..d14cd237 100644
--- a/libcpu/ChangeLog
+++ b/libcpu/ChangeLog
@@ -1,3 +1,12 @@
+2022-12-18 Yonggang Luo <luoyonggang@gmail.com>
+
+ * i386_mne.h: New file, extracted from i386_disasm.c.
+ * Makefile.am (noinst_HEADERS): Add i386_mne.h.
+ (i386_parse_CFLAGS): Removed.
+ * i386_disasm.c: Include i386_mne.h.
+ * i386_parse.y: Include i386_mne.h.
+ (instrtable_out): Use MNE_COUNT instead of NMNES.
+
2022-12-18 Yonggang Luo <luoyonggang@gmail.com>
* i386_disasm.c (i386_disasm): Use __asm instead of asm.
diff --git a/libcpu/Makefile.am b/libcpu/Makefile.am
index 57d0a164..4ba1be56 100644
--- a/libcpu/Makefile.am
+++ b/libcpu/Makefile.am
@@ -40,7 +40,7 @@ AM_YFLAGS = -p$(<F:parse.y=)
noinst_LIBRARIES = libcpu.a libcpu_pic.a
-noinst_HEADERS = i386_dis.h x86_64_dis.h
+noinst_HEADERS = i386_dis.h i386_mne.h x86_64_dis.h
libcpu_a_SOURCES = i386_disasm.c x86_64_disasm.c bpf_disasm.c riscv_disasm.c
@@ -92,7 +92,6 @@ libeu = ../lib/libeu.a
i386_lex_CFLAGS = -Wno-unused-label -Wno-unused-function -Wno-sign-compare \
-Wno-implicit-fallthrough
i386_parse.o: i386_parse.c i386.mnemonics
-i386_parse_CFLAGS = -DNMNES="`wc -l < i386.mnemonics`"
i386_lex.o: i386_parse.h
i386_gendis_LDADD = $(libeu) -lm $(obstack_LIBS)
diff --git a/libcpu/i386_disasm.c b/libcpu/i386_disasm.c
index 09946273..dec62bfa 100644
--- a/libcpu/i386_disasm.c
+++ b/libcpu/i386_disasm.c
@@ -46,10 +46,7 @@
#define MACHINE_ENCODING LITTLE_ENDIAN
#include "memory-access.h"
-
-#ifndef MNEFILE
-# define MNEFILE "i386.mnemonics"
-#endif
+#include "i386_mne.h"
#define MNESTRFIELD(line) MNESTRFIELD1 (line)
#define MNESTRFIELD1(line) str##line
@@ -71,15 +68,6 @@ static const union mnestr_t
}
};
-/* The index can be stored in the instrtab. */
-enum
- {
-#define MNE(name) MNE_##name,
-#include MNEFILE
-#undef MNE
- MNE_INVALID
- };
-
static const unsigned short int mneidx[] =
{
#define MNE(name) \
diff --git a/libcpu/i386_mne.h b/libcpu/i386_mne.h
new file mode 100644
index 00000000..d5157515
--- /dev/null
+++ b/libcpu/i386_mne.h
@@ -0,0 +1,46 @@
+/* Disassembler for x86, MNE enums.
+ Copyright (C) 2007, 2008, 2009, 2011 Red Hat, Inc.
+ This file is part of elfutils.
+
+ This file is free software; you can redistribute it and/or modify
+ it under the terms of either
+
+ * the GNU Lesser General Public License as published by the Free
+ Software Foundation; either version 3 of the License, or (at
+ your option) any later version
+
+ or
+
+ * the GNU General Public License as published by the Free
+ Software Foundation; either version 2 of the License, or (at
+ your option) any later version
+
+ or both in parallel, as here.
+
+ elfutils 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 copies of the GNU General Public License and
+ the GNU Lesser General Public License along with this program. If
+ not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef _I386_MNE_H
+#define _I386_MNE_H 1
+
+#ifndef MNEFILE
+# define MNEFILE "i386.mnemonics"
+#endif
+
+/* The index can be stored in the instrtab. */
+enum
+ {
+#define MNE(name) MNE_##name,
+#include MNEFILE
+#undef MNE
+ MNE_INVALID,
+ MNE_COUNT = MNE_INVALID,
+ };
+
+#endif /* i386_mne.h */
diff --git a/libcpu/i386_parse.y b/libcpu/i386_parse.y
index d2236d59..459684c6 100644
--- a/libcpu/i386_parse.y
+++ b/libcpu/i386_parse.y
@@ -46,6 +46,8 @@
#include <libeu.h>
#include <system.h>
+#include "i386_mne.h"
+
#define obstack_chunk_alloc xmalloc
#define obstack_chunk_free free
@@ -1107,11 +1109,6 @@ print_op_fct (const void *nodep, VISIT value,
}
}
-
-#if NMNES < 2
-# error "bogus NMNES value"
-#endif
-
static void
instrtable_out (void)
{
@@ -1123,7 +1120,7 @@ instrtable_out (void)
fprintf (outfile, "#define MNEMONIC_BITS %zu\n", best_mnemonic_bits);
#else
fprintf (outfile, "#define MNEMONIC_BITS %ld\n",
- lrint (ceil (log2 (NMNES))));
+ lrint (ceil (log2 (MNE_COUNT))));
#endif
fprintf (outfile, "#define SUFFIX_BITS %d\n", nbitsuf);
for (int i = 0; i < 3; ++i)
--
2.39.2
next prev parent reply other threads:[~2023-02-23 11:50 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-17 16:51 [PATCH v2 00/16] Patches for building with mingw/gcc msvc/clang-cl Yonggang Luo
2022-12-17 16:51 ` [PATCH v2 01/16] ignore build directory Yonggang Luo
2022-12-17 16:51 ` [PATCH v2 02/16] move platform depended include into system.h of libebl Yonggang Luo
2023-02-23 10:44 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 03/16] Use configure to detect HAVE_DECL_MMAP and use it for system doesn't provide sys/mman.h Yonggang Luo
2023-02-23 10:52 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 04/16] Fixes usage of basename about prototype differences Yonggang Luo
2023-02-23 11:00 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 05/16] libcpu: Remove the need of NMNES by using enum Yonggang Luo
2022-12-21 17:56 ` Mark Wielaard
2023-02-23 11:50 ` Mark Wielaard [this message]
2022-12-17 16:52 ` [PATCH v2 06/16] libcpu: Use __asm instead asm that can be recognized by both clang-cl and gcc Yonggang Luo
2022-12-21 18:07 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 07/16] libdw: Fixes compile of dwarf_whatattr.c and dwarf_whatform.c Yonggang Luo
2023-02-23 12:25 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 08/16] lib: Implement error properly even when not HAVE_ERR_H Yonggang Luo
2023-02-23 12:31 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 09/16] libelf: uid_t, gid_t and mode_t are not comes with msvcrt, so using long/unsigned long instead on win32 Yonggang Luo
2023-03-02 13:24 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 10/16] libasm: stdio_ext.h are not present " Yonggang Luo
2023-02-23 12:35 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 11/16] libebl/libdwelf: define ssize_t and pid_t for MSVC within installed header libdwelf.h and libebl.h Yonggang Luo
2023-03-02 13:32 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 12/16] libasm/debuginfod: fchmod doesn't present on win32 Yonggang Luo
2022-12-21 18:16 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 13/16] lib: isatty is not available on windows Yonggang Luo
2022-12-19 12:32 ` 罗勇刚(Yonggang Luo)
2022-12-17 16:52 ` [PATCH v2 14/16] Add function sys_get_page_size to replace platform dependent sysconf (_SC_PAGESIZE) Yonggang Luo
2023-03-02 16:33 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 15/16] libelf: F_GETFD may not predefined with msvc/mingw, guard the usage of it Yonggang Luo
2022-12-21 22:29 ` Mark Wielaard
2022-12-17 16:52 ` [PATCH v2 16/16] lib: Use HAVE_LIBINTL_H to guard #include <libintl.h> Yonggang Luo
2022-12-21 23:07 ` Mark Wielaard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fa21ee76f41a49be901a76b68fde8d4b6f2de1ca.camel@klomp.org \
--to=mark@klomp.org \
--cc=elfutils-devel@sourceware.org \
--cc=luoyonggang@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).