Re: [patch][x86] -march=icelake

[Uros Bizjak <ubizjak@gmail.com>]

On Sun, Nov 12, 2017 at 1:04 AM, Koval, Julia <julia.koval@intel.com> wrote:
> Hi, this patch adds new option -march=icelake. Isasets defined in: https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf
> I didn't add arch code to driver-i386.c, because there is no code available in SDM yet, only for cannonlake (https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf Chapter 2).

This means the driver will go through generic detection for
-march=native. Perhaps a comment should be added, so we won't forget
to add the model number when one is available.

> gcc/
>         * config.gcc: Add -march=icelake.
>         * config/i386/driver-i386.c (host_detect_local_cpu): Detect icelake.
>         * config/i386/i386-c.c (ix86_target_macros_internal): Handle icelake.
>         * config/i386/i386.c (processor_costs): Add m_ICELAKE.
>         (PTA_ICELAKE, PTA2_ICELAKE, PTA2_GFNI, PTA2_AVX512VBMI2, PTA2_VAES,
>         PTA2_AVX512VNNI, PTA2_VPCLMULQDQ, PTA2_RDPID, PTA2_AVX512BITALG): New.
>         (processor_target_table): Add icelake.
>         (ix86_option_override_internal): Add flags2 for new PTA, handle GFNI, RDPID.
>         (get_builtin_code_for_version): Handle icelake.
>         (M_INTEL_COREI7_ICELAKE): New.
>         * config/i386/i386.h (TARGET_ICELAKE, PROCESSOR_ICELAKE): New.
>         * doc/invoke.texi: Add -march=icelake.
> gcc/testsuite/
>         * gcc.target/i386/funcspec-56.inc: Handle new march.
>         * g++.dg/ext/mv16.C: Ditto.
> libgcc/
>         * config/i386/cpuinfo.h (processor_subtypes): Add INTEL_COREI7_ICELAKE.

@@ -3425,6 +3427,13 @@ ix86_option_override_internal (bool main_args_p,
 #define PTA_AVX5124FMAPS    (HOST_WIDE_INT_1 << 61)
 #define PTA_AVX512VPOPCNTDQ    (HOST_WIDE_INT_1 << 62)
 #define PTA_SGX            (HOST_WIDE_INT_1 << 63)
+#define PTA2_GFNI        (HOST_WIDE_INT_1 << 0)
+#define PTA2_AVX512VBMI2    (HOST_WIDE_INT_1 << 1)
+#define PTA2_VAES        (HOST_WIDE_INT_1 << 2)
+#define PTA2_AVX512VNNI        (HOST_WIDE_INT_1 << 3)
+#define PTA2_VPCLMULQDQ        (HOST_WIDE_INT_1 << 4)
+#define PTA2_RDPID        (HOST_WIDE_INT_1 << 5)
+#define PTA2_AVX512BITALG    (HOST_WIDE_INT_1 << 6)

Please add these options first.

On a related note, there should probably be a better way to extend
various bitmapped flag variables beyond 64bit words. We are constantly
going over 64bit sizes in target option masks, now the number of
processor flags doesn't fit in a word anymore. There are several
places one has to keep in mind in which word some specific flag lives,
and this  approach opens several ways to make a hard to detect
mistake. Does C++ offer a more elegant way?

Bellow, please find a suggestion of a couple of cosmetic changes.

Thanks,
Uros.

@@ -3425,6 +3427,13 @@ ix86_option_override_internal (bool main_args_p,
 #define PTA_AVX5124FMAPS    (HOST_WIDE_INT_1 << 61)
 #define PTA_AVX512VPOPCNTDQ    (HOST_WIDE_INT_1 << 62)
 #define PTA_SGX            (HOST_WIDE_INT_1 << 63)

Please add a comment here, that the folowing belongs to flags2.

+#define PTA2_GFNI        (HOST_WIDE_INT_1 << 0)
+#define PTA2_AVX512VBMI2    (HOST_WIDE_INT_1 << 1)
+#define PTA2_VAES        (HOST_WIDE_INT_1 << 2)


@@ -4105,6 +4124,12 @@ ix86_option_override_internal (bool main_args_p,
     if (processor_alias_table[i].flags & PTA_SGX
         && !(opts->x_ix86_isa_flags2_explicit & OPTION_MASK_ISA_SGX))
       opts->x_ix86_isa_flags2 |= OPTION_MASK_ISA_SGX;

Please add vertical space here to visually separate flags and flags2 processing.

+    if (processor_alias_table[i].flags2 & PTA2_RDPID
+        && !(opts->x_ix86_isa_flags2_explicit & OPTION_MASK_ISA_RDPID))
+      opts->x_ix86_isa_flags2 |= OPTION_MASK_ISA_RDPID;