Re: [PATCH 02/12] x86: fold various AVX512VL templates into their AVX512F counterparts

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Jul 11, 2018 at 2:30 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 04.06.18 at 15:19, <hjl.tools@gmail.com> wrote:
>> On Mon, Jun 4, 2018 at 6:03 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>> If you dislike the shorthand approach, passing i386-opc.tbl through
>>> the C pre-processor would be another option.
>>
>> I prefer a C pre-processor approach.
>
> Before spamming the list with the entire updated series, here's the
> half way reasonable patch I've been able to come up with (I continue
> to prefer the original approach though). If that's acceptable, I can
> submit the full series. If you dislike any of it, please give concrete
> suggestions on what to change.
>
> What I specifically don't fancy doing is anything that would require
> me to truly re-generate Makefile.in (with the "correct" version of
> automake). Should that be required for any reason, I'd then like to

I do regenerate Makefile.in and I have to do the same.  It shouldn't
be the reason not to use a C compiler.

> propose that I revert back to the original approach, allowing you
> to convert to pre-processing if that suits you better.
>
> One of the fundamental reasons why I'd prefer to stick to the first
> version's approach is that there's nowhere in binutils that I could
> find that pre-processing as a solitary step was used for anything.
> I'm simply concerned that with non-gcc compilers things may not
> work as expected.

There is only one i386-gen.c and nothing else.  We can add
pre-processing if needed.

> Jan
>
> --- a/opcodes/Makefile.am
> +++ b/opcodes/Makefile.am
> @@ -544,8 +544,10 @@ i386-gen.o: i386-gen.c i386-opc.h $(srcd
>  $(srcdir)/i386-tbl.h: $(srcdir)/i386-init.h
>         @echo $@
>
> -$(srcdir)/i386-init.h: @MAINT@ i386-gen$(EXEEXT_FOR_BUILD) i386-opc.tbl i386-reg.tbl
> -       ./i386-gen$(EXEEXT_FOR_BUILD) --srcdir $(srcdir)
> +$(srcdir)/i386-init.h: @MAINT@ i386-gen$(EXEEXT_FOR_BUILD) i386-opc.tbl i386-reg.tbl i386-opc.h
> +       $(CPP) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) - \
> +               < $(srcdir)/i386-opc.tbl \
> +               | ./i386-gen$(EXEEXT_FOR_BUILD) --srcdir $(srcdir)
>
>  i386-opc.lo: $(srcdir)/i386-tbl.h
>
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -1262,14 +1262,10 @@ process_i386_opcodes (FILE *table)
>    htab_t opcode_hash_table;
>    struct opcode_hash_entry **opcode_array;
>    unsigned int opcode_array_size = 1024;
> -  int lineno = 0;
> +  int lineno = 0, marker = 0;
>
>    filename = "i386-opc.tbl";
> -  fp = fopen (filename, "r");
> -
> -  if (fp == NULL)
> -    fail (_("can't find i386-opc.tbl for reading, errno = %s\n"),
> -         xstrerror (errno));
> +  fp = stdin;
>
>    i = 0;
>    opcode_array = (struct opcode_hash_entry **)
> @@ -1303,11 +1299,32 @@ process_i386_opcodes (FILE *table)
>        switch (p[0])
>         {
>         case '#':
> +         if (!strcmp("### MARKER ###", buf))
> +           marker = 1;
> +         else
> +           {
> +             /* Since we ignore all included files (we only care about their
> +                #define-s here), we don't need to monitor filenames.  The final
> +                line number directive is going to refer to the main source file
> +                again.  */
> +             char *end;
> +             unsigned long ln;
> +
> +             p = remove_leading_whitespaces (p + 1);
> +             if (!strncmp(p, "line", 4))
> +               p += 4;
> +             ln = strtoul (p, &end, 10);
> +             if (ln > 1 && ln < INT_MAX
> +                 && *remove_leading_whitespaces (end) == '"')
> +               lineno = ln - 1;
> +           }
>           /* Ignore comments.  */
>         case '\0':
>           continue;
>           break;
>         default:
> +         if (!marker)
> +           continue;
>           break;
>         }
>
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -18,6 +18,12 @@
>  // Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA
>  // 02110-1301, USA.
>
> +#define OPCODE_I386_H
> +#include "i386-opc.h"
> +#undef None
> +
> +### MARKER ###
> +
>  // Move instructions.
>  // We put the 64bit displacement first and we only mark constants
>  // larger than 32bit as Disp64.
>
>
>

I need to see the full patch.

-- 
H.J.