diff mbox

Split up option decoding code

Message ID Pine.LNX.4.64.1006111827200.17947@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers June 11, 2010, 6:28 p.m. UTC
In the process of sharing option processing machinery between the
driver and the compilers proper to improve multilib selection, I
expect an intermediate stage where the basic machinery is shared but
much of the logic associated with individual options isn't; the logic
for individual options can then be shared incrementally (parts of it,
e.g. anything setting up trees or RTL in response to options, not
being needed in the driver at all) until everything needed for
multilib selection is shared.

This patch moves towards that stage by separating the code that
converts part of the argv array into an option and its operand from
the code that processes the option further to change state in the
compiler; the former goes in opts-common.c (much of the latter will go
there eventually as well).  My plan is to follow this up by having an
initial pass converting the argv array to an array of
cl_decoded_option structures, with subsequent passes in both the
driver and the compilers proper working on that structure.  There are
several existing passes over argv, and these passes often do not work
reliably in the presence of option arguments that look like options
themselves, so this will improve consistency in option handling
between the driver and the compiler proper and within each of those.

The driver's use of common machinery can then start with
decode_cmdline_option using SWITCH_TAKES_ARG and WORD_SWITCH_TAKES_ARG
for unknown options so that an accurate array is produced in the
driver (and used by prune_options), before actually adding driver
options (and then miscellaneous options from specs) to the .opt files
so that it uses the machinery more usefully to convert options to
features.  I expect also to convert the translate_options code in
gcc.c to something better integrated with .opt files.

Bootstrapped with no regressions on x86_64-unknown-linux-gnu.  OK to
commit?

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

	* opts-common.c: Include options.h.
	(integral_argument): Move from opts.c.
	(decode_cmdline_option): New.  Based on read_cmdline_option.
	* opts.c (integral_argument): Move to opts-common.c.
	(read_cmdline_option): Move most contents to
	decode_cmdline_option.  Use %qs in diagnostics.
	* opts.h (CL_ERR_DISABLED, CL_ERR_MISSING_ARG, CL_ERR_WRONG_LANG,
	CL_ERR_UINT_ARG, struct cl_decoded_option, integral_argument,
	decode_cmdline_option): New.

testsuite:
2010-06-11  Joseph Myers  <joseph@codesourcery.com>

	* gcc.dg/funroll-loops-all.c: Update expected error.

Comments

Richard Biener June 11, 2010, 11:14 p.m. UTC | #1
On Fri, Jun 11, 2010 at 8:28 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> In the process of sharing option processing machinery between the
> driver and the compilers proper to improve multilib selection, I
> expect an intermediate stage where the basic machinery is shared but
> much of the logic associated with individual options isn't; the logic
> for individual options can then be shared incrementally (parts of it,
> e.g. anything setting up trees or RTL in response to options, not
> being needed in the driver at all) until everything needed for
> multilib selection is shared.
>
> This patch moves towards that stage by separating the code that
> converts part of the argv array into an option and its operand from
> the code that processes the option further to change state in the
> compiler; the former goes in opts-common.c (much of the latter will go
> there eventually as well).  My plan is to follow this up by having an
> initial pass converting the argv array to an array of
> cl_decoded_option structures, with subsequent passes in both the
> driver and the compilers proper working on that structure.  There are
> several existing passes over argv, and these passes often do not work
> reliably in the presence of option arguments that look like options
> themselves, so this will improve consistency in option handling
> between the driver and the compiler proper and within each of those.
>
> The driver's use of common machinery can then start with
> decode_cmdline_option using SWITCH_TAKES_ARG and WORD_SWITCH_TAKES_ARG
> for unknown options so that an accurate array is produced in the
> driver (and used by prune_options), before actually adding driver
> options (and then miscellaneous options from specs) to the .opt files
> so that it uses the machinery more usefully to convert options to
> features.  I expect also to convert the translate_options code in
> gcc.c to something better integrated with .opt files.
>
> Bootstrapped with no regressions on x86_64-unknown-linux-gnu.  OK to
> commit?

Ok.

Thanks,
Richard.

> 2010-06-11  Joseph Myers  <joseph@codesourcery.com>
>
>        * opts-common.c: Include options.h.
>        (integral_argument): Move from opts.c.
>        (decode_cmdline_option): New.  Based on read_cmdline_option.
>        * opts.c (integral_argument): Move to opts-common.c.
>        (read_cmdline_option): Move most contents to
>        decode_cmdline_option.  Use %qs in diagnostics.
>        * opts.h (CL_ERR_DISABLED, CL_ERR_MISSING_ARG, CL_ERR_WRONG_LANG,
>        CL_ERR_UINT_ARG, struct cl_decoded_option, integral_argument,
>        decode_cmdline_option): New.
>
> testsuite:
> 2010-06-11  Joseph Myers  <joseph@codesourcery.com>
>
>        * gcc.dg/funroll-loops-all.c: Update expected error.
>
> Index: opts-common.c
> ===================================================================
> --- opts-common.c       (revision 160599)
> +++ opts-common.c       (working copy)
> @@ -1,5 +1,5 @@
>  /* Command line option handling.
> -   Copyright (C) 2006, 2007, 2008 Free Software Foundation, Inc.
> +   Copyright (C) 2006, 2007, 2008, 2010 Free Software Foundation, Inc.
>
>  This file is part of GCC.
>
> @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.
>  #include "intl.h"
>  #include "coretypes.h"
>  #include "opts.h"
> +#include "options.h"
>
>  /* Perform a binary search to find which option the command-line INPUT
>    matches.  Returns its index in the option array, and N_OPTS
> @@ -107,6 +108,138 @@ find_opt (const char *input, int lang_ma
>   return match_wrong_lang;
>  }
>
> +/* If ARG is a non-negative integer made up solely of digits, return its
> +   value, otherwise return -1.  */
> +
> +int
> +integral_argument (const char *arg)
> +{
> +  const char *p = arg;
> +
> +  while (*p && ISDIGIT (*p))
> +    p++;
> +
> +  if (*p == '\0')
> +    return atoi (arg);
> +
> +  return -1;
> +}
> +
> +/* Decode the switch beginning at ARGV for the language indicated by
> +   LANG_MASK, into the structure *DECODED.  Returns the number of
> +   switches consumed.  */
> +
> +unsigned int
> +decode_cmdline_option (const char **argv, unsigned int lang_mask,
> +                      struct cl_decoded_option *decoded)
> +{
> +  size_t opt_index;
> +  const char *opt, *arg = 0;
> +  char *dup = 0;
> +  int value = 1;
> +  unsigned int result = 1;
> +  const struct cl_option *option;
> +  int errors = 0;
> +
> +  opt = argv[0];
> +
> +  opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
> +  if (opt_index == cl_options_count
> +      && (opt[1] == 'W' || opt[1] == 'f' || opt[1] == 'm')
> +      && opt[2] == 'n' && opt[3] == 'o' && opt[4] == '-')
> +    {
> +      /* Drop the "no-" from negative switches.  */
> +      size_t len = strlen (opt) - 3;
> +
> +      dup = XNEWVEC (char, len + 1);
> +      dup[0] = '-';
> +      dup[1] = opt[1];
> +      memcpy (dup + 2, opt + 5, len - 2 + 1);
> +      opt = dup;
> +      value = 0;
> +      opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
> +    }
> +
> +  if (opt_index == cl_options_count)
> +    goto done;
> +
> +  option = &cl_options[opt_index];
> +
> +  /* Reject negative form of switches that don't take negatives as
> +     unrecognized.  */
> +  if (!value && (option->flags & CL_REJECT_NEGATIVE))
> +    {
> +      opt_index = cl_options_count;
> +      goto done;
> +    }
> +
> +  /* Check to see if the option is disabled for this configuration.  */
> +  if (option->flags & CL_DISABLED)
> +    errors |= CL_ERR_DISABLED;
> +
> +  /* Sort out any argument the switch takes.  */
> +  if (option->flags & CL_JOINED)
> +    {
> +      /* Have arg point to the original switch.  This is because
> +        some code, such as disable_builtin_function, expects its
> +        argument to be persistent until the program exits.  */
> +      arg = argv[0] + cl_options[opt_index].opt_len + 1;
> +      if (!value)
> +       arg += strlen ("no-");
> +
> +      if (*arg == '\0' && !(option->flags & CL_MISSING_OK))
> +       {
> +         if (option->flags & CL_SEPARATE)
> +           {
> +             arg = argv[1];
> +             result = 2;
> +             if (arg == NULL)
> +               result = 1;
> +           }
> +         else
> +           /* Missing argument.  */
> +           arg = NULL;
> +       }
> +    }
> +  else if (option->flags & CL_SEPARATE)
> +    {
> +      arg = argv[1];
> +      result = 2;
> +      if (arg == NULL)
> +       result = 1;
> +    }
> +
> +  /* Check if this is a switch for a different front end.  */
> +  if (!(option->flags & (lang_mask | CL_COMMON | CL_TARGET)))
> +    errors |= CL_ERR_WRONG_LANG;
> +  else if ((option->flags & CL_TARGET)
> +          && (option->flags & CL_LANG_ALL)
> +          && !(option->flags & lang_mask))
> +    /* Complain for target flag language mismatches if any languages
> +       are specified.  */
> +      errors |= CL_ERR_WRONG_LANG;
> +
> +  if (arg == NULL && (option->flags & (CL_JOINED | CL_SEPARATE)))
> +    errors |= CL_ERR_MISSING_ARG;
> +
> +  /* If the switch takes an integer, convert it.  */
> +  if (arg && (option->flags & CL_UINTEGER))
> +    {
> +      value = integral_argument (arg);
> +      if (value == -1)
> +       errors |= CL_ERR_UINT_ARG;
> +    }
> +
> + done:
> +  if (dup)
> +    free (dup);
> +  decoded->opt_index = opt_index;
> +  decoded->arg = arg;
> +  decoded->value = value;
> +  decoded->errors = errors;
> +  return result;
> +}
> +
>  /* Return true if NEXT_OPT_IDX cancels OPT_IDX.  Return false if the
>    next one is the same as ORIG_NEXT_OPT_IDX.  */
>
> Index: testsuite/gcc.dg/funroll-loops-all.c
> ===================================================================
> --- testsuite/gcc.dg/funroll-loops-all.c        (revision 160599)
> +++ testsuite/gcc.dg/funroll-loops-all.c        (working copy)
> @@ -1,4 +1,4 @@
>  /* PR 17594 */
>  /* { dg-do compile } */
>  /* { dg-options "-funroll-loops-all" } */
> -/* { dg-error "unrecognized command line option \"-funroll-loops-all\"" "" { target *-*-* } 0 } */
> +/* { dg-error "unrecognized command line option '-funroll-loops-all'" "" { target *-*-* } 0 } */
> Index: opts.c
> ===================================================================
> --- opts.c      (revision 160599)
> +++ opts.c      (working copy)
> @@ -381,22 +381,6 @@ static void complain_wrong_lang (const c
>  static void set_debug_level (enum debug_info_type type, int extended,
>                             const char *arg);
>
> -/* If ARG is a non-negative integer made up solely of digits, return its
> -   value, otherwise return -1.  */
> -static int
> -integral_argument (const char *arg)
> -{
> -  const char *p = arg;
> -
> -  while (*p && ISDIGIT (*p))
> -    p++;
> -
> -  if (*p == '\0')
> -    return atoi (arg);
> -
> -  return -1;
> -}
> -
>  /* Return a malloced slash-separated list of languages in MASK.  */
>  static char *
>  write_langs (unsigned int mask)
> @@ -536,131 +520,61 @@ handle_option (int opt_index, int value,
>  static unsigned int
>  read_cmdline_option (const char **argv, unsigned int lang_mask)
>  {
> -  size_t opt_index;
> -  const char *opt, *arg = 0;
> -  char *dup = 0;
> -  int value = 1;
> -  unsigned int result = 0;
> +  struct cl_decoded_option decoded;
> +  unsigned int result;
> +  const char *opt;
>   const struct cl_option *option;
>
>   opt = argv[0];
>
> -  opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
> -  if (opt_index == cl_options_count
> -      && (opt[1] == 'W' || opt[1] == 'f' || opt[1] == 'm')
> -      && opt[2] == 'n' && opt[3] == 'o' && opt[4] == '-')
> -    {
> -      /* Drop the "no-" from negative switches.  */
> -      size_t len = strlen (opt) - 3;
> -
> -      dup = XNEWVEC (char, len + 1);
> -      dup[0] = '-';
> -      dup[1] = opt[1];
> -      memcpy (dup + 2, opt + 5, len - 2 + 1);
> -      opt = dup;
> -      value = 0;
> -      opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
> -      if (opt_index == cl_options_count && opt[1] == 'W')
> -       {
> -         /* We don't generate warnings for unknown -Wno-* options
> -             unless we issue diagnostics.  */
> +  result = decode_cmdline_option (argv, lang_mask, &decoded);
> +  if (decoded.opt_index == cl_options_count)
> +    {
> +      if (opt[1] == 'W' && opt[2] == 'n' && opt[3] == 'o' && opt[4] == '-')
> +       /* We don't generate warnings for unknown -Wno-* options
> +          unless we issue diagnostics.  */
>          postpone_unknown_option_warning (argv[0]);
> -         result = 1;
> -         goto done;
> -       }
> +      else
> +       error ("unrecognized command line option %qs", opt);
> +      return result;
>     }
>
> -  if (opt_index == cl_options_count)
> -    goto done;
> -
> -  option = &cl_options[opt_index];
> -
> -  /* Reject negative form of switches that don't take negatives as
> -     unrecognized.  */
> -  if (!value && (option->flags & CL_REJECT_NEGATIVE))
> -    goto done;
> -
> -  /* We've recognized this switch.  */
> -  result = 1;
> +  option = &cl_options[decoded.opt_index];
>
> -  /* Check to see if the option is disabled for this configuration.  */
> -  if (option->flags & CL_DISABLED)
> +  if (decoded.errors & CL_ERR_DISABLED)
>     {
>       error ("command line option %qs"
>             " is not supported by this configuration", opt);
>       goto done;
>     }
>
> -  /* Sort out any argument the switch takes.  */
> -  if (option->flags & CL_JOINED)
> -    {
> -      /* Have arg point to the original switch.  This is because
> -        some code, such as disable_builtin_function, expects its
> -        argument to be persistent until the program exits.  */
> -      arg = argv[0] + cl_options[opt_index].opt_len + 1;
> -      if (!value)
> -       arg += strlen ("no-");
> -
> -      if (*arg == '\0' && !(option->flags & CL_MISSING_OK))
> -       {
> -         if (option->flags & CL_SEPARATE)
> -           {
> -             arg = argv[1];
> -             result = 2;
> -           }
> -         else
> -           /* Missing argument.  */
> -           arg = NULL;
> -       }
> -    }
> -  else if (option->flags & CL_SEPARATE)
> -    {
> -      arg = argv[1];
> -      result = 2;
> -    }
> -
> -  /* Now we've swallowed any potential argument, complain if this
> -     is a switch for a different front end.  */
> -  if (!(option->flags & (lang_mask | CL_COMMON | CL_TARGET)))
> +  if (decoded.errors & CL_ERR_WRONG_LANG)
>     {
>       complain_wrong_lang (argv[0], option, lang_mask);
>       goto done;
>     }
> -  else if ((option->flags & CL_TARGET)
> -          && (option->flags & CL_LANG_ALL)
> -          && !(option->flags & lang_mask))
> +
> +  if (decoded.errors & CL_ERR_MISSING_ARG)
>     {
> -      /* Complain for target flag language mismatches if any languages
> -        are specified.  */
> -      complain_wrong_lang (argv[0], option, lang_mask);
> +      if (!lang_hooks.missing_argument (opt, decoded.opt_index))
> +       error ("missing argument to %qs", opt);
>       goto done;
>     }
>
> -  if (arg == NULL && (option->flags & (CL_JOINED | CL_SEPARATE)))
> +  if (decoded.errors & CL_ERR_UINT_ARG)
>     {
> -      if (!lang_hooks.missing_argument (opt, opt_index))
> -       error ("missing argument to \"%s\"", opt);
> +      error ("argument to %qs should be a non-negative integer",
> +            option->opt_text);
>       goto done;
>     }
>
> -  /* If the switch takes an integer, convert it.  */
> -  if (arg && (option->flags & CL_UINTEGER))
> -    {
> -      value = integral_argument (arg);
> -      if (value == -1)
> -       {
> -         error ("argument to \"%s\" should be a non-negative integer",
> -                option->opt_text);
> -         goto done;
> -       }
> -    }
> +  gcc_assert (!decoded.errors);
>
> -  if (!handle_option (opt_index, value, arg, lang_mask, DK_UNSPECIFIED))
> -    result = 0;
> +  if (!handle_option (decoded.opt_index, decoded.value, decoded.arg,
> +                     lang_mask, DK_UNSPECIFIED))
> +    error ("unrecognized command line option %qs", opt);
>
>  done:
> -  if (dup)
> -    free (dup);
>   return result;
>  }
>
> @@ -780,12 +694,6 @@ read_cmdline_options (unsigned int argc,
>        }
>
>       n = read_cmdline_option (argv + i, lang_mask);
> -
> -      if (!n)
> -       {
> -         n = 1;
> -         error ("unrecognized command line option \"%s\"", opt);
> -       }
>     }
>  }
>
> Index: opts.h
> ===================================================================
> --- opts.h      (revision 160599)
> +++ opts.h      (working copy)
> @@ -1,5 +1,5 @@
>  /* Command line option handling.
> -   Copyright (C) 2002, 2003, 2004, 2005, 2007, 2008
> +   Copyright (C) 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010
>    Free Software Foundation, Inc.
>
>  This file is part of GCC.
> @@ -90,6 +90,34 @@ extern const unsigned int cl_lang_count;
>  #define CL_UINTEGER            (1 << 29) /* Argument is an integer >=0.  */
>  #define CL_UNDOCUMENTED                (1 << 30) /* Do not output with --help.  */
>
> +/* Possible ways in which a command-line option may be erroneous.
> +   These do not include not being known at all; an option index of
> +   cl_options_count is used for that.  */
> +
> +#define CL_ERR_DISABLED                (1 << 0) /* Disabled in this configuration.  */
> +#define CL_ERR_MISSING_ARG     (1 << 1) /* Argument required but missing.  */
> +#define CL_ERR_WRONG_LANG      (1 << 2) /* Option for wrong language.  */
> +#define CL_ERR_UINT_ARG                (1 << 3) /* Bad unsigned integer argument.  */
> +
> +/* Structure describing the result of decoding an option.  */
> +
> +struct cl_decoded_option
> +{
> +  /* The index of this option, or cl_options_count if not known.  */
> +  size_t opt_index;
> +
> +  /* The string argument, or NULL if none.  */
> +  const char *arg;
> +
> +  /* For a boolean option, 1 for the true case and 0 for the "no-"
> +     case.  For an unsigned integer option, the value of the
> +     argument.  1 in all other cases.  */
> +  int value;
> +
> +  /* Any flags describing errors detected in this option.  */
> +  int errors;
> +};
> +
>  /* Input file names.  */
>
>  extern const char **in_fnames;
> @@ -99,6 +127,10 @@ extern const char **in_fnames;
>  extern unsigned num_in_fnames;
>
>  size_t find_opt (const char *input, int lang_mask);
> +extern int integral_argument (const char *arg);
> +extern unsigned int decode_cmdline_option (const char **argv,
> +                                          unsigned int lang_mask,
> +                                          struct cl_decoded_option *decoded);
>  extern void prune_options (int *argcp, char ***argvp);
>  extern void decode_options (unsigned int argc, const char **argv);
>  extern int option_enabled (int opt_idx);
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>
diff mbox

Patch

Index: opts-common.c
===================================================================
--- opts-common.c	(revision 160599)
+++ opts-common.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* Command line option handling.
-   Copyright (C) 2006, 2007, 2008 Free Software Foundation, Inc.
+   Copyright (C) 2006, 2007, 2008, 2010 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -22,6 +22,7 @@  along with GCC; see the file COPYING3.  
 #include "intl.h"
 #include "coretypes.h"
 #include "opts.h"
+#include "options.h"
 
 /* Perform a binary search to find which option the command-line INPUT
    matches.  Returns its index in the option array, and N_OPTS
@@ -107,6 +108,138 @@  find_opt (const char *input, int lang_ma
   return match_wrong_lang;
 }
 
+/* If ARG is a non-negative integer made up solely of digits, return its
+   value, otherwise return -1.  */
+
+int
+integral_argument (const char *arg)
+{
+  const char *p = arg;
+
+  while (*p && ISDIGIT (*p))
+    p++;
+
+  if (*p == '\0')
+    return atoi (arg);
+
+  return -1;
+}
+
+/* Decode the switch beginning at ARGV for the language indicated by
+   LANG_MASK, into the structure *DECODED.  Returns the number of
+   switches consumed.  */
+
+unsigned int
+decode_cmdline_option (const char **argv, unsigned int lang_mask,
+		       struct cl_decoded_option *decoded)
+{
+  size_t opt_index;
+  const char *opt, *arg = 0;
+  char *dup = 0;
+  int value = 1;
+  unsigned int result = 1;
+  const struct cl_option *option;
+  int errors = 0;
+
+  opt = argv[0];
+
+  opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
+  if (opt_index == cl_options_count
+      && (opt[1] == 'W' || opt[1] == 'f' || opt[1] == 'm')
+      && opt[2] == 'n' && opt[3] == 'o' && opt[4] == '-')
+    {
+      /* Drop the "no-" from negative switches.  */
+      size_t len = strlen (opt) - 3;
+
+      dup = XNEWVEC (char, len + 1);
+      dup[0] = '-';
+      dup[1] = opt[1];
+      memcpy (dup + 2, opt + 5, len - 2 + 1);
+      opt = dup;
+      value = 0;
+      opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
+    }
+
+  if (opt_index == cl_options_count)
+    goto done;
+
+  option = &cl_options[opt_index];
+
+  /* Reject negative form of switches that don't take negatives as
+     unrecognized.  */
+  if (!value && (option->flags & CL_REJECT_NEGATIVE))
+    {
+      opt_index = cl_options_count;
+      goto done;
+    }
+
+  /* Check to see if the option is disabled for this configuration.  */
+  if (option->flags & CL_DISABLED)
+    errors |= CL_ERR_DISABLED;
+
+  /* Sort out any argument the switch takes.  */
+  if (option->flags & CL_JOINED)
+    {
+      /* Have arg point to the original switch.  This is because
+	 some code, such as disable_builtin_function, expects its
+	 argument to be persistent until the program exits.  */
+      arg = argv[0] + cl_options[opt_index].opt_len + 1;
+      if (!value)
+	arg += strlen ("no-");
+
+      if (*arg == '\0' && !(option->flags & CL_MISSING_OK))
+	{
+	  if (option->flags & CL_SEPARATE)
+	    {
+	      arg = argv[1];
+	      result = 2;
+	      if (arg == NULL)
+		result = 1;
+	    }
+	  else
+	    /* Missing argument.  */
+	    arg = NULL;
+	}
+    }
+  else if (option->flags & CL_SEPARATE)
+    {
+      arg = argv[1];
+      result = 2;
+      if (arg == NULL)
+	result = 1;
+    }
+
+  /* Check if this is a switch for a different front end.  */
+  if (!(option->flags & (lang_mask | CL_COMMON | CL_TARGET)))
+    errors |= CL_ERR_WRONG_LANG;
+  else if ((option->flags & CL_TARGET)
+	   && (option->flags & CL_LANG_ALL)
+	   && !(option->flags & lang_mask))
+    /* Complain for target flag language mismatches if any languages
+       are specified.  */
+      errors |= CL_ERR_WRONG_LANG;
+
+  if (arg == NULL && (option->flags & (CL_JOINED | CL_SEPARATE)))
+    errors |= CL_ERR_MISSING_ARG;
+
+  /* If the switch takes an integer, convert it.  */
+  if (arg && (option->flags & CL_UINTEGER))
+    {
+      value = integral_argument (arg);
+      if (value == -1)
+	errors |= CL_ERR_UINT_ARG;
+    }
+
+ done:
+  if (dup)
+    free (dup);
+  decoded->opt_index = opt_index;
+  decoded->arg = arg;
+  decoded->value = value;
+  decoded->errors = errors;
+  return result;
+}
+
 /* Return true if NEXT_OPT_IDX cancels OPT_IDX.  Return false if the
    next one is the same as ORIG_NEXT_OPT_IDX.  */
 
Index: testsuite/gcc.dg/funroll-loops-all.c
===================================================================
--- testsuite/gcc.dg/funroll-loops-all.c	(revision 160599)
+++ testsuite/gcc.dg/funroll-loops-all.c	(working copy)
@@ -1,4 +1,4 @@ 
 /* PR 17594 */
 /* { dg-do compile } */
 /* { dg-options "-funroll-loops-all" } */
-/* { dg-error "unrecognized command line option \"-funroll-loops-all\"" "" { target *-*-* } 0 } */
+/* { dg-error "unrecognized command line option '-funroll-loops-all'" "" { target *-*-* } 0 } */
Index: opts.c
===================================================================
--- opts.c	(revision 160599)
+++ opts.c	(working copy)
@@ -381,22 +381,6 @@  static void complain_wrong_lang (const c
 static void set_debug_level (enum debug_info_type type, int extended,
 			     const char *arg);
 
-/* If ARG is a non-negative integer made up solely of digits, return its
-   value, otherwise return -1.  */
-static int
-integral_argument (const char *arg)
-{
-  const char *p = arg;
-
-  while (*p && ISDIGIT (*p))
-    p++;
-
-  if (*p == '\0')
-    return atoi (arg);
-
-  return -1;
-}
-
 /* Return a malloced slash-separated list of languages in MASK.  */
 static char *
 write_langs (unsigned int mask)
@@ -536,131 +520,61 @@  handle_option (int opt_index, int value,
 static unsigned int
 read_cmdline_option (const char **argv, unsigned int lang_mask)
 {
-  size_t opt_index;
-  const char *opt, *arg = 0;
-  char *dup = 0;
-  int value = 1;
-  unsigned int result = 0;
+  struct cl_decoded_option decoded;
+  unsigned int result;
+  const char *opt;
   const struct cl_option *option;
 
   opt = argv[0];
 
-  opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
-  if (opt_index == cl_options_count
-      && (opt[1] == 'W' || opt[1] == 'f' || opt[1] == 'm')
-      && opt[2] == 'n' && opt[3] == 'o' && opt[4] == '-')
-    {
-      /* Drop the "no-" from negative switches.  */
-      size_t len = strlen (opt) - 3;
-
-      dup = XNEWVEC (char, len + 1);
-      dup[0] = '-';
-      dup[1] = opt[1];
-      memcpy (dup + 2, opt + 5, len - 2 + 1);
-      opt = dup;
-      value = 0;
-      opt_index = find_opt (opt + 1, lang_mask | CL_COMMON | CL_TARGET);
-      if (opt_index == cl_options_count && opt[1] == 'W')
-	{
-	  /* We don't generate warnings for unknown -Wno-* options
-             unless we issue diagnostics.  */
+  result = decode_cmdline_option (argv, lang_mask, &decoded);
+  if (decoded.opt_index == cl_options_count)
+    {
+      if (opt[1] == 'W' && opt[2] == 'n' && opt[3] == 'o' && opt[4] == '-')
+	/* We don't generate warnings for unknown -Wno-* options
+	   unless we issue diagnostics.  */
 	  postpone_unknown_option_warning (argv[0]);
-	  result = 1;
-	  goto done;
-	}
+      else
+	error ("unrecognized command line option %qs", opt);
+      return result;
     }
 
-  if (opt_index == cl_options_count)
-    goto done;
-
-  option = &cl_options[opt_index];
-
-  /* Reject negative form of switches that don't take negatives as
-     unrecognized.  */
-  if (!value && (option->flags & CL_REJECT_NEGATIVE))
-    goto done;
-
-  /* We've recognized this switch.  */
-  result = 1;
+  option = &cl_options[decoded.opt_index];
 
-  /* Check to see if the option is disabled for this configuration.  */
-  if (option->flags & CL_DISABLED)
+  if (decoded.errors & CL_ERR_DISABLED)
     {
       error ("command line option %qs"
 	     " is not supported by this configuration", opt);
       goto done;
     }
 
-  /* Sort out any argument the switch takes.  */
-  if (option->flags & CL_JOINED)
-    {
-      /* Have arg point to the original switch.  This is because
-	 some code, such as disable_builtin_function, expects its
-	 argument to be persistent until the program exits.  */
-      arg = argv[0] + cl_options[opt_index].opt_len + 1;
-      if (!value)
-	arg += strlen ("no-");
-
-      if (*arg == '\0' && !(option->flags & CL_MISSING_OK))
-	{
-	  if (option->flags & CL_SEPARATE)
-	    {
-	      arg = argv[1];
-	      result = 2;
-	    }
-	  else
-	    /* Missing argument.  */
-	    arg = NULL;
-	}
-    }
-  else if (option->flags & CL_SEPARATE)
-    {
-      arg = argv[1];
-      result = 2;
-    }
-
-  /* Now we've swallowed any potential argument, complain if this
-     is a switch for a different front end.  */
-  if (!(option->flags & (lang_mask | CL_COMMON | CL_TARGET)))
+  if (decoded.errors & CL_ERR_WRONG_LANG)
     {
       complain_wrong_lang (argv[0], option, lang_mask);
       goto done;
     }
-  else if ((option->flags & CL_TARGET)
-	   && (option->flags & CL_LANG_ALL)
-	   && !(option->flags & lang_mask))
+
+  if (decoded.errors & CL_ERR_MISSING_ARG)
     {
-      /* Complain for target flag language mismatches if any languages
-	 are specified.  */
-      complain_wrong_lang (argv[0], option, lang_mask);
+      if (!lang_hooks.missing_argument (opt, decoded.opt_index))
+	error ("missing argument to %qs", opt);
       goto done;
     }
 
-  if (arg == NULL && (option->flags & (CL_JOINED | CL_SEPARATE)))
+  if (decoded.errors & CL_ERR_UINT_ARG)
     {
-      if (!lang_hooks.missing_argument (opt, opt_index))
-	error ("missing argument to \"%s\"", opt);
+      error ("argument to %qs should be a non-negative integer",
+	     option->opt_text);
       goto done;
     }
 
-  /* If the switch takes an integer, convert it.  */
-  if (arg && (option->flags & CL_UINTEGER))
-    {
-      value = integral_argument (arg);
-      if (value == -1)
-	{
-	  error ("argument to \"%s\" should be a non-negative integer",
-		 option->opt_text);
-	  goto done;
-	}
-    }
+  gcc_assert (!decoded.errors);
 
-  if (!handle_option (opt_index, value, arg, lang_mask, DK_UNSPECIFIED))
-    result = 0;
+  if (!handle_option (decoded.opt_index, decoded.value, decoded.arg,
+		      lang_mask, DK_UNSPECIFIED))
+    error ("unrecognized command line option %qs", opt);
 
  done:
-  if (dup)
-    free (dup);
   return result;
 }
 
@@ -780,12 +694,6 @@  read_cmdline_options (unsigned int argc,
 	}
 
       n = read_cmdline_option (argv + i, lang_mask);
-
-      if (!n)
-	{
-	  n = 1;
-	  error ("unrecognized command line option \"%s\"", opt);
-	}
     }
 }
 
Index: opts.h
===================================================================
--- opts.h	(revision 160599)
+++ opts.h	(working copy)
@@ -1,5 +1,5 @@ 
 /* Command line option handling.
-   Copyright (C) 2002, 2003, 2004, 2005, 2007, 2008
+   Copyright (C) 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010
    Free Software Foundation, Inc.
 
 This file is part of GCC.
@@ -90,6 +90,34 @@  extern const unsigned int cl_lang_count;
 #define CL_UINTEGER		(1 << 29) /* Argument is an integer >=0.  */
 #define CL_UNDOCUMENTED		(1 << 30) /* Do not output with --help.  */
 
+/* Possible ways in which a command-line option may be erroneous.
+   These do not include not being known at all; an option index of
+   cl_options_count is used for that.  */
+
+#define CL_ERR_DISABLED		(1 << 0) /* Disabled in this configuration.  */
+#define CL_ERR_MISSING_ARG	(1 << 1) /* Argument required but missing.  */
+#define CL_ERR_WRONG_LANG	(1 << 2) /* Option for wrong language.  */
+#define CL_ERR_UINT_ARG		(1 << 3) /* Bad unsigned integer argument.  */
+
+/* Structure describing the result of decoding an option.  */
+
+struct cl_decoded_option
+{
+  /* The index of this option, or cl_options_count if not known.  */
+  size_t opt_index;
+
+  /* The string argument, or NULL if none.  */
+  const char *arg;
+
+  /* For a boolean option, 1 for the true case and 0 for the "no-"
+     case.  For an unsigned integer option, the value of the
+     argument.  1 in all other cases.  */
+  int value;
+
+  /* Any flags describing errors detected in this option.  */
+  int errors;
+};
+
 /* Input file names.  */
 
 extern const char **in_fnames;
@@ -99,6 +127,10 @@  extern const char **in_fnames;
 extern unsigned num_in_fnames;
 
 size_t find_opt (const char *input, int lang_mask);
+extern int integral_argument (const char *arg);
+extern unsigned int decode_cmdline_option (const char **argv,
+					   unsigned int lang_mask,
+					   struct cl_decoded_option *decoded);
 extern void prune_options (int *argcp, char ***argvp);
 extern void decode_options (unsigned int argc, const char **argv);
 extern int option_enabled (int opt_idx);