On Tue, Apr 11, 2017 at 06:04:33PM -0500, Segher Boessenkool wrote:
> Hi!
>
> On Tue, Apr 11, 2017 at 05:32:41PM -0400, Michael Meissner wrote:
> > PR 80098 is an interaction between -mmodulo (ISA 3.0/power9 GPR modulo
> > instructions) and -mno-vsx where the -mmodulo option enables some of the ISA
> > 3.0 vector features, even though -mno-vsx was specified.
> >
> > To do this, I added a table for disabling other vector options when the
> > major
> > vector options (-mvsx, -mpower8-vector, and -mpower9-vector) are disabled.
> >
> > We could extend this if desired for other options (for example, -mno-popcntd
> > could disable -mmodulo and perhaps the vector options).
>
> Or we could just remove -mmodulo etc. What good do they do? They make
> testing infeasible: an exponential number of combinations to test.
We can't remove -mmodulo, -mpopcntd, -mcmpb, -mpopcntb as these are the basic
markers for -mcpu=power9/power7/power6/power5, and lots of other things depend
on these options. I'm not sure we have a marker for power8 that isn't vector
related.
Yeah, it would be better to have specific ISA levels, but we don't currently
have them.
> > @@ -3967,7 +3969,7 @@ rs6000_option_override_internal (bool gl
> > #endif
> > #ifdef OS_MISSING_ALTIVEC
> > if (OS_MISSING_ALTIVEC)
> > - set_masks &= ~(OPTION_MASK_ALTIVEC | OPTION_MASK_VSX);
> > + set_masks &= ~NO_ALTIVEC_MASKS;
>
> NO_ALTIVEC_MASKS isn't defined anywhere. Did you send the wrong patch?
I originally had NO_ALTIVEC_MASKS and then I redid the code, naming them
OTHER_<xxx>_VECTOR_MASKS. I missed fixing the section that disables Altivec
and VSX instructions if the assembler doesn't have Altivec support.
The following patch fixes this. I reran the tests on a little endian power8
system.
I also tested the code on a big endian power7 machine by building a compiler
and then rebuilding rs6000.o, adding the OS_MISSING_ALTIVEC define. I verified
by hand that it disables Altivec and VSX support by default.
Can I apply this to the trunk?
[gcc]
2017-04-12 Michael Meissner <meiss...@linux.vnet.ibm.com>
PR target/80098
* config/rs6000/rs6000-cpus.def (OTHER_P9_VECTOR_MASKS): Define
masks of options that should be turned off if the VSX vector
options are turned off.
(OTHER_P8_VECTOR_MASKS): Likewise.
(OTHER_VSX_VECTOR_MASKS): Likewise.
* config/rs6000/rs6000.c (rs6000_option_override_internal): Call
rs6000_disable_incompatible_switches to validate no type switches
like -mvsx.
(rs6000_incompatible_switch): New function to disallow turning on
other vector options if -mno-vsx, -mno-power8-vector, or
-mno-power9-vector are specified.
[gcc/testsuite]
2017-04-12 Michael Meissner <meiss...@linux.vnet.ibm.com>
PR target/80098
* gcc.target/powerpc/pr80098-1.c: New test.
* gcc.target/powerpc/pr80098-2.c: Likewise.
* gcc.target/powerpc/pr80098-3.c: Likewise.
* gcc.target/powerpc/pr80098-4.c: Likewise.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/rs6000-cpus.def
===================================================================
--- gcc/config/rs6000/rs6000-cpus.def (revision 246826)
+++ gcc/config/rs6000/rs6000-cpus.def (working copy)
@@ -84,6 +84,30 @@
| OPTION_MASK_UPPER_REGS_SF \
| OPTION_MASK_VSX_SMALL_INTEGER)
+/* Flags that need to be turned off if -mno-power9-vector. */
+#define OTHER_P9_VECTOR_MASKS (OPTION_MASK_FLOAT128_HW \
+ | OPTION_MASK_P9_DFORM_SCALAR \
+ | OPTION_MASK_P9_DFORM_VECTOR \
+ | OPTION_MASK_P9_MINMAX)
+
+/* Flags that need to be turned off if -mno-power8-vector. */
+#define OTHER_P8_VECTOR_MASKS (OTHER_P9_VECTOR_MASKS \
+ | OPTION_MASK_P9_VECTOR \
+ | OPTION_MASK_DIRECT_MOVE \
+ | OPTION_MASK_CRYPTO \
+ | OPTION_MASK_UPPER_REGS_SF) \
+
+/* Flags that need to be turned off if -mno-vsx. */
+#define OTHER_VSX_VECTOR_MASKS (OTHER_P8_VECTOR_MASKS \
+ | OPTION_MASK_EFFICIENT_UNALIGNED_VSX \
+ | OPTION_MASK_FLOAT128_KEYWORD \
+ | OPTION_MASK_FLOAT128_TYPE \
+ | OPTION_MASK_P8_VECTOR \
+ | OPTION_MASK_UPPER_REGS_DI \
+ | OPTION_MASK_UPPER_REGS_DF \
+ | OPTION_MASK_VSX_SMALL_INTEGER \
+ | OPTION_MASK_VSX_TIMODE)
+
#define POWERPC_7400_MASK (OPTION_MASK_PPC_GFXOPT | OPTION_MASK_ALTIVEC)
/* Deal with ports that do not have -mstrict-align. */
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 246826)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -1335,6 +1335,7 @@ static void rs6000_print_isa_options (FI
HOST_WIDE_INT);
static void rs6000_print_builtin_options (FILE *, int, const char *,
HOST_WIDE_INT);
+static HOST_WIDE_INT rs6000_disable_incompatible_switches (void);
static enum rs6000_reg_type register_to_reg_type (rtx, bool *);
static bool rs6000_secondary_reload_move (enum rs6000_reg_type,
@@ -3902,6 +3903,7 @@ rs6000_option_override_internal (bool gl
const char *implicit_cpu = OPTION_TARGET_CPU_DEFAULT;
HOST_WIDE_INT set_masks;
+ HOST_WIDE_INT ignore_masks;
int cpu_index;
int tune_index;
struct cl_target_option *main_target_opt
@@ -3967,7 +3969,8 @@ rs6000_option_override_internal (bool gl
#endif
#ifdef OS_MISSING_ALTIVEC
if (OS_MISSING_ALTIVEC)
- set_masks &= ~(OPTION_MASK_ALTIVEC | OPTION_MASK_VSX);
+ set_masks &= ~(OPTION_MASK_ALTIVEC | OPTION_MASK_VSX
+ | OTHER_VSX_VECTOR_MASK);
#endif
/* Don't override by the processor default if given explicitly. */
@@ -4270,11 +4273,15 @@ rs6000_option_override_internal (bool gl
if (TARGET_DEBUG_REG || TARGET_DEBUG_TARGET)
rs6000_print_isa_options (stderr, 0, "before defaults", rs6000_isa_flags);
+ /* Handle explicit -mno-{altivec,vsx,power8-vector,power9-vector} and turn
+ off all of the options that depend on those flags. */
+ ignore_masks = rs6000_disable_incompatible_switches ();
+
/* For the newer switches (vsx, dfp, etc.) set some of the older options,
unless the user explicitly used the -mno-<option> to disable the code. */
if (TARGET_P9_VECTOR || TARGET_MODULO || TARGET_P9_DFORM_SCALAR
|| TARGET_P9_DFORM_VECTOR || TARGET_P9_DFORM_BOTH > 0)
- rs6000_isa_flags |= (ISA_3_0_MASKS_SERVER & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (ISA_3_0_MASKS_SERVER & ~ignore_masks);
else if (TARGET_P9_MINMAX)
{
if (have_cpu)
@@ -4283,15 +4290,14 @@ rs6000_option_override_internal (bool gl
{
/* legacy behavior: allow -mcpu-power9 with certain
capabilities explicitly disabled. */
- rs6000_isa_flags |=
- (ISA_3_0_MASKS_SERVER & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (ISA_3_0_MASKS_SERVER & ~ignore_masks);
/* However, reject this automatic fix if certain
capabilities required for TARGET_P9_MINMAX support
have been explicitly disabled. */
if (((OPTION_MASK_VSX | OPTION_MASK_UPPER_REGS_SF
| OPTION_MASK_UPPER_REGS_DF) & rs6000_isa_flags)
!= (OPTION_MASK_VSX | OPTION_MASK_UPPER_REGS_SF
- | OPTION_MASK_UPPER_REGS_DF))
+ | OPTION_MASK_UPPER_REGS_DF))
error ("-mpower9-minmax incompatible with explicitly disabled
options");
}
else
@@ -4308,21 +4314,21 @@ rs6000_option_override_internal (bool gl
rs6000_isa_flags |= ISA_3_0_MASKS_SERVER;
}
else if (TARGET_P8_VECTOR || TARGET_DIRECT_MOVE || TARGET_CRYPTO)
- rs6000_isa_flags |= (ISA_2_7_MASKS_SERVER & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (ISA_2_7_MASKS_SERVER & ~ignore_masks);
else if (TARGET_VSX)
- rs6000_isa_flags |= (ISA_2_6_MASKS_SERVER & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (ISA_2_6_MASKS_SERVER & ~ignore_masks);
else if (TARGET_POPCNTD)
- rs6000_isa_flags |= (ISA_2_6_MASKS_EMBEDDED & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (ISA_2_6_MASKS_EMBEDDED & ~ignore_masks);
else if (TARGET_DFP)
- rs6000_isa_flags |= (ISA_2_5_MASKS_SERVER & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (ISA_2_5_MASKS_SERVER & ~ignore_masks);
else if (TARGET_CMPB)
- rs6000_isa_flags |= (ISA_2_5_MASKS_EMBEDDED & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (ISA_2_5_MASKS_EMBEDDED & ~ignore_masks);
else if (TARGET_FPRND)
- rs6000_isa_flags |= (ISA_2_4_MASKS & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (ISA_2_4_MASKS & ~ignore_masks);
else if (TARGET_POPCNTB)
- rs6000_isa_flags |= (ISA_2_2_MASKS & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (ISA_2_2_MASKS & ~ignore_masks);
else if (TARGET_ALTIVEC)
- rs6000_isa_flags |= (OPTION_MASK_PPC_GFXOPT & ~rs6000_isa_flags_explicit);
+ rs6000_isa_flags |= (OPTION_MASK_PPC_GFXOPT & ~ignore_masks);
if (TARGET_CRYPTO && !TARGET_ALTIVEC)
{
@@ -39716,6 +39722,68 @@ rs6000_print_builtin_options (FILE *file
ARRAY_SIZE (rs6000_builtin_mask_names));
}
+/* Handle explicit -mno-vsx, -mno-power8-vector, and -mno-power9-vector options
+ and turn off all setting other options by default if those options that
+ depend on the flags that are turned off. */
+
+static HOST_WIDE_INT
+rs6000_disable_incompatible_switches (void)
+{
+ HOST_WIDE_INT ignore_masks = rs6000_isa_flags_explicit;
+ size_t i, j;
+
+ static const struct {
+ const HOST_WIDE_INT no_flag; /* flag explicitly turned off. */
+ const HOST_WIDE_INT dep_flags; /* flags that depend on this option. */
+ const char *const name; /* name of the switch. */
+ } flags[] = {
+ { OPTION_MASK_P9_VECTOR, OTHER_P9_VECTOR_MASKS, "power9-vector" },
+ { OPTION_MASK_P8_VECTOR, OTHER_P8_VECTOR_MASKS, "power8-vector" },
+ { OPTION_MASK_VSX, OTHER_VSX_VECTOR_MASKS, "vsx" },
+ };
+
+ for (i = 0; i < ARRAY_SIZE (flags); i++)
+ {
+ HOST_WIDE_INT no_flag = flags[i].no_flag;
+
+ if ((rs6000_isa_flags & no_flag) == 0
+ && (rs6000_isa_flags_explicit & no_flag) != 0)
+ {
+ HOST_WIDE_INT dep_flags = flags[i].dep_flags;
+ HOST_WIDE_INT set_flags = (rs6000_isa_flags_explicit
+ & rs6000_isa_flags
+ & dep_flags);
+
+ if (set_flags)
+ {
+ for (j = 0; j < ARRAY_SIZE (rs6000_opt_masks); j++)
+ if ((set_flags & rs6000_opt_masks[j].mask) != 0)
+ {
+ set_flags &= ~rs6000_opt_masks[j].mask;
+ error ("-mno-%s turns off -m%s",
+ flags[i].name,
+ rs6000_opt_masks[j].name);
+ }
+
+ gcc_assert (!set_flags);
+ }
+
+ rs6000_isa_flags &= ~dep_flags;
+ ignore_masks |= no_flag | dep_flags;
+ }
+ }
+
+ if (!TARGET_P9_VECTOR
+ && (rs6000_isa_flags_explicit & OPTION_MASK_P9_VECTOR) != 0
+ && TARGET_P9_DFORM_BOTH > 0)
+ {
+ error ("-mno-power9-vector turns off -mpower9-dform");
+ TARGET_P9_DFORM_BOTH = 0;
+ }
+
+ return ignore_masks;
+}
+
/* Hook to determine if one function can safely inline another. */
Index: gcc/testsuite/gcc.target/powerpc/pr80098-3.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80098-3.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80098-3.c (revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { powerpc64*-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power7" } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-mcpu=power7 -mno-vsx -mdirect-move -mcrypto" } */
+
+int i;
+
+/* { dg-error "-mno-vsx turns off -mdirect-move" "PR80098" { target *-*-* } 0
} */
+/* { dg-error "-mno-vsx turns off -mcrypto" "PR80098" { target *-*-* } 0
} */
Index: gcc/testsuite/gcc.target/powerpc/pr80098-4.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80098-4.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80098-4.c (revision 0)
@@ -0,0 +1,8 @@
+/* { dg-do compile { target { powerpc64*-*-* && lp64 } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power7" } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-mcpu=power7 -mno-vsx -mvsx-timode" } */
+
+int i;
+
+/* { dg-error "-mno-vsx turns off -mvsx-timode" "PR80098" { target *-*-* } 0 }
*/
Index: gcc/testsuite/gcc.target/powerpc/pr80098-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80098-1.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80098-1.c (revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { powerpc64*-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power9" } } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-options "-mcpu=power9 -mno-power9-vector -mpower9-minmax
-mpower9-dform" } */
+
+int i;
+
+/* { dg-error "-mno-power9-vector turns off -mpower9-minmax" "PR80098" {
target *-*-* } 0 } */
+/* { dg-error "-mno-power9-vector turns off -mpower9-dform" "PR80098" {
target *-*-* } 0 } */
Index: gcc/testsuite/gcc.target/powerpc/pr80098-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80098-2.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80098-2.c (revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { powerpc64*-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power8" } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-options "-mcpu=power8 -mno-power8-vector -mdirect-move -mcrypto" } */
+
+int i;
+
+/* { dg-error "-mno-power8-vector turns off -mdirect-move" "PR80098" { target
*-*-* } 0 } */
+/* { dg-error "-mno-power8-vector turns off -mcrypto" "PR80098" { target
*-*-* } 0 } */
