GCC Mips has 31 Masks in mips.opt

GCC Mips has 31 Masks in mips.opt

[Steve Ellcey]

Richard,

While experimenting with a local GCC change I added two new Masks to
mips.opt and ran into a build failure about too many masks:

./options.h:4172:2: error: #error too many target masks

It looks like we already have 31 Masks in the MIPS mips.opt file and 32
is the limit.  It looks like the fix for this is to put some of the Masks
in a variable other then target_flags with the Var() syntax.  I see i386
and rs6000 doing this with ix86_isa_flags and rs6000_isa_flags.

Now I could just put my new flags (and any other new flags that come
up) in a separate variable, but I was wondering if we wanted to
move a set of existing Masks to a new variable instead of just using
a first-come first-serve approach to what goes into the default
target_flags and what goes into a new flags variable.

My thought is that by moving some of the existing Masks to a different
variable it would make it easier to add new flags later, especially if
someone is just adding a flag temporarily as an experiment to test
something.

Does this sound reasonable to you?  If so what flags do you think we
might want to move out of target_flags to a different variable?

Steve Ellcey
sellcey@mips.com

Re: GCC Mips has 31 Masks in mips.opt

[H.J. Lu]

On Fri, Jan 24, 2014 at 3:20 PM, Steve Ellcey <sellcey@mips.com> wrote:
> Richard,
>
> While experimenting with a local GCC change I added two new Masks to
> mips.opt and ran into a build failure about too many masks:
>
> ./options.h:4172:2: error: #error too many target masks
>
> It looks like we already have 31 Masks in the MIPS mips.opt file and 32
> is the limit.  It looks like the fix for this is to put some of the Masks
> in a variable other then target_flags with the Var() syntax.  I see i386
> and rs6000 doing this with ix86_isa_flags and rs6000_isa_flags.
>

i386 uses HOST_WIDE_INT for ix86_isa_flags, which
bugs us some time, until we need more than 64 bits.

-- 
H.J.

Re: GCC Mips has 31 Masks in mips.opt

[Richard Sandiford]

"Steve Ellcey " <sellcey@mips.com> writes:
> Richard,
>
> While experimenting with a local GCC change I added two new Masks to
> mips.opt and ran into a build failure about too many masks:
>
> ./options.h:4172:2: error: #error too many target masks
>
> It looks like we already have 31 Masks in the MIPS mips.opt file and 32
> is the limit.  It looks like the fix for this is to put some of the Masks
> in a variable other then target_flags with the Var() syntax.  I see i386
> and rs6000 doing this with ix86_isa_flags and rs6000_isa_flags.
>
> Now I could just put my new flags (and any other new flags that come
> up) in a separate variable, but I was wondering if we wanted to
> move a set of existing Masks to a new variable instead of just using
> a first-come first-serve approach to what goes into the default
> target_flags and what goes into a new flags variable.

Yeah, I've been trying to use separate variables for new flags.
That's why a lot of the MIPS options have discrete TARGET_… variables
(defined via Var(TARGET_…)).  target_flags should only really be needed
for options whose defaults are controlled by config.gcc.

E.g. from a quick look, -mdsp, -mdspr2, -mfp-exceptions,
-mfused-madd and -mips3d don't need to be masks and could easily
be converted to Var(TARGET_…).  That's pre-approved it works.
Others could be moved too with a bit more effort, but hopefully
those 5 will be enough for now.

Thanks,
Richard

Re: GCC Mips has 31 Masks in mips.opt

[Steve Ellcey]

On Sat, 2014-01-25 at 20:34 +0000, Richard Sandiford wrote:

> Yeah, I've been trying to use separate variables for new flags.
> That's why a lot of the MIPS options have discrete TARGET_В… variables
> (defined via Var(TARGET_В…)).  target_flags should only really be needed
> for options whose defaults are controlled by config.gcc.

I guess I could probably have done that for my experiments, but I didn't
think of using Var's instead of Mask's for some reason.  It's probably
a good idea to free up some space anyway.

> E.g. from a quick look, -mdsp, -mdspr2, -mfp-exceptions,
> -mfused-madd and -mips3d don't need to be masks and could easily
> be converted to Var(TARGET_В…).  That's pre-approved it works.
> Others could be moved too with a bit more effort, but hopefully
> those 5 will be enough for now.

I think it would be enough for now.  Here is the patch I came up with
and tested.  I had to tweak a couple of things in
gcc/common/config/mips/mips-common.c so I wouldn't mind if you took a
look at it before I checked it in.  Testing looked all right once I
initialized TARGET_FP_EXCEPTIONS and TARGET_FUSED_MADD to 1.

Steve Ellcey
sellcey@mips.com


2014-01-27  Steve Ellcey  <sellcey@mips.com>

	* common/config/mips/mips-common.c (TARGET_DEFAULT_TARGET_FLAGS):
	Remove TARGET_FP_EXCEPTIONS_DEFAULT and MASK_FUSED_MADD.
	* config/mips/mips.c (mips_option_override): Change setting
	of TARGET_DSP.
	* config/mips/mips.h (TARGET_FP_EXCEPTIONS_DEFAULT): Remove.
	* config/mips/mips.opt (DSP, DSPR2, FP_EXCEPTIONS, FUSED_MADD,
	MIPS3D) Change from Mask to Var.

diff --git a/gcc/common/config/mips/mips-common.c b/gcc/common/config/mips/mips-common.c
index cece4ae..7dd8d2d 100644
--- a/gcc/common/config/mips/mips-common.c
+++ b/gcc/common/config/mips/mips-common.c
@@ -62,9 +62,7 @@ static const struct default_options mips_option_optimization_table[] =
   (TARGET_DEFAULT				\
    | TARGET_CPU_DEFAULT				\
    | TARGET_ENDIAN_DEFAULT			\
-   | TARGET_FP_EXCEPTIONS_DEFAULT		\
-   | MASK_CHECK_ZERO_DIV			\
-   | MASK_FUSED_MADD)
+   | MASK_CHECK_ZERO_DIV)
 #undef TARGET_HANDLE_OPTION
 #define TARGET_HANDLE_OPTION mips_handle_option
 
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 5bad0f8..0726830 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -17084,9 +17084,9 @@ mips_option_override (void)
       mips_r10k_cache_barrier = R10K_CACHE_BARRIER_NONE;
     }
 
-  /* If TARGET_DSPR2, enable MASK_DSP.  */
+  /* If TARGET_DSPR2, enable TARGET_DSP.  */
   if (TARGET_DSPR2)
-    target_flags |= MASK_DSP;
+    TARGET_DSP = TARGET_DSPR2;
 
   /* .eh_frame addresses should be the same width as a C pointer.
      Most MIPS ABIs support only one pointer size, so the assembler
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index bc9d301..150cf0c 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -588,10 +588,6 @@ struct mips_cpu_info {
 #define TARGET_ENDIAN_DEFAULT MASK_BIG_ENDIAN
 #endif
 
-#ifndef TARGET_FP_EXCEPTIONS_DEFAULT
-#define TARGET_FP_EXCEPTIONS_DEFAULT MASK_FP_EXCEPTIONS
-#endif
-
 #ifdef IN_LIBGCC2
 #undef TARGET_64BIT
 /* Make this compile time constant for libgcc2 */
diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt
index 877ec59..be551a7 100644
--- a/gcc/config/mips/mips.opt
+++ b/gcc/config/mips/mips.opt
@@ -116,11 +116,11 @@ Target Report RejectNegative InverseMask(SINGLE_FLOAT, DOUBLE_FLOAT)
 Allow hardware floating-point instructions to cover both 32-bit and 64-bit operations
 
 mdsp
-Target Report Mask(DSP)
+Target Report Var(TARGET_DSP)
 Use MIPS-DSP instructions
 
 mdspr2
-Target Report Mask(DSPR2)
+Target Report Var(TARGET_DSPR2)
 Use MIPS-DSP REV 2 instructions
 
 mdebug
@@ -190,7 +190,7 @@ Target Report Var(TARGET_4300_MUL_FIX)
 Work around an early 4300 hardware bug
 
 mfp-exceptions
-Target Report Mask(FP_EXCEPTIONS)
+Target Report Var(TARGET_FP_EXCEPTIONS) Init(1)
 FP exceptions are enabled
 
 mfp32
@@ -206,7 +206,7 @@ Target RejectNegative Joined Var(mips_cache_flush_func) Init(CACHE_FLUSH_FUNC)
 -mflush-func=FUNC	Use FUNC to flush the cache before calling stack trampolines
 
 mfused-madd
-Target Report Mask(FUSED_MADD)
+Target Report Var(TARGET_FUSED_MADD) Init(1)
 Generate floating-point multiply-add instructions
 
 mabs=
@@ -264,7 +264,7 @@ Target Report RejectNegative Mask(MIPS16)
 Generate MIPS16 code
 
 mips3d
-Target Report RejectNegative Mask(MIPS3D)
+Target Report RejectNegative Var(TARGET_MIPS3D)
 Use MIPS-3D instructions
 
 mllsc
@@ -324,7 +324,7 @@ Target Report RejectNegative InverseMask(MIPS16)
 Generate normal-mode code
 
 mno-mips3d
-Target Report RejectNegative InverseMask(MIPS3D)
+Target Report RejectNegative Var(TARGET_MIPS3D, 0)
 Do not use MIPS-3D instructions
 
 mpaired-single

Re: GCC Mips has 31 Masks in mips.opt

[Richard Sandiford]

Steve Ellcey <sellcey@mips.com> writes:
> On Sat, 2014-01-25 at 20:34 +0000, Richard Sandiford wrote:
>
>> Yeah, I've been trying to use separate variables for new flags.
>> That's why a lot of the MIPS options have discrete TARGET_… variables
>> (defined via Var(TARGET_…)).  target_flags should only really be needed
>> for options whose defaults are controlled by config.gcc.
>
> I guess I could probably have done that for my experiments, but I didn't
> think of using Var's instead of Mask's for some reason.  It's probably
> a good idea to free up some space anyway.
>
>> E.g. from a quick look, -mdsp, -mdspr2, -mfp-exceptions,
>> -mfused-madd and -mips3d don't need to be masks and could easily
>> be converted to Var(TARGET_…).  That's pre-approved it works.
>> Others could be moved too with a bit more effort, but hopefully
>> those 5 will be enough for now.
>
> I think it would be enough for now.  Here is the patch I came up with
> and tested.  I had to tweak a couple of things in
> gcc/common/config/mips/mips-common.c so I wouldn't mind if you took a
> look at it before I checked it in.  Testing looked all right once I
> initialized TARGET_FP_EXCEPTIONS and TARGET_FUSED_MADD to 1.

Looks good, although:

> @@ -17084,9 +17084,9 @@ mips_option_override (void)
>        mips_r10k_cache_barrier = R10K_CACHE_BARRIER_NONE;
>      }
>  
> -  /* If TARGET_DSPR2, enable MASK_DSP.  */
> +  /* If TARGET_DSPR2, enable TARGET_DSP.  */
>    if (TARGET_DSPR2)
> -    target_flags |= MASK_DSP;
> +    TARGET_DSP = TARGET_DSPR2;

I'd prefer "= true" here, in case we ever use -1 as an initial "not set"
value.  No need for a full retest for that change, spot checking would be
fine IMO.

Thanks,
Richard