RE: [PATCH, i386] Refactor -mprefer-avx[128|256] options into common -mprefer-vector-width=[none|128|256|512]

["Shalnov, Sergey" <sergey.shalnov@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2661 bytes --]

Uros,
I did new patch with all comments addressed as proposed.
1. old option -mprefer-avx128 is Alias(mprefer-vector-width=, 128, none)
2. Simplified default initialization (as Bernhard proposed)
3. Fixed documentation (proposed by Sandra)
4. Several tests are changed to use new style of the option but many leaved with -mprefer-avx128 (one test with new style -mprefer-vector-width=128)


2017-11-21  Sergey Shalnov  <Sergey.Shalnov@intel.com>

gcc/
	* config/i386/i386-opts.h (enum prefer_vector_width): Added new enum
	for the new option -mprefer-vector-width=[none|128|256|512].
	* config/i386/i386.c (ix86_target_string): remove old style options
	-mprefer-avx256 and make -mprefer-avx128 as alias.
	(ix86_option_override_internal):  Apply defaults for the
	-mprefer-vector-width=[128|256] option.
	* config/i386/i386.h (TARGET_PREFER_AVX128, TARGET_PREFER_AVX256):
	Implement macros to work with -mprefer-vector-width=.
	* config/i386/i386.opt: Implemented option
	-mprefer-vector-width=[none|128|256|512].
	* doc/invoke.texi: Documentation for
	-mprefer-vector-width=[none|128|256|512].

gcc/testsuite/
	* g++.dg/ext/pr57362.C (__attribute__): Apply new option syntax.
	* g++.dg/torture/pr81249.C: Ditto.
	* gcc.target/i386/avx512f-constant-float-return.c: Ditto.
	* gcc.target/i386/avx512f-prefer.c: Ditto.
	* gcc.target/i386/pr82460-2.c: Ditto.

Please merge this patch if you think it is acceptable.
Thank you
Sergey


-----Original Message-----
From: Uros Bizjak [mailto:ubizjak@gmail.com] 
Sent: Tuesday, November 14, 2017 7:57 AM
To: Joseph Myers <joseph@codesourcery.com>
Cc: Shalnov, Sergey <sergey.shalnov@intel.com>; gcc-patches@gcc.gnu.org; kirill.yukhin@gmail.com; Koval, Julia <julia.koval@intel.com>; Senkevich, Andrew <andrew.senkevich@intel.com>; Peryt, Sebastian <sebastian.peryt@intel.com>; Ivchenko, Alexander <alexander.ivchenko@intel.com>
Subject: Re: [PATCH, i386] Refactor -mprefer-avx[128|256] options into common -mprefer-vector-width=[none|128|256|512]

On Tue, Nov 14, 2017 at 12:14 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Mon, 13 Nov 2017, Uros Bizjak wrote:
>
>> [BTW: --mprefer-avx128 should be marked RejectNegative from the 
>> beginning; let's just assume nobody uses it in its (somehow weird) 
>> negative "-mno-prefer-avx128" form.]
>
> It's used in that form in various testcases that otherwise fail when 
> GCC is configured --with-arch= some CPU that defaults to -mprefer-avx128.

In this case, an even better choice would be:

Alias(mprefer-vector-width=, 128, none)

So, -mno-prefer-avx128 would just set the default back to none.

Uros.

[-- Attachment #2: 0004-Refactoring-options-mprefer-avx-128-256-into-one-mpr.patch --]
[-- Type: application/octet-stream, Size: 10239 bytes --]

From f6fa45a434334bcba763ab115591513f4f8a2dc5 Mon Sep 17 00:00:00 2001
From: Sergey Shalnov <Sergey.Shalnov@intel.com>
Date: Tue, 21 Nov 2017 09:03:54 +0300
Subject: [PATCH 1/1] Refactoring options mprefer-avx[128|256] into one
 mprefer-vector-width

---
 gcc/config/i386/i386-opts.h                        |  7 ++++++
 gcc/config/i386/i386.c                             | 16 ++++++------
 gcc/config/i386/i386.h                             |  5 ++++
 gcc/config/i386/i386.opt                           | 29 ++++++++++++++++++----
 gcc/doc/invoke.texi                                | 27 +++++++++++++-------
 gcc/testsuite/g++.dg/ext/pr57362.C                 |  4 +--
 gcc/testsuite/g++.dg/torture/pr81249.C             |  2 +-
 .../i386/avx512f-constant-float-return.c           |  2 +-
 gcc/testsuite/gcc.target/i386/avx512f-prefer.c     |  2 +-
 gcc/testsuite/gcc.target/i386/pr82460-2.c          |  2 +-
 10 files changed, 68 insertions(+), 28 deletions(-)

diff --git a/gcc/config/i386/i386-opts.h b/gcc/config/i386/i386-opts.h
index 542cd0f..8c37a74 100644
--- a/gcc/config/i386/i386-opts.h
+++ b/gcc/config/i386/i386-opts.h
@@ -99,4 +99,11 @@ enum stack_protector_guard {
   SSP_GLOBAL    /* global canary */
 };
 
+enum prefer_vector_width {
+    PVW_NONE,
+    PVW_AVX128,
+    PVW_AVX256,
+    PVW_AVX512
+};
+
 #endif
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 1775697..fb03d12 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2847,15 +2847,13 @@ ix86_target_string (HOST_WIDE_INT isa, HOST_WIDE_INT isa2,
     { "-mstv",				MASK_STV },
     { "-mavx256-split-unaligned-load",	MASK_AVX256_SPLIT_UNALIGNED_LOAD },
     { "-mavx256-split-unaligned-store",	MASK_AVX256_SPLIT_UNALIGNED_STORE },
-    { "-mprefer-avx128",		MASK_PREFER_AVX128 },
     { "-mcall-ms2sysv-xlogues",		MASK_CALL_MS2SYSV_XLOGUES }
   };
 
   /* Additional flag options.  */
   static struct ix86_target_opts flag2_opts[] =
   {
-    { "-mgeneral-regs-only",		OPTION_MASK_GENERAL_REGS_ONLY },
-    { "-mprefer-avx256",		OPTION_MASK_PREFER_AVX256 },
+    { "-mgeneral-regs-only",		OPTION_MASK_GENERAL_REGS_ONLY }
   };
 
   const char *opts[ARRAY_SIZE (isa_opts) + ARRAY_SIZE (isa2_opts)
@@ -4686,16 +4684,18 @@ ix86_option_override_internal (bool main_args_p,
   if (!ix86_tune_features[X86_TUNE_AVX256_UNALIGNED_STORE_OPTIMAL]
       && !(opts_set->x_target_flags & MASK_AVX256_SPLIT_UNALIGNED_STORE))
     opts->x_target_flags |= MASK_AVX256_SPLIT_UNALIGNED_STORE;
+
   /* Enable 128-bit AVX instruction generation
      for the auto-vectorizer.  */
   if (TARGET_AVX128_OPTIMAL
-      && !(opts_set->x_target_flags & MASK_PREFER_AVX128))
-    opts->x_target_flags |= MASK_PREFER_AVX128;
-  /* Use 256-bit AVX instructions instead of 512-bit AVX instructions
+      && (opts_set->x_prefer_vector_width_type == PVW_NONE))
+    opts->x_prefer_vector_width_type = PVW_AVX128;
+
+  /* Use 256-bit AVX instruction generation
      in the auto-vectorizer.  */
   if (ix86_tune_features[X86_TUNE_AVX256_OPTIMAL]
-      && !(opts_set->x_ix86_target_flags & OPTION_MASK_PREFER_AVX256))
-    opts->x_ix86_target_flags |= OPTION_MASK_PREFER_AVX256;
+      && (opts_set->x_prefer_vector_width_type == PVW_NONE))
+    opts->x_prefer_vector_width_type = PVW_AVX256;
 
   if (opts->x_ix86_recip_name)
     {
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index bc601c8..6eadc03 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2678,6 +2678,11 @@ extern void debug_dispatch_window (int);
 #define TARGET_RECIP_VEC_DIV	((recip_mask & RECIP_MASK_VEC_DIV) != 0)
 #define TARGET_RECIP_VEC_SQRT	((recip_mask & RECIP_MASK_VEC_SQRT) != 0)
 
+/* Use 128-bit AVX instructions in the auto-vectorizer.  */
+#define TARGET_PREFER_AVX128	(prefer_vector_width_type == PVW_AVX128)
+/* Use 256-bit AVX instructions in the auto-vectorizer.  */
+#define TARGET_PREFER_AVX256	(prefer_vector_width_type == PVW_AVX256)
+
 #define IX86_HLE_ACQUIRE (1 << 16)
 #define IX86_HLE_RELEASE (1 << 17)
 
diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 0fb4698..1827a0c 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -182,6 +182,10 @@ int x_ix86_tune_no_default
 TargetSave
 enum ix86_veclibabi x_ix86_veclibabi_type
 
+;; -mprefer-vector-width=
+TargetSave
+enum prefer_vector_width x_prefer_vector_width_type
+
 ;; x86 options
 m128bit-long-double
 Target RejectNegative Report Mask(128BIT_LONG_DOUBLE) Save
@@ -588,12 +592,27 @@ Do dispatch scheduling if processor is bdver1, bdver2, bdver3, bdver4
 or znver1 and Haifa scheduling is selected.
 
 mprefer-avx128
-Target Report Mask(PREFER_AVX128) Save
-Use 128-bit AVX instructions instead of 256-bit AVX instructions in the auto-vectorizer.
+Target Undocumented Alias(mprefer-vector-width=, 128, none)
+
+mprefer-vector-width=
+Target Report RejectNegative Joined Var(prefer_vector_width_type) Enum(prefer_vector_width) Init(PVW_NONE)
+Use given register vector width instructions instead of maximum register width in the auto-vectorizer.
+
+Enum
+Name(prefer_vector_width) Type(enum prefer_vector_width)
+Known preferred register vector length (to use with the -mprefer-vector-width= option)
+
+EnumValue
+Enum(prefer_vector_width) String(none) Value(PVW_NONE)
+
+EnumValue
+Enum(prefer_vector_width) String(128) Value(PVW_AVX128)
 
-mprefer-avx256
-Target Report Mask(PREFER_AVX256) Var(ix86_target_flags) Save
-Use 256-bit AVX instructions instead of 512-bit AVX instructions in the auto-vectorizer.
+EnumValue
+Enum(prefer_vector_width) String(256) Value(PVW_AVX256)
+
+EnumValue
+Enum(prefer_vector_width) String(512) Value(PVW_AVX512)
 
 ;; ISA support
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e18fa54..b483710 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1196,7 +1196,7 @@ See RS/6000 and PowerPC Options.
 -mincoming-stack-boundary=@var{num} @gol
 -mcld  -mcx16  -msahf  -mmovbe  -mcrc32 @gol
 -mrecip  -mrecip=@var{opt} @gol
--mvzeroupper  -mprefer-avx128 -mprefer-avx256 @gol
+-mvzeroupper  -mprefer-vector-width=@var{opt} @gol
 -mmmx  -msse  -msse2  -msse3  -mssse3  -msse4.1  -msse4.2  -msse4  -mavx @gol
 -mavx2  -mavx512f  -mavx512pf  -mavx512er  -mavx512cd  -mavx512vl @gol
 -mavx512bw  -mavx512dq  -mavx512ifma  -mavx512vbmi  -msha  -maes @gol
@@ -26088,15 +26088,24 @@ before a transfer of control flow out of the function to minimize
 the AVX to SSE transition penalty as well as remove unnecessary @code{zeroupper}
 intrinsics.
 
-@item -mprefer-avx128
-@opindex mprefer-avx128
-This option instructs GCC to use 128-bit AVX instructions instead of
-256-bit AVX instructions in the auto-vectorizer.
+@item -mprefer-vector-width=@var{opt}
+@opindex mprefer-vector-width
+This option instructs GCC to use @var{opt}-bit vector width in instructions
+instead of default on the selected platform.
 
-@item -mprefer-avx256
-@opindex mprefer-avx256
-This option instructs GCC to use 256-bit AVX instructions instead of
-512-bit AVX instructions in the auto-vectorizer.
+@table @samp
+@item none
+No extra limitations applied to GCC other than defined by the selected platform.
+
+@item 128
+Prefer 128-bit vector width for instructions.
+
+@item 256
+Prefer 256-bit vector width for instructions.
+
+@item 512
+Prefer 512-bit vector width for instructions.
+@end table
 
 @item -mcx16
 @opindex mcx16
diff --git a/gcc/testsuite/g++.dg/ext/pr57362.C b/gcc/testsuite/g++.dg/ext/pr57362.C
index 3613b63..c9b3119 100644
--- a/gcc/testsuite/g++.dg/ext/pr57362.C
+++ b/gcc/testsuite/g++.dg/ext/pr57362.C
@@ -79,9 +79,9 @@ __attribute__((target("vzeroupper")))
 int foo(void) { return 1; }
 __attribute__((target("dispatch-scheduler")))
 int foo(void) { return 1; }
-__attribute__((target("prefer-avx128")))
+__attribute__((target("prefer-vector-width=128")))
 int foo(void) { return 1; }
-__attribute__((target("prefer-avx256")))
+__attribute__((target("prefer-vector-width=256")))
 int foo(void) { return 1; }
 __attribute__((target("32")))
 int foo(void) { return 1; }
diff --git a/gcc/testsuite/g++.dg/torture/pr81249.C b/gcc/testsuite/g++.dg/torture/pr81249.C
index 7bf617b..5af319d 100644
--- a/gcc/testsuite/g++.dg/torture/pr81249.C
+++ b/gcc/testsuite/g++.dg/torture/pr81249.C
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-additional-options "-mavx2 -mprefer-avx128" { target x86_64-*-* i?86-*-* } } */
+/* { dg-additional-options "-mavx2 -mprefer-vector-width=128" { target x86_64-*-* i?86-*-* } } */
 
 typedef struct rtx_def *rtx;
 union rtunion {
diff --git a/gcc/testsuite/gcc.target/i386/avx512f-constant-float-return.c b/gcc/testsuite/gcc.target/i386/avx512f-constant-float-return.c
index 153cf69..ed0becc 100644
--- a/gcc/testsuite/gcc.target/i386/avx512f-constant-float-return.c
+++ b/gcc/testsuite/gcc.target/i386/avx512f-constant-float-return.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O3 -march=skylake-avx512 -mprefer-avx256" } */
+/* { dg-options "-O3 -march=skylake-avx512 -mprefer-vector-width=256" } */
 /* { dg-final { scan-assembler-not "%zmm\[0-9\]+" } } */
 
 float
diff --git a/gcc/testsuite/gcc.target/i386/avx512f-prefer.c b/gcc/testsuite/gcc.target/i386/avx512f-prefer.c
index defe51e..a74ce07 100644
--- a/gcc/testsuite/gcc.target/i386/avx512f-prefer.c
+++ b/gcc/testsuite/gcc.target/i386/avx512f-prefer.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O3 -march=skylake-avx512 -mprefer-avx256" } */
+/* { dg-options "-O3 -march=skylake-avx512 -mprefer-vector-width=256" } */
 /* { dg-final { scan-assembler-not "%zmm\[0-9\]+" } } */
 /* { dg-final { scan-assembler "vmulpd" } } */
 
diff --git a/gcc/testsuite/gcc.target/i386/pr82460-2.c b/gcc/testsuite/gcc.target/i386/pr82460-2.c
index 4d96521..4a45bee 100644
--- a/gcc/testsuite/gcc.target/i386/pr82460-2.c
+++ b/gcc/testsuite/gcc.target/i386/pr82460-2.c
@@ -1,6 +1,6 @@
 /* PR target/82460 */
 /* { dg-do compile } */
-/* { dg-options "-O2 -ftree-vectorize -mavx512vbmi -mno-prefer-avx256" } */
+/* { dg-options "-O2 -ftree-vectorize -mavx512vbmi -mprefer-vector-width=none" } */
 /* We want to reuse the permutation mask in the loop, so use vpermt2b rather
    than vpermi2b.  */
 /* { dg-final { scan-assembler-not {\mvpermi2b\M} } } */
-- 
1.8.3.1