public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful)
@ 2010-11-05  2:35 Joseph S. Myers
  2010-11-05 10:41 ` Richard Guenther
  2010-11-05 13:08 ` Jack Howarth
  0 siblings, 2 replies; 9+ messages in thread
From: Joseph S. Myers @ 2010-11-05  2:35 UTC (permalink / raw)
  To: gcc-patches

Continuing the ARM-sponsored series of option-handling cleanup patches
preparing for the multilib selection changes described at
<http://gcc.gnu.org/ml/gcc/2010-01/msg00063.html> (which changes to
multilib selection itself will now clearly need to wait for 4.7), this
71st patch in the series completes the removal of
WORD_SWITCH_TAKES_ARG.

After patch 70
<http://gcc.gnu.org/ml/gcc-patches/2010-10/msg02373.html> the only
remaining definition of this macro was for Darwin, because
WORD_SWITCH_TAKES_ARG was the only way of describing an option as
taking more than one argument and Darwin has such options (passed to
the linker by specs).  This patch adds a facility to .opt files for
options to take more than one separate argument and describes the
Darwin options in the .opt file accordingly.  (I do not expect any new
options to use this facility, as multi-argument options seem
confusingly different from normal practice for options.)

I added a two-bit field to the option flags to handle options with 1-4
arguments.  A range of 0-3, by making CL_SEPARATE into a two-bit field
rather than using a separate field, would have sufficed, but making
CL_SEPARATE into a two-bit field would have involved more complicated
changes to --help handling that presumes flags are single bits; with
this patch the option-handling changes are safer and more
self-contained.

We are however quite close to running out of bits in the flags field;
bits 0-7 are used for languages, and 11-30 for other flags, and the Go
front end will cause bits 0-8 to be used for languages.  That leaves
just two bits spare for languages, and I know that Debian at least
builds the compiler with driver support for two extra languages
(Pascal and D); I do not know if any distributors are building with
three or more extra languages.  It shouldn't be hard to use bit 31
(making flags consistently unsigned), but beyond that it will be
necessary to split up flags in some way (maybe making those above
CL_MAX_OPTION_CLASS, or at least those not relevant to --help, into
bit-fields rather than allocating particular bits explicitly).

Bootstrapped with no regressions on x86_64-unknown-linux-gnu.  I also
tested building xgcc for a cross to i686-darwin and examined the
decoded options under GDB, but it might be useful for someone to run
tests on Darwin (though I suspect a bootstrap and testsuite run may
not exercise the affected options).  OK to commit?

2010-11-04  Joseph Myers  <joseph@codesourcery.com>

	* defaults.h (DEFAULT_WORD_SWITCH_TAKES_ARG,
	WORD_SWITCH_TAKES_ARG): Remove.
	* doc/options.texi (Args): Document.
	* doc/tm.texi.in (WORD_SWITCH_TAKES_ARG): Remove.
	* doc/tm.texi: Regenerate.
	* opt-functions.awk (switch_flags): Handle Args.
	* opts-common.c: Update comment on tm.h include.
	(decode_cmdline_option): Handle options with multiple arguments.
	Don't check WORD_SWITCH_TAKES_ARG for unknown options.
	* opts.h (CL_SEPARATE_NARGS_SHIFT, CL_SEPARATE_NARGS_MASK):
	Define.
	(CL_PARAMS, CL_WARNING, CL_OPTIMIZATION, CL_DRIVER, CL_TARGET,
	CL_COMMON): Update values.
	* system.h (WORD_SWITCH_TAKES_ARG): Poison.
	* config/darwin.h (WORD_SWITCH_TAKES_ARG): Remove.
	* config/darwin.opt (Zsegaddr, sectalign, sectcreate,
	sectobjectsymbols, sectorder, segcreate, segprot): New.

Index: gcc/doc/options.texi
===================================================================
--- gcc/doc/options.texi	(revision 166309)
+++ gcc/doc/options.texi	(working copy)
@@ -162,6 +162,10 @@ generic error message is used.  @var{mes
 @samp{%qs} format, which will be used to format the name of the option
 passed.
 
+@item Args(@var{n})
+For an option marked @code{Separate}, indicate that it takes @var{n}
+arguments.  The default is 1.
+
 @item UInteger
 The option's argument is a non-negative integer.  The option parser
 will check and convert the argument before passing it to the relevant
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 166309)
+++ gcc/doc/tm.texi	(working copy)
@@ -99,21 +99,6 @@ from being defined in the @file{.h} file
 @c prevent bad page break with this line
 You can control the compilation driver.
 
-@defmac WORD_SWITCH_TAKES_ARG (@var{name})
-A C expression which determines whether the option @option{-@var{name}}
-takes arguments.  The value should be the number of arguments that
-option takes--zero, for many options.
-This macro does not need to handle options defined in @file{.opt}
-files, only those that are handled purely through specs.
-
-By default, this macro is defined as
-@code{DEFAULT_WORD_SWITCH_TAKES_ARG}, which handles the standard options
-properly.  You need not define @code{WORD_SWITCH_TAKES_ARG} unless you
-wish to add additional options which take arguments.  Any redefinition
-should call @code{DEFAULT_WORD_SWITCH_TAKES_ARG} and then check for
-additional options.
-@end defmac
-
 @defmac TARGET_OPTION_TRANSLATE_TABLE
 If defined, a list of pairs of strings, the first of which is a
 potential command line target to the @file{gcc} driver program, and the
Index: gcc/doc/tm.texi.in
===================================================================
--- gcc/doc/tm.texi.in	(revision 166309)
+++ gcc/doc/tm.texi.in	(working copy)
@@ -99,21 +99,6 @@ from being defined in the @file{.h} file
 @c prevent bad page break with this line
 You can control the compilation driver.
 
-@defmac WORD_SWITCH_TAKES_ARG (@var{name})
-A C expression which determines whether the option @option{-@var{name}}
-takes arguments.  The value should be the number of arguments that
-option takes--zero, for many options.
-This macro does not need to handle options defined in @file{.opt}
-files, only those that are handled purely through specs.
-
-By default, this macro is defined as
-@code{DEFAULT_WORD_SWITCH_TAKES_ARG}, which handles the standard options
-properly.  You need not define @code{WORD_SWITCH_TAKES_ARG} unless you
-wish to add additional options which take arguments.  Any redefinition
-should call @code{DEFAULT_WORD_SWITCH_TAKES_ARG} and then check for
-additional options.
-@end defmac
-
 @defmac TARGET_OPTION_TRANSLATE_TABLE
 If defined, a list of pairs of strings, the first of which is a
 potential command line target to the @file{gcc} driver program, and the
Index: gcc/opts-common.c
===================================================================
--- gcc/opts-common.c	(revision 166309)
+++ gcc/opts-common.c	(working copy)
@@ -24,8 +24,7 @@ along with GCC; see the file COPYING3.  
 #include "opts.h"
 #include "flags.h"
 #include "diagnostic.h"
-#include "tm.h" /* For WORD_SWITCH_TAKES_ARG and
-		   TARGET_OPTION_TRANSLATE_TABLE.  */
+#include "tm.h" /* For TARGET_OPTION_TRANSLATE_TABLE.  */
 
 static void prune_options (struct cl_decoded_option **, unsigned int *);
 
@@ -288,7 +287,7 @@ decode_cmdline_option (const char **argv
   size_t opt_index;
   const char *arg = 0;
   int value = 1;
-  unsigned int result = 1, i, extra_args;
+  unsigned int result = 1, i, extra_args, separate_args;
   int adjust_len = 0;
   size_t total_len;
   char *p;
@@ -366,10 +365,15 @@ decode_cmdline_option (const char **argv
     errors |= CL_ERR_DISABLED;
 
   /* Determine whether there may be a separate argument based on
-     whether this option is being processed for the driver.  */
+     whether this option is being processed for the driver, and, if
+     so, how many such arguments.  */
   separate_arg_flag = ((option->flags & CL_SEPARATE)
 		       && !((option->flags & CL_NO_DRIVER_ARG)
 			    && (lang_mask & CL_DRIVER)));
+  separate_args = (separate_arg_flag
+		   ? ((option->flags & CL_SEPARATE_NARGS_MASK)
+		      >> CL_SEPARATE_NARGS_SHIFT) + 1
+		   : 0);
   joined_arg_flag = (option->flags & CL_JOINED) != 0;
 
   /* Sort out any argument the switch takes.  */
@@ -399,10 +403,14 @@ decode_cmdline_option (const char **argv
   else if (separate_arg_flag)
     {
       arg = argv[extra_args + 1];
-      result = extra_args + 2;
-      if (arg == NULL)
-	result = extra_args + 1;
-      else
+      for (i = 0; i < separate_args; i++)
+	if (argv[extra_args + 1 + i] == NULL)
+	  {
+	    errors |= CL_ERR_MISSING_ARG;
+	    break;
+	  }
+      result = extra_args + 1 + i;
+      if (arg != NULL)
 	have_separate_arg = true;
     }
 
@@ -461,6 +469,11 @@ decode_cmdline_option (const char **argv
 				    && (lang_mask & CL_DRIVER)));
 	  joined_arg_flag = (option->flags & CL_JOINED) != 0;
 
+	  if (separate_args > 1 || (option->flags & CL_SEPARATE_NARGS_MASK))
+	    gcc_assert (separate_args
+			== ((option->flags & CL_SEPARATE_NARGS_MASK)
+			    >> CL_SEPARATE_NARGS_SHIFT) + 1);
+
 	  if (!(errors & CL_ERR_MISSING_ARG))
 	    {
 	      if (separate_arg_flag || joined_arg_flag)
@@ -504,22 +517,7 @@ decode_cmdline_option (const char **argv
   decoded->warn_message = warn_message;
 
   if (opt_index == OPT_SPECIAL_unknown)
-    {
-      /* Skip the correct number of arguments for options handled
-	 through specs.  */
-      const char *popt ATTRIBUTE_UNUSED = argv[0] + 1;
-
-      gcc_assert (result == 1);
-      if (WORD_SWITCH_TAKES_ARG (popt))
-	result += WORD_SWITCH_TAKES_ARG (popt);
-      if (result > 1)
-	for (i = 1; i < result; i++)
-	  if (argv[i] == NULL)
-	    {
-	      result = i;
-	      break;
-	    }
-    }
+    gcc_assert (result == 1);
 
   gcc_assert (result >= 1 && result <= ARRAY_SIZE (decoded->canonical_option));
   decoded->canonical_option_num_elements = result;
@@ -538,7 +536,21 @@ decode_cmdline_option (const char **argv
 	decoded->canonical_option[i] = NULL;
     }
   if (opt_index != OPT_SPECIAL_unknown && opt_index != OPT_SPECIAL_ignore)
-    generate_canonical_option (opt_index, arg, value, decoded);
+    {
+      generate_canonical_option (opt_index, arg, value, decoded);
+      if (separate_args > 1)
+	{
+	  for (i = 0; i < separate_args; i++)
+	    {
+	      if (argv[extra_args + 1 + i] == NULL)
+		  break;
+	      else
+		decoded->canonical_option[1 + i] = argv[extra_args + 1 + i];
+	    }
+	  gcc_assert (result == 1 + i);
+	  decoded->canonical_option_num_elements = result;
+	}
+    }
   decoded->orig_option_with_args_text = p = XNEWVEC (char, total_len);
   for (i = 0; i < result; i++)
     {
Index: gcc/defaults.h
===================================================================
--- gcc/defaults.h	(revision 166309)
+++ gcc/defaults.h	(working copy)
@@ -32,14 +32,6 @@ see the files COPYING3 and COPYING.RUNTI
 #define GET_ENVIRONMENT(VALUE, NAME) do { (VALUE) = getenv (NAME); } while (0)
 #endif
 
-/* This defines which multi-letter switches take arguments.  */
-
-#define DEFAULT_WORD_SWITCH_TAKES_ARG(STR)		0
-
-#ifndef WORD_SWITCH_TAKES_ARG
-#define WORD_SWITCH_TAKES_ARG(STR) DEFAULT_WORD_SWITCH_TAKES_ARG (STR)
-#endif
-
 /* Store in OUTPUT a string (made with alloca) containing an
    assembler-name for a local static variable or function named NAME.
    LABELNO is an integer which is different for each call.  */
Index: gcc/opts.h
===================================================================
--- gcc/opts.h	(revision 166309)
+++ gcc/opts.h	(working copy)
@@ -71,12 +71,12 @@ extern const unsigned int cl_options_cou
 extern const char *const lang_names[];
 extern const unsigned int cl_lang_count;
 
-#define CL_PARAMS               (1 << 13) /* Fake entry.  Used to display --param info with --help.  */
-#define CL_WARNING		(1 << 14) /* Enables an (optional) warning message.  */
-#define CL_OPTIMIZATION		(1 << 15) /* Enables an (optional) optimization.  */
-#define CL_DRIVER		(1 << 16) /* Driver option.  */
-#define CL_TARGET		(1 << 17) /* Target-specific option.  */
-#define CL_COMMON		(1 << 18) /* Language-independent.  */
+#define CL_PARAMS               (1 << 11) /* Fake entry.  Used to display --param info with --help.  */
+#define CL_WARNING		(1 << 12) /* Enables an (optional) warning message.  */
+#define CL_OPTIMIZATION		(1 << 13) /* Enables an (optional) optimization.  */
+#define CL_DRIVER		(1 << 14) /* Driver option.  */
+#define CL_TARGET		(1 << 15) /* Target-specific option.  */
+#define CL_COMMON		(1 << 16) /* Language-independent.  */
 
 #define CL_MIN_OPTION_CLASS	CL_PARAMS
 #define CL_MAX_OPTION_CLASS	CL_COMMON
@@ -86,6 +86,11 @@ extern const unsigned int cl_lang_count;
    This distinction is important because --help will not list options
    which only have these higher bits set.  */
 
+/* Options marked with CL_SEPARATE take a number of separate arguments
+   (1 to 4) that is one more than the number in this bit-field.  */
+#define CL_SEPARATE_NARGS_SHIFT	17
+#define CL_SEPARATE_NARGS_MASK	(3 << CL_SEPARATE_NARGS_SHIFT)
+
 #define CL_SEPARATE_ALIAS	(1 << 19) /* Option is an alias when used with separate argument.  */
 #define CL_NO_DRIVER_ARG	(1 << 20) /* Option takes no argument in the driver.  */
 #define CL_REJECT_DRIVER	(1 << 21) /* Reject this option in the driver.  */
Index: gcc/system.h
===================================================================
--- gcc/system.h	(revision 166309)
+++ gcc/system.h	(working copy)
@@ -776,7 +776,7 @@ extern void fancy_abort (const char *, i
 	STACK_CHECK_PROBE_INTERVAL STACK_CHECK_PROBE_LOAD		   \
 	ORDER_REGS_FOR_LOCAL_ALLOC FUNCTION_OUTGOING_VALUE		   \
 	ASM_DECLARE_CONSTANT_NAME MODIFY_TARGET_NAME SWITCHES_NEED_SPACES  \
-	SWITCH_CURTAILS_COMPILATION SWITCH_TAKES_ARG
+	SWITCH_CURTAILS_COMPILATION SWITCH_TAKES_ARG WORD_SWITCH_TAKES_ARG
 
 /* Hooks that are no longer used.  */
  #pragma GCC poison LANG_HOOKS_FUNCTION_MARK LANG_HOOKS_FUNCTION_FREE	\
Index: gcc/opt-functions.awk
===================================================================
--- gcc/opt-functions.awk	(revision 166309)
+++ gcc/opt-functions.awk	(working copy)
@@ -100,6 +100,12 @@ function switch_flags (flags)
 	  test_flag("Warning", flags,  " | CL_WARNING") \
 	  test_flag("Optimization", flags,  " | CL_OPTIMIZATION") \
 	  test_flag("Report", flags, " | CL_REPORT")
+	sep_args = opt_args("Args", flags)
+	if (sep_args != "") {
+		sep_args--
+		result = result " | (" sep_args \
+		    " << CL_SEPARATE_NARGS_SHIFT)"
+	}
 	sub( "^0 \\| ", "", result )
 	return result
 }
Index: gcc/config/darwin.opt
===================================================================
--- gcc/config/darwin.opt	(revision 166309)
+++ gcc/config/darwin.opt	(working copy)
@@ -96,6 +96,9 @@ Driver Separate
 Zseg_addr_table
 Driver Separate
 
+Zsegaddr
+Driver Separate Args(2)
+
 Zsegs_read_only_addr
 Driver Separate
 
@@ -129,9 +132,27 @@ Driver Separate
 read_only_relocs
 Driver Separate
 
+sectalign
+Driver Separate Args(3)
+
+sectcreate
+Driver Separate Args(3)
+
+sectobjectsymbols
+Driver Separate Args(2)
+
+sectorder
+Driver Separate Args(3)
+
 seg1addr
 Driver Separate
 
+segcreate
+Driver Separate Args(3)
+
+segprot
+Driver Separate Args(3)
+
 segs_read_only_addr
 Driver Separate
 
Index: gcc/config/darwin.h
===================================================================
--- gcc/config/darwin.h	(revision 166309)
+++ gcc/config/darwin.h	(working copy)
@@ -194,20 +194,6 @@ extern GTY(()) int darwin_ms_struct;
     darwin_override_options ();						\
   } while (0)
 
-/* These compiler options take n arguments.  */
-
-#undef  WORD_SWITCH_TAKES_ARG
-#define WORD_SWITCH_TAKES_ARG(STR)              \
-  (DEFAULT_WORD_SWITCH_TAKES_ARG (STR) ? 1 :    \
-   !strcmp (STR, "sectcreate") ? 3 :            \
-   !strcmp (STR, "sectorder") ? 3 :             \
-   !strcmp (STR, "Zsegaddr") ? 2 :              \
-   !strcmp (STR, "segprot") ? 3 :               \
-   !strcmp (STR, "sectalign") ? 3 :             \
-   !strcmp (STR, "sectobjectsymbols") ? 2 :     \
-   !strcmp (STR, "segcreate") ? 3 :             \
-   0)
-
 #define SUBTARGET_C_COMMON_OVERRIDE_OPTIONS do {                        \
     if (flag_mkernel || flag_apple_kext)				\
       {									\

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful)
  2010-11-05  2:35 Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful) Joseph S. Myers
@ 2010-11-05 10:41 ` Richard Guenther
  2010-11-05 13:08 ` Jack Howarth
  1 sibling, 0 replies; 9+ messages in thread
From: Richard Guenther @ 2010-11-05 10:41 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: gcc-patches

On Fri, Nov 5, 2010 at 2:15 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> Continuing the ARM-sponsored series of option-handling cleanup patches
> preparing for the multilib selection changes described at
> <http://gcc.gnu.org/ml/gcc/2010-01/msg00063.html> (which changes to
> multilib selection itself will now clearly need to wait for 4.7), this
> 71st patch in the series completes the removal of
> WORD_SWITCH_TAKES_ARG.
>
> After patch 70
> <http://gcc.gnu.org/ml/gcc-patches/2010-10/msg02373.html> the only
> remaining definition of this macro was for Darwin, because
> WORD_SWITCH_TAKES_ARG was the only way of describing an option as
> taking more than one argument and Darwin has such options (passed to
> the linker by specs).  This patch adds a facility to .opt files for
> options to take more than one separate argument and describes the
> Darwin options in the .opt file accordingly.  (I do not expect any new
> options to use this facility, as multi-argument options seem
> confusingly different from normal practice for options.)
>
> I added a two-bit field to the option flags to handle options with 1-4
> arguments.  A range of 0-3, by making CL_SEPARATE into a two-bit field
> rather than using a separate field, would have sufficed, but making
> CL_SEPARATE into a two-bit field would have involved more complicated
> changes to --help handling that presumes flags are single bits; with
> this patch the option-handling changes are safer and more
> self-contained.
>
> We are however quite close to running out of bits in the flags field;
> bits 0-7 are used for languages, and 11-30 for other flags, and the Go
> front end will cause bits 0-8 to be used for languages.  That leaves
> just two bits spare for languages, and I know that Debian at least
> builds the compiler with driver support for two extra languages
> (Pascal and D); I do not know if any distributors are building with
> three or more extra languages.  It shouldn't be hard to use bit 31
> (making flags consistently unsigned), but beyond that it will be
> necessary to split up flags in some way (maybe making those above
> CL_MAX_OPTION_CLASS, or at least those not relevant to --help, into
> bit-fields rather than allocating particular bits explicitly).
>
> Bootstrapped with no regressions on x86_64-unknown-linux-gnu.  I also
> tested building xgcc for a cross to i686-darwin and examined the
> decoded options under GDB, but it might be useful for someone to run
> tests on Darwin (though I suspect a bootstrap and testsuite run may
> not exercise the affected options).  OK to commit?

Ok.

Thanks,
Richard.

> 2010-11-04  Joseph Myers  <joseph@codesourcery.com>
>
>        * defaults.h (DEFAULT_WORD_SWITCH_TAKES_ARG,
>        WORD_SWITCH_TAKES_ARG): Remove.
>        * doc/options.texi (Args): Document.
>        * doc/tm.texi.in (WORD_SWITCH_TAKES_ARG): Remove.
>        * doc/tm.texi: Regenerate.
>        * opt-functions.awk (switch_flags): Handle Args.
>        * opts-common.c: Update comment on tm.h include.
>        (decode_cmdline_option): Handle options with multiple arguments.
>        Don't check WORD_SWITCH_TAKES_ARG for unknown options.
>        * opts.h (CL_SEPARATE_NARGS_SHIFT, CL_SEPARATE_NARGS_MASK):
>        Define.
>        (CL_PARAMS, CL_WARNING, CL_OPTIMIZATION, CL_DRIVER, CL_TARGET,
>        CL_COMMON): Update values.
>        * system.h (WORD_SWITCH_TAKES_ARG): Poison.
>        * config/darwin.h (WORD_SWITCH_TAKES_ARG): Remove.
>        * config/darwin.opt (Zsegaddr, sectalign, sectcreate,
>        sectobjectsymbols, sectorder, segcreate, segprot): New.
>
> Index: gcc/doc/options.texi
> ===================================================================
> --- gcc/doc/options.texi        (revision 166309)
> +++ gcc/doc/options.texi        (working copy)
> @@ -162,6 +162,10 @@ generic error message is used.  @var{mes
>  @samp{%qs} format, which will be used to format the name of the option
>  passed.
>
> +@item Args(@var{n})
> +For an option marked @code{Separate}, indicate that it takes @var{n}
> +arguments.  The default is 1.
> +
>  @item UInteger
>  The option's argument is a non-negative integer.  The option parser
>  will check and convert the argument before passing it to the relevant
> Index: gcc/doc/tm.texi
> ===================================================================
> --- gcc/doc/tm.texi     (revision 166309)
> +++ gcc/doc/tm.texi     (working copy)
> @@ -99,21 +99,6 @@ from being defined in the @file{.h} file
>  @c prevent bad page break with this line
>  You can control the compilation driver.
>
> -@defmac WORD_SWITCH_TAKES_ARG (@var{name})
> -A C expression which determines whether the option @option{-@var{name}}
> -takes arguments.  The value should be the number of arguments that
> -option takes--zero, for many options.
> -This macro does not need to handle options defined in @file{.opt}
> -files, only those that are handled purely through specs.
> -
> -By default, this macro is defined as
> -@code{DEFAULT_WORD_SWITCH_TAKES_ARG}, which handles the standard options
> -properly.  You need not define @code{WORD_SWITCH_TAKES_ARG} unless you
> -wish to add additional options which take arguments.  Any redefinition
> -should call @code{DEFAULT_WORD_SWITCH_TAKES_ARG} and then check for
> -additional options.
> -@end defmac
> -
>  @defmac TARGET_OPTION_TRANSLATE_TABLE
>  If defined, a list of pairs of strings, the first of which is a
>  potential command line target to the @file{gcc} driver program, and the
> Index: gcc/doc/tm.texi.in
> ===================================================================
> --- gcc/doc/tm.texi.in  (revision 166309)
> +++ gcc/doc/tm.texi.in  (working copy)
> @@ -99,21 +99,6 @@ from being defined in the @file{.h} file
>  @c prevent bad page break with this line
>  You can control the compilation driver.
>
> -@defmac WORD_SWITCH_TAKES_ARG (@var{name})
> -A C expression which determines whether the option @option{-@var{name}}
> -takes arguments.  The value should be the number of arguments that
> -option takes--zero, for many options.
> -This macro does not need to handle options defined in @file{.opt}
> -files, only those that are handled purely through specs.
> -
> -By default, this macro is defined as
> -@code{DEFAULT_WORD_SWITCH_TAKES_ARG}, which handles the standard options
> -properly.  You need not define @code{WORD_SWITCH_TAKES_ARG} unless you
> -wish to add additional options which take arguments.  Any redefinition
> -should call @code{DEFAULT_WORD_SWITCH_TAKES_ARG} and then check for
> -additional options.
> -@end defmac
> -
>  @defmac TARGET_OPTION_TRANSLATE_TABLE
>  If defined, a list of pairs of strings, the first of which is a
>  potential command line target to the @file{gcc} driver program, and the
> Index: gcc/opts-common.c
> ===================================================================
> --- gcc/opts-common.c   (revision 166309)
> +++ gcc/opts-common.c   (working copy)
> @@ -24,8 +24,7 @@ along with GCC; see the file COPYING3.
>  #include "opts.h"
>  #include "flags.h"
>  #include "diagnostic.h"
> -#include "tm.h" /* For WORD_SWITCH_TAKES_ARG and
> -                  TARGET_OPTION_TRANSLATE_TABLE.  */
> +#include "tm.h" /* For TARGET_OPTION_TRANSLATE_TABLE.  */
>
>  static void prune_options (struct cl_decoded_option **, unsigned int *);
>
> @@ -288,7 +287,7 @@ decode_cmdline_option (const char **argv
>   size_t opt_index;
>   const char *arg = 0;
>   int value = 1;
> -  unsigned int result = 1, i, extra_args;
> +  unsigned int result = 1, i, extra_args, separate_args;
>   int adjust_len = 0;
>   size_t total_len;
>   char *p;
> @@ -366,10 +365,15 @@ decode_cmdline_option (const char **argv
>     errors |= CL_ERR_DISABLED;
>
>   /* Determine whether there may be a separate argument based on
> -     whether this option is being processed for the driver.  */
> +     whether this option is being processed for the driver, and, if
> +     so, how many such arguments.  */
>   separate_arg_flag = ((option->flags & CL_SEPARATE)
>                       && !((option->flags & CL_NO_DRIVER_ARG)
>                            && (lang_mask & CL_DRIVER)));
> +  separate_args = (separate_arg_flag
> +                  ? ((option->flags & CL_SEPARATE_NARGS_MASK)
> +                     >> CL_SEPARATE_NARGS_SHIFT) + 1
> +                  : 0);
>   joined_arg_flag = (option->flags & CL_JOINED) != 0;
>
>   /* Sort out any argument the switch takes.  */
> @@ -399,10 +403,14 @@ decode_cmdline_option (const char **argv
>   else if (separate_arg_flag)
>     {
>       arg = argv[extra_args + 1];
> -      result = extra_args + 2;
> -      if (arg == NULL)
> -       result = extra_args + 1;
> -      else
> +      for (i = 0; i < separate_args; i++)
> +       if (argv[extra_args + 1 + i] == NULL)
> +         {
> +           errors |= CL_ERR_MISSING_ARG;
> +           break;
> +         }
> +      result = extra_args + 1 + i;
> +      if (arg != NULL)
>        have_separate_arg = true;
>     }
>
> @@ -461,6 +469,11 @@ decode_cmdline_option (const char **argv
>                                    && (lang_mask & CL_DRIVER)));
>          joined_arg_flag = (option->flags & CL_JOINED) != 0;
>
> +         if (separate_args > 1 || (option->flags & CL_SEPARATE_NARGS_MASK))
> +           gcc_assert (separate_args
> +                       == ((option->flags & CL_SEPARATE_NARGS_MASK)
> +                           >> CL_SEPARATE_NARGS_SHIFT) + 1);
> +
>          if (!(errors & CL_ERR_MISSING_ARG))
>            {
>              if (separate_arg_flag || joined_arg_flag)
> @@ -504,22 +517,7 @@ decode_cmdline_option (const char **argv
>   decoded->warn_message = warn_message;
>
>   if (opt_index == OPT_SPECIAL_unknown)
> -    {
> -      /* Skip the correct number of arguments for options handled
> -        through specs.  */
> -      const char *popt ATTRIBUTE_UNUSED = argv[0] + 1;
> -
> -      gcc_assert (result == 1);
> -      if (WORD_SWITCH_TAKES_ARG (popt))
> -       result += WORD_SWITCH_TAKES_ARG (popt);
> -      if (result > 1)
> -       for (i = 1; i < result; i++)
> -         if (argv[i] == NULL)
> -           {
> -             result = i;
> -             break;
> -           }
> -    }
> +    gcc_assert (result == 1);
>
>   gcc_assert (result >= 1 && result <= ARRAY_SIZE (decoded->canonical_option));
>   decoded->canonical_option_num_elements = result;
> @@ -538,7 +536,21 @@ decode_cmdline_option (const char **argv
>        decoded->canonical_option[i] = NULL;
>     }
>   if (opt_index != OPT_SPECIAL_unknown && opt_index != OPT_SPECIAL_ignore)
> -    generate_canonical_option (opt_index, arg, value, decoded);
> +    {
> +      generate_canonical_option (opt_index, arg, value, decoded);
> +      if (separate_args > 1)
> +       {
> +         for (i = 0; i < separate_args; i++)
> +           {
> +             if (argv[extra_args + 1 + i] == NULL)
> +                 break;
> +             else
> +               decoded->canonical_option[1 + i] = argv[extra_args + 1 + i];
> +           }
> +         gcc_assert (result == 1 + i);
> +         decoded->canonical_option_num_elements = result;
> +       }
> +    }
>   decoded->orig_option_with_args_text = p = XNEWVEC (char, total_len);
>   for (i = 0; i < result; i++)
>     {
> Index: gcc/defaults.h
> ===================================================================
> --- gcc/defaults.h      (revision 166309)
> +++ gcc/defaults.h      (working copy)
> @@ -32,14 +32,6 @@ see the files COPYING3 and COPYING.RUNTI
>  #define GET_ENVIRONMENT(VALUE, NAME) do { (VALUE) = getenv (NAME); } while (0)
>  #endif
>
> -/* This defines which multi-letter switches take arguments.  */
> -
> -#define DEFAULT_WORD_SWITCH_TAKES_ARG(STR)             0
> -
> -#ifndef WORD_SWITCH_TAKES_ARG
> -#define WORD_SWITCH_TAKES_ARG(STR) DEFAULT_WORD_SWITCH_TAKES_ARG (STR)
> -#endif
> -
>  /* Store in OUTPUT a string (made with alloca) containing an
>    assembler-name for a local static variable or function named NAME.
>    LABELNO is an integer which is different for each call.  */
> Index: gcc/opts.h
> ===================================================================
> --- gcc/opts.h  (revision 166309)
> +++ gcc/opts.h  (working copy)
> @@ -71,12 +71,12 @@ extern const unsigned int cl_options_cou
>  extern const char *const lang_names[];
>  extern const unsigned int cl_lang_count;
>
> -#define CL_PARAMS               (1 << 13) /* Fake entry.  Used to display --param info with --help.  */
> -#define CL_WARNING             (1 << 14) /* Enables an (optional) warning message.  */
> -#define CL_OPTIMIZATION                (1 << 15) /* Enables an (optional) optimization.  */
> -#define CL_DRIVER              (1 << 16) /* Driver option.  */
> -#define CL_TARGET              (1 << 17) /* Target-specific option.  */
> -#define CL_COMMON              (1 << 18) /* Language-independent.  */
> +#define CL_PARAMS               (1 << 11) /* Fake entry.  Used to display --param info with --help.  */
> +#define CL_WARNING             (1 << 12) /* Enables an (optional) warning message.  */
> +#define CL_OPTIMIZATION                (1 << 13) /* Enables an (optional) optimization.  */
> +#define CL_DRIVER              (1 << 14) /* Driver option.  */
> +#define CL_TARGET              (1 << 15) /* Target-specific option.  */
> +#define CL_COMMON              (1 << 16) /* Language-independent.  */
>
>  #define CL_MIN_OPTION_CLASS    CL_PARAMS
>  #define CL_MAX_OPTION_CLASS    CL_COMMON
> @@ -86,6 +86,11 @@ extern const unsigned int cl_lang_count;
>    This distinction is important because --help will not list options
>    which only have these higher bits set.  */
>
> +/* Options marked with CL_SEPARATE take a number of separate arguments
> +   (1 to 4) that is one more than the number in this bit-field.  */
> +#define CL_SEPARATE_NARGS_SHIFT        17
> +#define CL_SEPARATE_NARGS_MASK (3 << CL_SEPARATE_NARGS_SHIFT)
> +
>  #define CL_SEPARATE_ALIAS      (1 << 19) /* Option is an alias when used with separate argument.  */
>  #define CL_NO_DRIVER_ARG       (1 << 20) /* Option takes no argument in the driver.  */
>  #define CL_REJECT_DRIVER       (1 << 21) /* Reject this option in the driver.  */
> Index: gcc/system.h
> ===================================================================
> --- gcc/system.h        (revision 166309)
> +++ gcc/system.h        (working copy)
> @@ -776,7 +776,7 @@ extern void fancy_abort (const char *, i
>        STACK_CHECK_PROBE_INTERVAL STACK_CHECK_PROBE_LOAD                  \
>        ORDER_REGS_FOR_LOCAL_ALLOC FUNCTION_OUTGOING_VALUE                 \
>        ASM_DECLARE_CONSTANT_NAME MODIFY_TARGET_NAME SWITCHES_NEED_SPACES  \
> -       SWITCH_CURTAILS_COMPILATION SWITCH_TAKES_ARG
> +       SWITCH_CURTAILS_COMPILATION SWITCH_TAKES_ARG WORD_SWITCH_TAKES_ARG
>
>  /* Hooks that are no longer used.  */
>  #pragma GCC poison LANG_HOOKS_FUNCTION_MARK LANG_HOOKS_FUNCTION_FREE  \
> Index: gcc/opt-functions.awk
> ===================================================================
> --- gcc/opt-functions.awk       (revision 166309)
> +++ gcc/opt-functions.awk       (working copy)
> @@ -100,6 +100,12 @@ function switch_flags (flags)
>          test_flag("Warning", flags,  " | CL_WARNING") \
>          test_flag("Optimization", flags,  " | CL_OPTIMIZATION") \
>          test_flag("Report", flags, " | CL_REPORT")
> +       sep_args = opt_args("Args", flags)
> +       if (sep_args != "") {
> +               sep_args--
> +               result = result " | (" sep_args \
> +                   " << CL_SEPARATE_NARGS_SHIFT)"
> +       }
>        sub( "^0 \\| ", "", result )
>        return result
>  }
> Index: gcc/config/darwin.opt
> ===================================================================
> --- gcc/config/darwin.opt       (revision 166309)
> +++ gcc/config/darwin.opt       (working copy)
> @@ -96,6 +96,9 @@ Driver Separate
>  Zseg_addr_table
>  Driver Separate
>
> +Zsegaddr
> +Driver Separate Args(2)
> +
>  Zsegs_read_only_addr
>  Driver Separate
>
> @@ -129,9 +132,27 @@ Driver Separate
>  read_only_relocs
>  Driver Separate
>
> +sectalign
> +Driver Separate Args(3)
> +
> +sectcreate
> +Driver Separate Args(3)
> +
> +sectobjectsymbols
> +Driver Separate Args(2)
> +
> +sectorder
> +Driver Separate Args(3)
> +
>  seg1addr
>  Driver Separate
>
> +segcreate
> +Driver Separate Args(3)
> +
> +segprot
> +Driver Separate Args(3)
> +
>  segs_read_only_addr
>  Driver Separate
>
> Index: gcc/config/darwin.h
> ===================================================================
> --- gcc/config/darwin.h (revision 166309)
> +++ gcc/config/darwin.h (working copy)
> @@ -194,20 +194,6 @@ extern GTY(()) int darwin_ms_struct;
>     darwin_override_options ();                                                \
>   } while (0)
>
> -/* These compiler options take n arguments.  */
> -
> -#undef  WORD_SWITCH_TAKES_ARG
> -#define WORD_SWITCH_TAKES_ARG(STR)              \
> -  (DEFAULT_WORD_SWITCH_TAKES_ARG (STR) ? 1 :    \
> -   !strcmp (STR, "sectcreate") ? 3 :            \
> -   !strcmp (STR, "sectorder") ? 3 :             \
> -   !strcmp (STR, "Zsegaddr") ? 2 :              \
> -   !strcmp (STR, "segprot") ? 3 :               \
> -   !strcmp (STR, "sectalign") ? 3 :             \
> -   !strcmp (STR, "sectobjectsymbols") ? 2 :     \
> -   !strcmp (STR, "segcreate") ? 3 :             \
> -   0)
> -
>  #define SUBTARGET_C_COMMON_OVERRIDE_OPTIONS do {                        \
>     if (flag_mkernel || flag_apple_kext)                               \
>       {                                                                        \
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful)
  2010-11-05  2:35 Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful) Joseph S. Myers
  2010-11-05 10:41 ` Richard Guenther
@ 2010-11-05 13:08 ` Jack Howarth
  2010-11-05 15:00   ` IainS
  1 sibling, 1 reply; 9+ messages in thread
From: Jack Howarth @ 2010-11-05 13:08 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: gcc-patches

On Fri, Nov 05, 2010 at 01:15:10AM +0000, Joseph S. Myers wrote:
> Continuing the ARM-sponsored series of option-handling cleanup patches
> preparing for the multilib selection changes described at
> <http://gcc.gnu.org/ml/gcc/2010-01/msg00063.html> (which changes to
> multilib selection itself will now clearly need to wait for 4.7), this
> 71st patch in the series completes the removal of
> WORD_SWITCH_TAKES_ARG.
> 
> After patch 70
> <http://gcc.gnu.org/ml/gcc-patches/2010-10/msg02373.html> the only
> remaining definition of this macro was for Darwin, because
> WORD_SWITCH_TAKES_ARG was the only way of describing an option as
> taking more than one argument and Darwin has such options (passed to
> the linker by specs).  This patch adds a facility to .opt files for
> options to take more than one separate argument and describes the
> Darwin options in the .opt file accordingly.  (I do not expect any new
> options to use this facility, as multi-argument options seem
> confusingly different from normal practice for options.)
> 
> I added a two-bit field to the option flags to handle options with 1-4
> arguments.  A range of 0-3, by making CL_SEPARATE into a two-bit field
> rather than using a separate field, would have sufficed, but making
> CL_SEPARATE into a two-bit field would have involved more complicated
> changes to --help handling that presumes flags are single bits; with
> this patch the option-handling changes are safer and more
> self-contained.
> 
> We are however quite close to running out of bits in the flags field;
> bits 0-7 are used for languages, and 11-30 for other flags, and the Go
> front end will cause bits 0-8 to be used for languages.  That leaves
> just two bits spare for languages, and I know that Debian at least
> builds the compiler with driver support for two extra languages
> (Pascal and D); I do not know if any distributors are building with
> three or more extra languages.  It shouldn't be hard to use bit 31
> (making flags consistently unsigned), but beyond that it will be
> necessary to split up flags in some way (maybe making those above
> CL_MAX_OPTION_CLASS, or at least those not relevant to --help, into
> bit-fields rather than allocating particular bits explicitly).
> 
> Bootstrapped with no regressions on x86_64-unknown-linux-gnu.  I also
> tested building xgcc for a cross to i686-darwin and examined the
> decoded options under GDB, but it might be useful for someone to run
> tests on Darwin (though I suspect a bootstrap and testsuite run may
> not exercise the affected options).  OK to commit?

Joseph,
    Bootstraps without regressions  on x86_64-apple-darwin10...

http://gcc.gnu.org/ml/gcc-testresults/2010-11/msg00402.html

       Jack

> 
> 2010-11-04  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* defaults.h (DEFAULT_WORD_SWITCH_TAKES_ARG,
> 	WORD_SWITCH_TAKES_ARG): Remove.
> 	* doc/options.texi (Args): Document.
> 	* doc/tm.texi.in (WORD_SWITCH_TAKES_ARG): Remove.
> 	* doc/tm.texi: Regenerate.
> 	* opt-functions.awk (switch_flags): Handle Args.
> 	* opts-common.c: Update comment on tm.h include.
> 	(decode_cmdline_option): Handle options with multiple arguments.
> 	Don't check WORD_SWITCH_TAKES_ARG for unknown options.
> 	* opts.h (CL_SEPARATE_NARGS_SHIFT, CL_SEPARATE_NARGS_MASK):
> 	Define.
> 	(CL_PARAMS, CL_WARNING, CL_OPTIMIZATION, CL_DRIVER, CL_TARGET,
> 	CL_COMMON): Update values.
> 	* system.h (WORD_SWITCH_TAKES_ARG): Poison.
> 	* config/darwin.h (WORD_SWITCH_TAKES_ARG): Remove.
> 	* config/darwin.opt (Zsegaddr, sectalign, sectcreate,
> 	sectobjectsymbols, sectorder, segcreate, segprot): New.
> 
> Index: gcc/doc/options.texi
> ===================================================================
> --- gcc/doc/options.texi	(revision 166309)
> +++ gcc/doc/options.texi	(working copy)
> @@ -162,6 +162,10 @@ generic error message is used.  @var{mes
>  @samp{%qs} format, which will be used to format the name of the option
>  passed.
>  
> +@item Args(@var{n})
> +For an option marked @code{Separate}, indicate that it takes @var{n}
> +arguments.  The default is 1.
> +
>  @item UInteger
>  The option's argument is a non-negative integer.  The option parser
>  will check and convert the argument before passing it to the relevant
> Index: gcc/doc/tm.texi
> ===================================================================
> --- gcc/doc/tm.texi	(revision 166309)
> +++ gcc/doc/tm.texi	(working copy)
> @@ -99,21 +99,6 @@ from being defined in the @file{.h} file
>  @c prevent bad page break with this line
>  You can control the compilation driver.
>  
> -@defmac WORD_SWITCH_TAKES_ARG (@var{name})
> -A C expression which determines whether the option @option{-@var{name}}
> -takes arguments.  The value should be the number of arguments that
> -option takes--zero, for many options.
> -This macro does not need to handle options defined in @file{.opt}
> -files, only those that are handled purely through specs.
> -
> -By default, this macro is defined as
> -@code{DEFAULT_WORD_SWITCH_TAKES_ARG}, which handles the standard options
> -properly.  You need not define @code{WORD_SWITCH_TAKES_ARG} unless you
> -wish to add additional options which take arguments.  Any redefinition
> -should call @code{DEFAULT_WORD_SWITCH_TAKES_ARG} and then check for
> -additional options.
> -@end defmac
> -
>  @defmac TARGET_OPTION_TRANSLATE_TABLE
>  If defined, a list of pairs of strings, the first of which is a
>  potential command line target to the @file{gcc} driver program, and the
> Index: gcc/doc/tm.texi.in
> ===================================================================
> --- gcc/doc/tm.texi.in	(revision 166309)
> +++ gcc/doc/tm.texi.in	(working copy)
> @@ -99,21 +99,6 @@ from being defined in the @file{.h} file
>  @c prevent bad page break with this line
>  You can control the compilation driver.
>  
> -@defmac WORD_SWITCH_TAKES_ARG (@var{name})
> -A C expression which determines whether the option @option{-@var{name}}
> -takes arguments.  The value should be the number of arguments that
> -option takes--zero, for many options.
> -This macro does not need to handle options defined in @file{.opt}
> -files, only those that are handled purely through specs.
> -
> -By default, this macro is defined as
> -@code{DEFAULT_WORD_SWITCH_TAKES_ARG}, which handles the standard options
> -properly.  You need not define @code{WORD_SWITCH_TAKES_ARG} unless you
> -wish to add additional options which take arguments.  Any redefinition
> -should call @code{DEFAULT_WORD_SWITCH_TAKES_ARG} and then check for
> -additional options.
> -@end defmac
> -
>  @defmac TARGET_OPTION_TRANSLATE_TABLE
>  If defined, a list of pairs of strings, the first of which is a
>  potential command line target to the @file{gcc} driver program, and the
> Index: gcc/opts-common.c
> ===================================================================
> --- gcc/opts-common.c	(revision 166309)
> +++ gcc/opts-common.c	(working copy)
> @@ -24,8 +24,7 @@ along with GCC; see the file COPYING3.  
>  #include "opts.h"
>  #include "flags.h"
>  #include "diagnostic.h"
> -#include "tm.h" /* For WORD_SWITCH_TAKES_ARG and
> -		   TARGET_OPTION_TRANSLATE_TABLE.  */
> +#include "tm.h" /* For TARGET_OPTION_TRANSLATE_TABLE.  */
>  
>  static void prune_options (struct cl_decoded_option **, unsigned int *);
>  
> @@ -288,7 +287,7 @@ decode_cmdline_option (const char **argv
>    size_t opt_index;
>    const char *arg = 0;
>    int value = 1;
> -  unsigned int result = 1, i, extra_args;
> +  unsigned int result = 1, i, extra_args, separate_args;
>    int adjust_len = 0;
>    size_t total_len;
>    char *p;
> @@ -366,10 +365,15 @@ decode_cmdline_option (const char **argv
>      errors |= CL_ERR_DISABLED;
>  
>    /* Determine whether there may be a separate argument based on
> -     whether this option is being processed for the driver.  */
> +     whether this option is being processed for the driver, and, if
> +     so, how many such arguments.  */
>    separate_arg_flag = ((option->flags & CL_SEPARATE)
>  		       && !((option->flags & CL_NO_DRIVER_ARG)
>  			    && (lang_mask & CL_DRIVER)));
> +  separate_args = (separate_arg_flag
> +		   ? ((option->flags & CL_SEPARATE_NARGS_MASK)
> +		      >> CL_SEPARATE_NARGS_SHIFT) + 1
> +		   : 0);
>    joined_arg_flag = (option->flags & CL_JOINED) != 0;
>  
>    /* Sort out any argument the switch takes.  */
> @@ -399,10 +403,14 @@ decode_cmdline_option (const char **argv
>    else if (separate_arg_flag)
>      {
>        arg = argv[extra_args + 1];
> -      result = extra_args + 2;
> -      if (arg == NULL)
> -	result = extra_args + 1;
> -      else
> +      for (i = 0; i < separate_args; i++)
> +	if (argv[extra_args + 1 + i] == NULL)
> +	  {
> +	    errors |= CL_ERR_MISSING_ARG;
> +	    break;
> +	  }
> +      result = extra_args + 1 + i;
> +      if (arg != NULL)
>  	have_separate_arg = true;
>      }
>  
> @@ -461,6 +469,11 @@ decode_cmdline_option (const char **argv
>  				    && (lang_mask & CL_DRIVER)));
>  	  joined_arg_flag = (option->flags & CL_JOINED) != 0;
>  
> +	  if (separate_args > 1 || (option->flags & CL_SEPARATE_NARGS_MASK))
> +	    gcc_assert (separate_args
> +			== ((option->flags & CL_SEPARATE_NARGS_MASK)
> +			    >> CL_SEPARATE_NARGS_SHIFT) + 1);
> +
>  	  if (!(errors & CL_ERR_MISSING_ARG))
>  	    {
>  	      if (separate_arg_flag || joined_arg_flag)
> @@ -504,22 +517,7 @@ decode_cmdline_option (const char **argv
>    decoded->warn_message = warn_message;
>  
>    if (opt_index == OPT_SPECIAL_unknown)
> -    {
> -      /* Skip the correct number of arguments for options handled
> -	 through specs.  */
> -      const char *popt ATTRIBUTE_UNUSED = argv[0] + 1;
> -
> -      gcc_assert (result == 1);
> -      if (WORD_SWITCH_TAKES_ARG (popt))
> -	result += WORD_SWITCH_TAKES_ARG (popt);
> -      if (result > 1)
> -	for (i = 1; i < result; i++)
> -	  if (argv[i] == NULL)
> -	    {
> -	      result = i;
> -	      break;
> -	    }
> -    }
> +    gcc_assert (result == 1);
>  
>    gcc_assert (result >= 1 && result <= ARRAY_SIZE (decoded->canonical_option));
>    decoded->canonical_option_num_elements = result;
> @@ -538,7 +536,21 @@ decode_cmdline_option (const char **argv
>  	decoded->canonical_option[i] = NULL;
>      }
>    if (opt_index != OPT_SPECIAL_unknown && opt_index != OPT_SPECIAL_ignore)
> -    generate_canonical_option (opt_index, arg, value, decoded);
> +    {
> +      generate_canonical_option (opt_index, arg, value, decoded);
> +      if (separate_args > 1)
> +	{
> +	  for (i = 0; i < separate_args; i++)
> +	    {
> +	      if (argv[extra_args + 1 + i] == NULL)
> +		  break;
> +	      else
> +		decoded->canonical_option[1 + i] = argv[extra_args + 1 + i];
> +	    }
> +	  gcc_assert (result == 1 + i);
> +	  decoded->canonical_option_num_elements = result;
> +	}
> +    }
>    decoded->orig_option_with_args_text = p = XNEWVEC (char, total_len);
>    for (i = 0; i < result; i++)
>      {
> Index: gcc/defaults.h
> ===================================================================
> --- gcc/defaults.h	(revision 166309)
> +++ gcc/defaults.h	(working copy)
> @@ -32,14 +32,6 @@ see the files COPYING3 and COPYING.RUNTI
>  #define GET_ENVIRONMENT(VALUE, NAME) do { (VALUE) = getenv (NAME); } while (0)
>  #endif
>  
> -/* This defines which multi-letter switches take arguments.  */
> -
> -#define DEFAULT_WORD_SWITCH_TAKES_ARG(STR)		0
> -
> -#ifndef WORD_SWITCH_TAKES_ARG
> -#define WORD_SWITCH_TAKES_ARG(STR) DEFAULT_WORD_SWITCH_TAKES_ARG (STR)
> -#endif
> -
>  /* Store in OUTPUT a string (made with alloca) containing an
>     assembler-name for a local static variable or function named NAME.
>     LABELNO is an integer which is different for each call.  */
> Index: gcc/opts.h
> ===================================================================
> --- gcc/opts.h	(revision 166309)
> +++ gcc/opts.h	(working copy)
> @@ -71,12 +71,12 @@ extern const unsigned int cl_options_cou
>  extern const char *const lang_names[];
>  extern const unsigned int cl_lang_count;
>  
> -#define CL_PARAMS               (1 << 13) /* Fake entry.  Used to display --param info with --help.  */
> -#define CL_WARNING		(1 << 14) /* Enables an (optional) warning message.  */
> -#define CL_OPTIMIZATION		(1 << 15) /* Enables an (optional) optimization.  */
> -#define CL_DRIVER		(1 << 16) /* Driver option.  */
> -#define CL_TARGET		(1 << 17) /* Target-specific option.  */
> -#define CL_COMMON		(1 << 18) /* Language-independent.  */
> +#define CL_PARAMS               (1 << 11) /* Fake entry.  Used to display --param info with --help.  */
> +#define CL_WARNING		(1 << 12) /* Enables an (optional) warning message.  */
> +#define CL_OPTIMIZATION		(1 << 13) /* Enables an (optional) optimization.  */
> +#define CL_DRIVER		(1 << 14) /* Driver option.  */
> +#define CL_TARGET		(1 << 15) /* Target-specific option.  */
> +#define CL_COMMON		(1 << 16) /* Language-independent.  */
>  
>  #define CL_MIN_OPTION_CLASS	CL_PARAMS
>  #define CL_MAX_OPTION_CLASS	CL_COMMON
> @@ -86,6 +86,11 @@ extern const unsigned int cl_lang_count;
>     This distinction is important because --help will not list options
>     which only have these higher bits set.  */
>  
> +/* Options marked with CL_SEPARATE take a number of separate arguments
> +   (1 to 4) that is one more than the number in this bit-field.  */
> +#define CL_SEPARATE_NARGS_SHIFT	17
> +#define CL_SEPARATE_NARGS_MASK	(3 << CL_SEPARATE_NARGS_SHIFT)
> +
>  #define CL_SEPARATE_ALIAS	(1 << 19) /* Option is an alias when used with separate argument.  */
>  #define CL_NO_DRIVER_ARG	(1 << 20) /* Option takes no argument in the driver.  */
>  #define CL_REJECT_DRIVER	(1 << 21) /* Reject this option in the driver.  */
> Index: gcc/system.h
> ===================================================================
> --- gcc/system.h	(revision 166309)
> +++ gcc/system.h	(working copy)
> @@ -776,7 +776,7 @@ extern void fancy_abort (const char *, i
>  	STACK_CHECK_PROBE_INTERVAL STACK_CHECK_PROBE_LOAD		   \
>  	ORDER_REGS_FOR_LOCAL_ALLOC FUNCTION_OUTGOING_VALUE		   \
>  	ASM_DECLARE_CONSTANT_NAME MODIFY_TARGET_NAME SWITCHES_NEED_SPACES  \
> -	SWITCH_CURTAILS_COMPILATION SWITCH_TAKES_ARG
> +	SWITCH_CURTAILS_COMPILATION SWITCH_TAKES_ARG WORD_SWITCH_TAKES_ARG
>  
>  /* Hooks that are no longer used.  */
>   #pragma GCC poison LANG_HOOKS_FUNCTION_MARK LANG_HOOKS_FUNCTION_FREE	\
> Index: gcc/opt-functions.awk
> ===================================================================
> --- gcc/opt-functions.awk	(revision 166309)
> +++ gcc/opt-functions.awk	(working copy)
> @@ -100,6 +100,12 @@ function switch_flags (flags)
>  	  test_flag("Warning", flags,  " | CL_WARNING") \
>  	  test_flag("Optimization", flags,  " | CL_OPTIMIZATION") \
>  	  test_flag("Report", flags, " | CL_REPORT")
> +	sep_args = opt_args("Args", flags)
> +	if (sep_args != "") {
> +		sep_args--
> +		result = result " | (" sep_args \
> +		    " << CL_SEPARATE_NARGS_SHIFT)"
> +	}
>  	sub( "^0 \\| ", "", result )
>  	return result
>  }
> Index: gcc/config/darwin.opt
> ===================================================================
> --- gcc/config/darwin.opt	(revision 166309)
> +++ gcc/config/darwin.opt	(working copy)
> @@ -96,6 +96,9 @@ Driver Separate
>  Zseg_addr_table
>  Driver Separate
>  
> +Zsegaddr
> +Driver Separate Args(2)
> +
>  Zsegs_read_only_addr
>  Driver Separate
>  
> @@ -129,9 +132,27 @@ Driver Separate
>  read_only_relocs
>  Driver Separate
>  
> +sectalign
> +Driver Separate Args(3)
> +
> +sectcreate
> +Driver Separate Args(3)
> +
> +sectobjectsymbols
> +Driver Separate Args(2)
> +
> +sectorder
> +Driver Separate Args(3)
> +
>  seg1addr
>  Driver Separate
>  
> +segcreate
> +Driver Separate Args(3)
> +
> +segprot
> +Driver Separate Args(3)
> +
>  segs_read_only_addr
>  Driver Separate
>  
> Index: gcc/config/darwin.h
> ===================================================================
> --- gcc/config/darwin.h	(revision 166309)
> +++ gcc/config/darwin.h	(working copy)
> @@ -194,20 +194,6 @@ extern GTY(()) int darwin_ms_struct;
>      darwin_override_options ();						\
>    } while (0)
>  
> -/* These compiler options take n arguments.  */
> -
> -#undef  WORD_SWITCH_TAKES_ARG
> -#define WORD_SWITCH_TAKES_ARG(STR)              \
> -  (DEFAULT_WORD_SWITCH_TAKES_ARG (STR) ? 1 :    \
> -   !strcmp (STR, "sectcreate") ? 3 :            \
> -   !strcmp (STR, "sectorder") ? 3 :             \
> -   !strcmp (STR, "Zsegaddr") ? 2 :              \
> -   !strcmp (STR, "segprot") ? 3 :               \
> -   !strcmp (STR, "sectalign") ? 3 :             \
> -   !strcmp (STR, "sectobjectsymbols") ? 2 :     \
> -   !strcmp (STR, "segcreate") ? 3 :             \
> -   0)
> -
>  #define SUBTARGET_C_COMMON_OVERRIDE_OPTIONS do {                        \
>      if (flag_mkernel || flag_apple_kext)				\
>        {									\
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful)
  2010-11-05 13:08 ` Jack Howarth
@ 2010-11-05 15:00   ` IainS
  2010-11-05 15:18     ` Mike Stump
  0 siblings, 1 reply; 9+ messages in thread
From: IainS @ 2010-11-05 15:00 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: GCC Patches


On 5 Nov 2010, at 13:05, Jack Howarth wrote:

> On Fri, Nov 05, 2010 at 01:15:10AM +0000, Joseph S. Myers wrote:
>> Continuing the ARM-sponsored series of option-handling cleanup  
>> patches
>> preparing for the multilib selection changes described at
>> <http://gcc.gnu.org/ml/gcc/2010-01/msg00063.html> (which changes to
>> multilib selection itself will now clearly need to wait for 4.7),  
>> this
>> 71st patch in the series completes the removal of
>> WORD_SWITCH_TAKES_ARG.
>>
>> After patch 70
>> <http://gcc.gnu.org/ml/gcc-patches/2010-10/msg02373.html> the only
>> remaining definition of this macro was for Darwin, because
>> WORD_SWITCH_TAKES_ARG was the only way of describing an option as
>> taking more than one argument and Darwin has such options (passed to
>> the linker by specs).  This patch adds a facility to .opt files for
>> options to take more than one separate argument and describes the
>> Darwin options in the .opt file accordingly.  (I do not expect any  
>> new
>> options to use this facility, as multi-argument options seem
>> confusingly different from normal practice for options.)
>>
>> I added a two-bit field to the option flags to handle options with  
>> 1-4
>> arguments.  A range of 0-3, by making CL_SEPARATE into a two-bit  
>> field
>> rather than using a separate field, would have sufficed, but making
>> CL_SEPARATE into a two-bit field would have involved more complicated
>> changes to --help handling that presumes flags are single bits; with
>> this patch the option-handling changes are safer and more
>> self-contained.
>>
>> We are however quite close to running out of bits in the flags field;
>> bits 0-7 are used for languages, and 11-30 for other flags, and the  
>> Go
>> front end will cause bits 0-8 to be used for languages.  That leaves
>> just two bits spare for languages, and I know that Debian at least
>> builds the compiler with driver support for two extra languages
>> (Pascal and D); I do not know if any distributors are building with
>> three or more extra languages.  It shouldn't be hard to use bit 31
>> (making flags consistently unsigned), but beyond that it will be
>> necessary to split up flags in some way (maybe making those above
>> CL_MAX_OPTION_CLASS, or at least those not relevant to --help, into
>> bit-fields rather than allocating particular bits explicitly).
>>
>> Bootstrapped with no regressions on x86_64-unknown-linux-gnu.  I also
>> tested building xgcc for a cross to i686-darwin and examined the
>> decoded options under GDB, but it might be useful for someone to run
>> tests on Darwin (though I suspect a bootstrap and testsuite run may
>> not exercise the affected options).  OK to commit?
>
>    Bootstraps without regressions  on x86_64-apple-darwin10...

and on i686-darwin9. (powerpc-d9 is running)

Additionally, I made a trivial example using the "-sectcreate" option  
- which produced the expected output.

thanks for the tidy-up!

Iain

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful)
  2010-11-05 15:00   ` IainS
@ 2010-11-05 15:18     ` Mike Stump
  2010-11-05 15:29       ` IainS
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Stump @ 2010-11-05 15:18 UTC (permalink / raw)
  To: IainS; +Cc: Joseph S. Myers, GCC Patches

On Nov 5, 2010, at 7:24 AM, IainS wrote:
> Additionally, I made a trivial example using the "-sectcreate" option - which produced the expected output.

Using gcc -v -segaddr bla on the gcc line is the other test to try...  If ld gets a -segaddr bla option, its fine.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful)
  2010-11-05 15:18     ` Mike Stump
@ 2010-11-05 15:29       ` IainS
  2010-11-06 14:26         ` IainS
  0 siblings, 1 reply; 9+ messages in thread
From: IainS @ 2010-11-05 15:29 UTC (permalink / raw)
  To: Mike Stump; +Cc: Joseph S. Myers, GCC Patches


On 5 Nov 2010, at 15:10, Mike Stump wrote:

> On Nov 5, 2010, at 7:24 AM, IainS wrote:
>> Additionally, I made a trivial example using the "-sectcreate"  
>> option - which produced the expected output.
>
> Using gcc -v -segaddr bla on the gcc line is the other test to  
> try...  If ld gets a -segaddr bla option, its fine.

Yep, that one works too.
  I guess we could construct some kind of darwin-specific test but it  
would really require inspecting the output of otool  - since we need  
post-linker output to check.
(and there's no mechanism in the  test-suite to do that AFAIK).

Iain

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful)
  2010-11-05 15:29       ` IainS
@ 2010-11-06 14:26         ` IainS
  2010-11-08 22:04           ` Mike Stump
  0 siblings, 1 reply; 9+ messages in thread
From: IainS @ 2010-11-06 14:26 UTC (permalink / raw)
  To: Mike Stump; +Cc: Joseph S. Myers, GCC Patches


On 5 Nov 2010, at 15:24, IainS wrote:

>
> On 5 Nov 2010, at 15:10, Mike Stump wrote:
>
>> On Nov 5, 2010, at 7:24 AM, IainS wrote:
>>> Additionally, I made a trivial example using the "-sectcreate"  
>>> option - which produced the expected output.
>>
>> Using gcc -v -segaddr bla on the gcc line is the other test to  
>> try...  If ld gets a -segaddr bla option, its fine.
>
> Yep, that one works too.
> I guess we could construct some kind of darwin-specific test but it  
> would really require inspecting the output of otool  - since we need  
> post-linker output to check.
> (and there's no mechanism in the  test-suite to do that AFAIK).


The testsuite has a nop test already that checks the command line  
doesn't fail

----

But, it is possible to do at least a rudimentary functionality test  
for segaddr.

(It would be possible to do something similar with sectcreate -  
however, dg-additional-files doesn't seem to work for the non-remote  
case.)

Should we apply something like this as a backstop?
Iain

Index: gcc/testsuite/gcc.dg/darwin-segaddr.c
===================================================================
--- gcc/testsuite/gcc.dg/darwin-segaddr.c	(revision 0)
+++ gcc/testsuite/gcc.dg/darwin-segaddr.c	(revision 0)
@@ -0,0 +1,19 @@
+/* Check that -segaddr gets through and works.  */
+/* { dg-do run { target *-*-darwin* } } */
+/* { dg-options "-O0 -segaddr __TEST 0x200000" { target { *-*-darwin*  
&& { ! lp64 } } } } */
+/* { dg-options "-O0 -segaddr __TEST 0x110000000" { target { *-*- 
darwin* && lp64 } } } */
+
+extern void abort ();
+
+int t __attribute__((section("__TEST,__test")));
+
+int main (void)
+{
+#ifdef __LP64__
+  if ((unsigned long long) &t != 0x110000000ULL)
+#else
+  if ((unsigned long) &t != 0x200000UL)
+#endif
+    abort ();
+  return 0;
+}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful)
  2010-11-06 14:26         ` IainS
@ 2010-11-08 22:04           ` Mike Stump
  2010-11-13 14:59             ` IainS
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Stump @ 2010-11-08 22:04 UTC (permalink / raw)
  To: IainS; +Cc: Joseph S. Myers, GCC Patches

On Nov 6, 2010, at 4:39 AM, IainS wrote:
> But, it is possible to do at least a rudimentary functionality test for segaddr.
> 
> (It would be possible to do something similar with sectcreate - however, dg-additional-files doesn't seem to work for the non-remote case.)
> 
> Should we apply something like this as a backstop?

Well, if you're going to write a testcase, it's hard to say no...

Ok.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful)
  2010-11-08 22:04           ` Mike Stump
@ 2010-11-13 14:59             ` IainS
  0 siblings, 0 replies; 9+ messages in thread
From: IainS @ 2010-11-13 14:59 UTC (permalink / raw)
  To: GCC Patches

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


On 8 Nov 2010, at 22:04, Mike Stump wrote:

> On Nov 6, 2010, at 4:39 AM, IainS wrote:
>> But, it is possible to do at least a rudimentary functionality test  
>> for segaddr.
>>
>> (It would be possible to do something similar with sectcreate -  
>> however, dg-additional-files doesn't seem to work for the non- 
>> remote case.)
>>
>> Should we apply something like this as a backstop?
>
> Well, if you're going to write a testcase, it's hard to say no...

added as r166705
cheers
Iain


[-- Attachment #2: 166704-dar-segaddr.txt --]
[-- Type: text/plain, Size: 1167 bytes --]

Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 166703)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2010-11-13  Iain Sandoe  <iains@gcc.gnu.org>
+
+	* gcc.dg/darwin-segaddr.c: New test for multiple argument c/l switch.
+
 2010-11-13  Tobias Burnus  <burnus@net-b.de>
 
 	PR fortran/45742
Index: gcc/testsuite/gcc.dg/darwin-segaddr.c
===================================================================
--- gcc/testsuite/gcc.dg/darwin-segaddr.c	(revision 0)
+++ gcc/testsuite/gcc.dg/darwin-segaddr.c	(revision 0)
@@ -0,0 +1,19 @@
+/* Check that -segaddr gets through and works.  */
+/* { dg-do run { target *-*-darwin* } } */
+/* { dg-options "-O0 -segaddr __TEST 0x200000" { target { *-*-darwin* && { ! lp64 } } } } */
+/* { dg-options "-O0 -segaddr __TEST 0x110000000" { target { *-*-darwin* && lp64 } } } */
+
+extern void abort ();
+
+int t __attribute__((section("__TEST,__test")));
+
+int main (void)
+{
+#ifdef __LP64__
+  if ((unsigned long long) &t != 0x110000000ULL)
+#else
+  if ((unsigned long) &t != 0x200000UL)
+#endif
+    abort ();
+  return 0;
+}

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2010-11-13 12:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-05  2:35 Remove WORD_SWITCH_TAKES_ARG (Darwin testers useful) Joseph S. Myers
2010-11-05 10:41 ` Richard Guenther
2010-11-05 13:08 ` Jack Howarth
2010-11-05 15:00   ` IainS
2010-11-05 15:18     ` Mike Stump
2010-11-05 15:29       ` IainS
2010-11-06 14:26         ` IainS
2010-11-08 22:04           ` Mike Stump
2010-11-13 14:59             ` IainS

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).