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

Jan Beulich JBeulich@suse.com
Mon Jul 16 13:09:00 GMT 2018


>>> On 16.07.18 at 14:45, <hjl.tools@gmail.com> wrote:
> 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.

This isn't about using a C compiler, but about possibly having to do
more fancy automake-ery in Makefile.ac. If the patch as presented
is fine, then all is good.

>> 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.

I don't think I understand this remark.

>> --- 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.

This is the full patch (minus the ChangeLog entries of course). All
following patches are basically or entirely unchanged from this
submission (the sole exception being the mechanical switch to this
pre-processing model), just that there are several further patches
at the end now.

Jan



More information about the Binutils mailing list