Re: patch: powerpc .opt support (unfinished)

[Richard Sandiford <rsandifo at redhat dot com>]

Thanks for doing this.  For the record: I've just looked at how you're
using the .opt machinery.  No comments at all on the rs6000 specifics
(which you know much better than me) or the impact of changing the stuff
in config/*.

Anyway, it looks good to me.  A few comments:

Aldy Hernandez <aldyh@redhat.com> writes:
> + mshared
> + Target RejectNegative Var(internal_ignored_lynx_shared)
> + Use shared libraries

There's no need for this Var(internal_ignored...) stuff.  It's OK to
have stateless options:

     mshared
     Target RejectNegative
     Use shared libraries

Similarly elsewhere.

> Index: config/frv/frv.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/config/frv/frv.c,v
> retrieving revision 1.85
> diff -c -p -r1.85 frv.c
> *** config/frv/frv.c	22 Mar 2005 19:19:17 -0000	1.85
> --- config/frv/frv.c	5 Apr 2005 13:23:18 -0000
> *************** static int frv_arg_partial_bytes (CUMULA
> *** 437,442 ****
> --- 437,443 ----
>   #undef TARGET_MACHINE_DEPENDENT_REORG
>   #define TARGET_MACHINE_DEPENDENT_REORG frv_reorg
>   
> + #define TARGET_HAVE_TLS 1
>   struct gcc_target targetm = TARGET_INITIALIZER;
>   
>   #define FRV_SYMBOL_REF_TLS_P(RTX) \

(Unrelated hunk... just a heads-up)

> *************** rs6000_override_options (const char *def
> *** 1229,1234 ****
> --- 1194,1213 ----
>   	}
>       }
>   
> +   /* Fix-up leftovers from .opt handling.  */
> +   {
> +     /* FIXME: free some bits in target_flags, and re-do this.  */
> +     if (INTERNAL_TARGET_NO_POWER)
> +       target_flags &= ~(MASK_POWER | MASK_POWER2
> + 			| MASK_MULTIPLE | MASK_STRING);
> +     if (INTERNAL_TARGET_NO_POWERPC)
> +       target_flags &= ~(MASK_POWERPC | MASK_PPC_GPOPT
> + 			| MASK_PPC_GFXOPT | MASK_POWERPC64);
> +     if (INTERNAL_TARGET_FULL_TOC)
> +       target_flags &= ~(MASK_NO_FP_IN_TOC | MASK_NO_SUM_IN_TOC
> + 			| MASK_MINIMAL_TOC);
> +   }
> + 

Rather than add this INTERNAL_TARGET_* stuff, it's probably better
to clear the bits in handle_option, a bit like you do for the other
multi-mask options.  (Again, not every option needs a variable or mask
associated with it.)  This also keeps compatibility with the current
behaviour, which is to clear the flags when the option is encountered.

> + #if 0
> +       /* FIXME: for .opt stuff.  We have no way to know wether an
> + 	 option was set specifically-- something akin to
> + 	 target_flags_explicit.  */
>         if (rs6000_abi_string == 0)
>   	rs6000_spe_abi = 0;
>         if (rs6000_spe_string == 0)

This is handling strings, not target masks, so I'm not sure
that target_flags_explicit comes into it.  Can't you just make
handle_option record whether -mabi, etc., was used?

FWIW, there's no reason why you can't keep rs6000_abi_string if you
want to.  AFAIK, the options parser is supposed to guarantee that the
argument passed to handle_option persists to the end of compilation,
so you could just have:

      case OPT_mabi_:
        [...checks...]
        rs6000_abi_string = arg;
        return true;

> + ; FIXME: Duplicate
> + ;msched-epilog
> + ;Target Undocumented Mask(SCHED_PROLOG)

You can now use MaskExists for this.

Richard