* [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)
@ 2016-09-05 18:03 Jakub Jelinek
2016-09-05 19:42 ` Uros Bizjak
2016-09-05 19:44 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Manuel López-Ibáñez
0 siblings, 2 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-09-05 18:03 UTC (permalink / raw)
To: Uros Bizjak, David Malcolm; +Cc: gcc-patches
Hi!
While most of the i386.opt -m....= options have enum args and thus
cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy= (and also
-mrecip=) don't use that, with the CPU strings being maintained inside of a
function rather than in some *.def file that could be also sourced into the
*.opt or something (and similarly for the strategies).
This patch adds inform calls that handle those similarly to what
cmdline_handle_error does for the options with enum values.
In addition, it adds %qs instead of %s in a couple of spaces, and
stops reporting incorrect attribute option("march=...") when it is
target("march=...") etc.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-09-05 Jakub Jelinek <jakub@redhat.com>
PR middle-end/77475
* config/i386/i386.c: Include spellcheck.h.
(ix86_parse_stringop_strategy_string): Simplify, use %qs instead of %s
where desirable, use argument instead of arg in the diagnostic
wording, add list of supported strategies and spellcheck hint.
(ix86_option_override_internal): Emit target("m...") instead of
option("m...") in the diagnostic. Use %qs instead of %s in invalid
-march/-mtune option diagnostic. Add list of supported arches/tunings
and spellcheck hint.
* gcc.target/i386/pr65990.c: Adjust expected diagnostics.
--- gcc/config/i386/i386.c.jj 2016-09-05 12:41:03.000000000 +0200
+++ gcc/config/i386/i386.c 2016-09-05 16:44:45.184981211 +0200
@@ -76,6 +76,7 @@ along with GCC; see the file COPYING3.
#include "case-cfn-macros.h"
#include "regrename.h"
#include "dojump.h"
+#include "spellcheck.h"
/* This file should be included last. */
#include "target-def.h"
@@ -4533,19 +4534,19 @@ ix86_parse_stringop_strategy_string (cha
next_range_str = strchr (curr_range_str, ',');
if (next_range_str)
*next_range_str++ = '\0';
+ const char *opt
+ = is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=";
if (3 != sscanf (curr_range_str, "%20[^:]:%d:%10s",
alg_name, &maxs, align))
{
- error ("wrong arg %s to option %s", curr_range_str,
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("wrong argument %qs to option %qs", curr_range_str, opt);
return;
}
if (n > 0 && (maxs < (input_ranges[n - 1].max + 1) && maxs != -1))
{
- error ("size ranges of option %s should be increasing",
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("size ranges of option %qs should be increasing", opt);
return;
}
@@ -4555,9 +4556,35 @@ ix86_parse_stringop_strategy_string (cha
if (i == last_alg)
{
- error ("wrong stringop strategy name %s specified for option %s",
- alg_name,
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("wrong stringop strategy name %qs specified for option %s",
+ alg_name, opt);
+
+ size_t len = 0;
+ for (i = 0; i < last_alg; i++)
+ len += strlen (stringop_alg_names[i]) + 1;
+
+ char *s, *p;
+ auto_vec <const char *> candidates;
+ s = XALLOCAVEC (char, len);
+ p = s;
+ for (i = 0; i < last_alg; i++)
+ if ((stringop_alg) i != rep_prefix_8_byte || TARGET_64BIT)
+ {
+ size_t arglen = strlen (stringop_alg_names[i]);
+ memcpy (p, stringop_alg_names[i], arglen);
+ p[arglen] = ' ';
+ p += arglen + 1;
+ candidates.safe_push (stringop_alg_names[i]);
+ }
+ p[-1] = 0;
+ const char *hint = find_closest_string (alg_name, &candidates);
+ if (hint)
+ inform (input_location,
+ "valid arguments to %qs are: %s; did you mean %qs?",
+ opt, s, hint);
+ else
+ inform (input_location, "valid arguments to %qs are: %s",
+ opt, s);
return;
}
@@ -4565,10 +4592,8 @@ ix86_parse_stringop_strategy_string (cha
&& !TARGET_64BIT)
{
/* rep; movq isn't available in 32-bit code. */
- error ("stringop strategy name %s specified for option %s "
- "not supported for 32-bit code",
- alg_name,
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("stringop strategy name %qs specified for option %qs "
+ "not supported for 32-bit code", alg_name, opt);
return;
}
@@ -4580,8 +4605,7 @@ ix86_parse_stringop_strategy_string (cha
input_ranges[n].noalign = true;
else
{
- error ("unknown alignment %s specified for option %s",
- align, is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("unknown alignment %qs specified for option %qs", align, opt);
return;
}
n++;
@@ -5041,7 +5065,7 @@ ix86_option_override_internal (bool main
}
else
{
- prefix = "option(\"";
+ prefix = "target(\"";
suffix = "\")";
sw = "attribute";
}
@@ -5480,8 +5504,41 @@ ix86_option_override_internal (bool main
error ("intel CPU can be used only for %stune=%s %s",
prefix, suffix, sw);
else if (i == pta_size)
- error ("bad value (%s) for %sarch=%s %s",
- opts->x_ix86_arch_string, prefix, suffix, sw);
+ {
+ error ("bad value (%qs) for %<%sarch=%s%> %s",
+ opts->x_ix86_arch_string, prefix, suffix, sw);
+
+ size_t len = 0;
+ for (i = 0; i < pta_size; i++)
+ len += strlen (processor_alias_table[i].name) + 1;
+
+ char *s, *p;
+ auto_vec <const char *> candidates;
+ s = XALLOCAVEC (char, len);
+ p = s;
+ for (i = 0; i < pta_size; i++)
+ if (strcmp (processor_alias_table[i].name, "generic")
+ && strcmp (processor_alias_table[i].name, "intel")
+ && (!TARGET_64BIT_P (opts->x_ix86_isa_flags)
+ || (processor_alias_table[i].flags & PTA_64BIT)))
+ {
+ size_t arglen = strlen (processor_alias_table[i].name);
+ memcpy (p, processor_alias_table[i].name, arglen);
+ p[arglen] = ' ';
+ p += arglen + 1;
+ candidates.safe_push (processor_alias_table[i].name);
+ }
+ p[-1] = 0;
+ const char *hint
+ = find_closest_string (opts->x_ix86_arch_string, &candidates);
+ if (hint)
+ inform (input_location,
+ "valid arguments to %<%sarch=%s%> %s are: %s; "
+ "did you mean %qs?", prefix, suffix, sw, s, hint);
+ else
+ inform (input_location, "valid arguments to %<%sarch=%s%> %s are: %s",
+ prefix, suffix, sw, s);
+ }
ix86_arch_mask = 1u << ix86_arch;
for (i = 0; i < X86_ARCH_LAST; ++i)
@@ -5523,8 +5580,40 @@ ix86_option_override_internal (bool main
}
if (ix86_tune_specified && i == pta_size)
- error ("bad value (%s) for %stune=%s %s",
- opts->x_ix86_tune_string, prefix, suffix, sw);
+ {
+ error ("bad value (%qs) for %<%stune=%s%> %s",
+ opts->x_ix86_tune_string, prefix, suffix, sw);
+
+ size_t len = 0;
+ for (i = 0; i < pta_size; i++)
+ len += strlen (processor_alias_table[i].name) + 1;
+
+ char *s, *p;
+ auto_vec <const char *> candidates;
+ s = XALLOCAVEC (char, len);
+ p = s;
+ for (i = 0; i < pta_size; i++)
+ if (!TARGET_64BIT_P (opts->x_ix86_isa_flags)
+ || (processor_alias_table[i].flags & PTA_64BIT))
+ {
+ size_t arglen = strlen (processor_alias_table[i].name);
+ memcpy (p, processor_alias_table[i].name, arglen);
+ p[arglen] = ' ';
+ p += arglen + 1;
+ candidates.safe_push (processor_alias_table[i].name);
+ }
+ p[-1] = 0;
+ const char *hint
+ = find_closest_string (opts->x_ix86_tune_string, &candidates);
+ if (hint)
+ inform (input_location,
+ "valid arguments to %<%stune=%s%> %s are: %s; "
+ "did you mean %qs?", prefix, suffix, sw, s, hint);
+ else
+ inform (input_location, "valid arguments to %<%stune=%s%> %s are: %s",
+ prefix, suffix, sw, s);
+
+ }
set_ix86_tune_features (ix86_tune, opts->x_ix86_dump_tunes);
--- gcc/testsuite/gcc.target/i386/pr65990.c.jj 2016-05-22 12:20:14.000000000 +0200
+++ gcc/testsuite/gcc.target/i386/pr65990.c 2016-09-05 19:00:48.000000000 +0200
@@ -1,7 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-mtune=btver2 -mmemcpy-strategy=rep_8byte:-1:noalign" }
-/* { dg-error "stringop strategy name rep_8byte specified for option -mmemcpy_strategy= not supported for 32-bit code" "" { target ia32 } 0 } */
+/* { dg-error "stringop strategy name 'rep_8byte' specified for option '-mmemcpy_strategy=' not supported for 32-bit code" "" { target ia32 } 0 } */
struct U9
{
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)
2016-09-05 18:03 [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Jakub Jelinek
@ 2016-09-05 19:42 ` Uros Bizjak
2016-09-06 0:20 ` David Malcolm
2016-09-06 20:41 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, i386 part) Jakub Jelinek
2016-09-05 19:44 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Manuel López-Ibáñez
1 sibling, 2 replies; 12+ messages in thread
From: Uros Bizjak @ 2016-09-05 19:42 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: David Malcolm, gcc-patches
On Mon, Sep 5, 2016 at 7:25 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> While most of the i386.opt -m....= options have enum args and thus
> cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy= (and also
> -mrecip=) don't use that, with the CPU strings being maintained inside of a
> function rather than in some *.def file that could be also sourced into the
> *.opt or something (and similarly for the strategies).
>
> This patch adds inform calls that handle those similarly to what
> cmdline_handle_error does for the options with enum values.
> In addition, it adds %qs instead of %s in a couple of spaces, and
> stops reporting incorrect attribute option("march=...") when it is
> target("march=...") etc.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2016-09-05 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/77475
> * config/i386/i386.c: Include spellcheck.h.
> (ix86_parse_stringop_strategy_string): Simplify, use %qs instead of %s
> where desirable, use argument instead of arg in the diagnostic
> wording, add list of supported strategies and spellcheck hint.
> (ix86_option_override_internal): Emit target("m...") instead of
> option("m...") in the diagnostic. Use %qs instead of %s in invalid
> -march/-mtune option diagnostic. Add list of supported arches/tunings
> and spellcheck hint.
>
> * gcc.target/i386/pr65990.c: Adjust expected diagnostics.
OK as far as x86 target is concerned, but please allow a day for David
to eventually comment on the implementation.
Thanks,
Uros.
> --- gcc/config/i386/i386.c.jj 2016-09-05 12:41:03.000000000 +0200
> +++ gcc/config/i386/i386.c 2016-09-05 16:44:45.184981211 +0200
> @@ -76,6 +76,7 @@ along with GCC; see the file COPYING3.
> #include "case-cfn-macros.h"
> #include "regrename.h"
> #include "dojump.h"
> +#include "spellcheck.h"
>
> /* This file should be included last. */
> #include "target-def.h"
> @@ -4533,19 +4534,19 @@ ix86_parse_stringop_strategy_string (cha
> next_range_str = strchr (curr_range_str, ',');
> if (next_range_str)
> *next_range_str++ = '\0';
> + const char *opt
> + = is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=";
>
> if (3 != sscanf (curr_range_str, "%20[^:]:%d:%10s",
> alg_name, &maxs, align))
> {
> - error ("wrong arg %s to option %s", curr_range_str,
> - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("wrong argument %qs to option %qs", curr_range_str, opt);
> return;
> }
>
> if (n > 0 && (maxs < (input_ranges[n - 1].max + 1) && maxs != -1))
> {
> - error ("size ranges of option %s should be increasing",
> - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("size ranges of option %qs should be increasing", opt);
> return;
> }
>
> @@ -4555,9 +4556,35 @@ ix86_parse_stringop_strategy_string (cha
>
> if (i == last_alg)
> {
> - error ("wrong stringop strategy name %s specified for option %s",
> - alg_name,
> - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("wrong stringop strategy name %qs specified for option %s",
> + alg_name, opt);
> +
> + size_t len = 0;
> + for (i = 0; i < last_alg; i++)
> + len += strlen (stringop_alg_names[i]) + 1;
> +
> + char *s, *p;
> + auto_vec <const char *> candidates;
> + s = XALLOCAVEC (char, len);
> + p = s;
> + for (i = 0; i < last_alg; i++)
> + if ((stringop_alg) i != rep_prefix_8_byte || TARGET_64BIT)
> + {
> + size_t arglen = strlen (stringop_alg_names[i]);
> + memcpy (p, stringop_alg_names[i], arglen);
> + p[arglen] = ' ';
> + p += arglen + 1;
> + candidates.safe_push (stringop_alg_names[i]);
> + }
> + p[-1] = 0;
> + const char *hint = find_closest_string (alg_name, &candidates);
> + if (hint)
> + inform (input_location,
> + "valid arguments to %qs are: %s; did you mean %qs?",
> + opt, s, hint);
> + else
> + inform (input_location, "valid arguments to %qs are: %s",
> + opt, s);
> return;
> }
>
> @@ -4565,10 +4592,8 @@ ix86_parse_stringop_strategy_string (cha
> && !TARGET_64BIT)
> {
> /* rep; movq isn't available in 32-bit code. */
> - error ("stringop strategy name %s specified for option %s "
> - "not supported for 32-bit code",
> - alg_name,
> - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("stringop strategy name %qs specified for option %qs "
> + "not supported for 32-bit code", alg_name, opt);
> return;
> }
>
> @@ -4580,8 +4605,7 @@ ix86_parse_stringop_strategy_string (cha
> input_ranges[n].noalign = true;
> else
> {
> - error ("unknown alignment %s specified for option %s",
> - align, is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("unknown alignment %qs specified for option %qs", align, opt);
> return;
> }
> n++;
> @@ -5041,7 +5065,7 @@ ix86_option_override_internal (bool main
> }
> else
> {
> - prefix = "option(\"";
> + prefix = "target(\"";
> suffix = "\")";
> sw = "attribute";
> }
> @@ -5480,8 +5504,41 @@ ix86_option_override_internal (bool main
> error ("intel CPU can be used only for %stune=%s %s",
> prefix, suffix, sw);
> else if (i == pta_size)
> - error ("bad value (%s) for %sarch=%s %s",
> - opts->x_ix86_arch_string, prefix, suffix, sw);
> + {
> + error ("bad value (%qs) for %<%sarch=%s%> %s",
> + opts->x_ix86_arch_string, prefix, suffix, sw);
> +
> + size_t len = 0;
> + for (i = 0; i < pta_size; i++)
> + len += strlen (processor_alias_table[i].name) + 1;
> +
> + char *s, *p;
> + auto_vec <const char *> candidates;
> + s = XALLOCAVEC (char, len);
> + p = s;
> + for (i = 0; i < pta_size; i++)
> + if (strcmp (processor_alias_table[i].name, "generic")
> + && strcmp (processor_alias_table[i].name, "intel")
> + && (!TARGET_64BIT_P (opts->x_ix86_isa_flags)
> + || (processor_alias_table[i].flags & PTA_64BIT)))
> + {
> + size_t arglen = strlen (processor_alias_table[i].name);
> + memcpy (p, processor_alias_table[i].name, arglen);
> + p[arglen] = ' ';
> + p += arglen + 1;
> + candidates.safe_push (processor_alias_table[i].name);
> + }
> + p[-1] = 0;
> + const char *hint
> + = find_closest_string (opts->x_ix86_arch_string, &candidates);
> + if (hint)
> + inform (input_location,
> + "valid arguments to %<%sarch=%s%> %s are: %s; "
> + "did you mean %qs?", prefix, suffix, sw, s, hint);
> + else
> + inform (input_location, "valid arguments to %<%sarch=%s%> %s are: %s",
> + prefix, suffix, sw, s);
> + }
>
> ix86_arch_mask = 1u << ix86_arch;
> for (i = 0; i < X86_ARCH_LAST; ++i)
> @@ -5523,8 +5580,40 @@ ix86_option_override_internal (bool main
> }
>
> if (ix86_tune_specified && i == pta_size)
> - error ("bad value (%s) for %stune=%s %s",
> - opts->x_ix86_tune_string, prefix, suffix, sw);
> + {
> + error ("bad value (%qs) for %<%stune=%s%> %s",
> + opts->x_ix86_tune_string, prefix, suffix, sw);
> +
> + size_t len = 0;
> + for (i = 0; i < pta_size; i++)
> + len += strlen (processor_alias_table[i].name) + 1;
> +
> + char *s, *p;
> + auto_vec <const char *> candidates;
> + s = XALLOCAVEC (char, len);
> + p = s;
> + for (i = 0; i < pta_size; i++)
> + if (!TARGET_64BIT_P (opts->x_ix86_isa_flags)
> + || (processor_alias_table[i].flags & PTA_64BIT))
> + {
> + size_t arglen = strlen (processor_alias_table[i].name);
> + memcpy (p, processor_alias_table[i].name, arglen);
> + p[arglen] = ' ';
> + p += arglen + 1;
> + candidates.safe_push (processor_alias_table[i].name);
> + }
> + p[-1] = 0;
> + const char *hint
> + = find_closest_string (opts->x_ix86_tune_string, &candidates);
> + if (hint)
> + inform (input_location,
> + "valid arguments to %<%stune=%s%> %s are: %s; "
> + "did you mean %qs?", prefix, suffix, sw, s, hint);
> + else
> + inform (input_location, "valid arguments to %<%stune=%s%> %s are: %s",
> + prefix, suffix, sw, s);
> +
> + }
>
> set_ix86_tune_features (ix86_tune, opts->x_ix86_dump_tunes);
>
> --- gcc/testsuite/gcc.target/i386/pr65990.c.jj 2016-05-22 12:20:14.000000000 +0200
> +++ gcc/testsuite/gcc.target/i386/pr65990.c 2016-09-05 19:00:48.000000000 +0200
> @@ -1,7 +1,7 @@
> /* { dg-do compile } */
> /* { dg-options "-mtune=btver2 -mmemcpy-strategy=rep_8byte:-1:noalign" }
>
> -/* { dg-error "stringop strategy name rep_8byte specified for option -mmemcpy_strategy= not supported for 32-bit code" "" { target ia32 } 0 } */
> +/* { dg-error "stringop strategy name 'rep_8byte' specified for option '-mmemcpy_strategy=' not supported for 32-bit code" "" { target ia32 } 0 } */
>
> struct U9
> {
>
> Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)
2016-09-05 18:03 [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Jakub Jelinek
2016-09-05 19:42 ` Uros Bizjak
@ 2016-09-05 19:44 ` Manuel López-Ibáñez
2016-09-05 20:00 ` Jakub Jelinek
2016-09-05 20:02 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Manuel López-Ibáñez
1 sibling, 2 replies; 12+ messages in thread
From: Manuel López-Ibáñez @ 2016-09-05 19:44 UTC (permalink / raw)
To: Jakub Jelinek, Uros Bizjak, David Malcolm; +Cc: GCC Patches
On 05/09/16 18:25, Jakub Jelinek wrote:
> Hi!
>
> While most of the i386.opt -m....= options have enum args and thus
> cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy= (and also
> -mrecip=) don't use that, with the CPU strings being maintained inside of a
> function rather than in some *.def file that could be also sourced into the
> *.opt or something (and similarly for the strategies).
>
> This patch adds inform calls that handle those similarly to what
> cmdline_handle_error does for the options with enum values.
> In addition, it adds %qs instead of %s in a couple of spaces, and
> stops reporting incorrect attribute option("march=...") when it is
> target("march=...") etc.
Something like the following should avoid a lot of (future) duplication (untested):
Index: opts-common.c
===================================================================
--- opts-common.c (revision 239995)
+++ opts-common.c (working copy)
@@ -1069,6 +1069,40 @@
decoded->errors = 0;
}
+static void
+candidates_to_string(char *s, const auto_vec <const char *> *candidates)
+{
+ int i;
+ const char *candidate;
+ char *p = s;
+ FOR_EACH_VEC_ELT (*candidates, i, candidate)
+ {
+ gcc_assert (candidate);
+ size_t arglen = strlen (candidate);
+ memcpy (p, candidate, arglen);
+ p[arglen] = ' ';
+ p += arglen + 1;
+ }
+ p[-1] = 0;
+}
+
+void
+unrecognized_argument_error (location_t loc, const char *opt, const char *arg,
+ const auto_vec <const char*> &candidates,
+ size_t total_len)
+{
+ error_at (loc, "argument %qs to %qs not recognized", arg, opt);
+
+ char *s = XALLOCAVEC (char, total_len);
+ candidates_to_string (s, &candidates);
+ const char *hint = find_closest_string (arg, &candidates);
+ if (hint)
+ inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
+ opt, s, hint);
+ else
+ inform (loc, "valid arguments to %qs are: %s", opt, s);
+}
+
/* Perform diagnostics for read_cmdline_option and control_warning_option
functions. Returns true if an error has been diagnosed.
LOC and LANG_MASK arguments like in read_cmdline_option.
@@ -1107,40 +1141,22 @@
if (errors & CL_ERR_ENUM_ARG)
{
const struct cl_enum *e = &cl_enums[option->var_enum];
- unsigned int i;
- size_t len;
- char *s, *p;
-
if (e->unknown_error)
error_at (loc, e->unknown_error, arg);
else
- error_at (loc, "unrecognized argument in option %qs", opt);
-
- len = 0;
- for (i = 0; e->values[i].arg != NULL; i++)
- len += strlen (e->values[i].arg) + 1;
-
- auto_vec <const char *> candidates;
- s = XALLOCAVEC (char, len);
- p = s;
- for (i = 0; e->values[i].arg != NULL; i++)
{
- if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
- continue;
- size_t arglen = strlen (e->values[i].arg);
- memcpy (p, e->values[i].arg, arglen);
- p[arglen] = ' ';
- p += arglen + 1;
- candidates.safe_push (e->values[i].arg);
+ size_t len = 0;
+ unsigned int i;
+ auto_vec <const char *> candidates;
+ for (i = 0; e->values[i].arg != NULL; i++)
+ {
+ if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
+ continue;
+ len += strlen (e->values[i].arg) + 1;
+ candidates.safe_push (e->values[i].arg);
+ }
+ unrecognized_argument_error (loc, opt, arg, candidates, len);
}
- p[-1] = 0;
- const char *hint = find_closest_string (arg, &candidates);
- if (hint)
- inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
- option->opt_text, s, hint);
- else
- inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
-
return true;
}
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)
2016-09-05 19:44 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Manuel López-Ibáñez
@ 2016-09-05 20:00 ` Jakub Jelinek
2016-09-06 20:40 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, generic part) Jakub Jelinek
2016-09-05 20:02 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Manuel López-Ibáñez
1 sibling, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2016-09-05 20:00 UTC (permalink / raw)
To: Manuel López-Ibáñez
Cc: Uros Bizjak, David Malcolm, GCC Patches
On Mon, Sep 05, 2016 at 08:42:37PM +0100, Manuel López-Ibáñez wrote:
> Something like the following should avoid a lot of (future) duplication (untested):
You're right. I've missed that I actually push the candidates into a vector
anyway, so the concatenation can be done in a common code.
> Index: opts-common.c
> ===================================================================
> --- opts-common.c (revision 239995)
> +++ opts-common.c (working copy)
> @@ -1069,6 +1069,40 @@
> decoded->errors = 0;
> }
>
> +static void
> +candidates_to_string(char *s, const auto_vec <const char *> *candidates)
Formatting, missing space before (.
> +{
> + int i;
> + const char *candidate;
> + char *p = s;
> + FOR_EACH_VEC_ELT (*candidates, i, candidate)
> + {
> + gcc_assert (candidate);
> + size_t arglen = strlen (candidate);
> + memcpy (p, candidate, arglen);
> + p[arglen] = ' ';
> + p += arglen + 1;
> + }
> + p[-1] = 0;
> +}
As I think this isn't performance sensitive code, I think it would be better
to simplify the callers and compute total_len inside of the following
function (i.e. walk the vector 3 times (once in unrecognized_argument_error
to compute total_len, once in candidates_to_string, once in
find_closest_string).
> +
> +void
> +unrecognized_argument_error (location_t loc, const char *opt, const char *arg,
> + const auto_vec <const char*> &candidates,
> + size_t total_len)
> +{
> + error_at (loc, "argument %qs to %qs not recognized", arg, opt);
> +
> + char *s = XALLOCAVEC (char, total_len);
> + candidates_to_string (s, &candidates);
> + const char *hint = find_closest_string (arg, &candidates);
> + if (hint)
> + inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
> + opt, s, hint);
> + else
> + inform (loc, "valid arguments to %qs are: %s", opt, s);
> +}
> +
> /* Perform diagnostics for read_cmdline_option and control_warning_option
> functions. Returns true if an error has been diagnosed.
> LOC and LANG_MASK arguments like in read_cmdline_option.
> @@ -1107,40 +1141,22 @@
> if (errors & CL_ERR_ENUM_ARG)
> {
> const struct cl_enum *e = &cl_enums[option->var_enum];
> - unsigned int i;
> - size_t len;
> - char *s, *p;
> -
> if (e->unknown_error)
> error_at (loc, e->unknown_error, arg);
> else
> - error_at (loc, "unrecognized argument in option %qs", opt);
> -
> - len = 0;
> - for (i = 0; e->values[i].arg != NULL; i++)
> - len += strlen (e->values[i].arg) + 1;
> -
> - auto_vec <const char *> candidates;
> - s = XALLOCAVEC (char, len);
> - p = s;
> - for (i = 0; e->values[i].arg != NULL; i++)
> {
> - if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
> - continue;
> - size_t arglen = strlen (e->values[i].arg);
> - memcpy (p, e->values[i].arg, arglen);
> - p[arglen] = ' ';
> - p += arglen + 1;
> - candidates.safe_push (e->values[i].arg);
> + size_t len = 0;
> + unsigned int i;
> + auto_vec <const char *> candidates;
> + for (i = 0; e->values[i].arg != NULL; i++)
> + {
> + if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
> + continue;
> + len += strlen (e->values[i].arg) + 1;
> + candidates.safe_push (e->values[i].arg);
> + }
> + unrecognized_argument_error (loc, opt, arg, candidates, len);
> }
> - p[-1] = 0;
> - const char *hint = find_closest_string (arg, &candidates);
> - if (hint)
> - inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
> - option->opt_text, s, hint);
> - else
> - inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
> -
> return true;
> }
>
Plus obviously the unrecognized_argument_error needs to be declared in some
header file.
That said, for x86 -march/-mtune uses this is problematic, as it uses either
%<-march=%> argument or target("march=") attribute wording there depending
on whether it is a command line option or target attribute. Though, it is
not good this way for translation anyway. Perhaps use XNEWVEC instead of
XALLOCAVEC for the all options string, and have the helper function just
return that + hint, inform by itself and then free the string?
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)
2016-09-05 19:44 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Manuel López-Ibáñez
2016-09-05 20:00 ` Jakub Jelinek
@ 2016-09-05 20:02 ` Manuel López-Ibáñez
1 sibling, 0 replies; 12+ messages in thread
From: Manuel López-Ibáñez @ 2016-09-05 20:02 UTC (permalink / raw)
To: Jakub Jelinek, Uros Bizjak, David Malcolm; +Cc: GCC Patches
On 05/09/16 20:42, Manuel López-Ibáñez wrote:
> On 05/09/16 18:25, Jakub Jelinek wrote:
>> Hi!
>>
>> While most of the i386.opt -m....= options have enum args and thus
>> cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy= (and also
>> -mrecip=) don't use that, with the CPU strings being maintained inside of a
>> function rather than in some *.def file that could be also sourced into the
>> *.opt or something (and similarly for the strategies).
>>
>> This patch adds inform calls that handle those similarly to what
>> cmdline_handle_error does for the options with enum values.
>> In addition, it adds %qs instead of %s in a couple of spaces, and
>> stops reporting incorrect attribute option("march=...") when it is
>> target("march=...") etc.
>
> Something like the following should avoid a lot of (future) duplication
> (untested):
My proposal had an obvious regression if (e->unknown_error). This one might be
better:
Index: opts-common.c
===================================================================
--- opts-common.c (revision 239995)
+++ opts-common.c (working copy)
@@ -1069,6 +1069,38 @@
decoded->errors = 0;
}
+static void
+candidates_to_string (char *s, const auto_vec <const char *> *candidates)
+{
+ int i;
+ const char *candidate;
+ char *p = s;
+ FOR_EACH_VEC_ELT (*candidates, i, candidate)
+ {
+ gcc_assert (candidate);
+ size_t arglen = strlen (candidate);
+ memcpy (p, candidate, arglen);
+ p[arglen] = ' ';
+ p += arglen + 1;
+ }
+ p[-1] = 0;
+}
+
+void
+unrecognized_argument_hint (location_t loc, const char *opt, const char *arg,
+ const auto_vec <const char*> &candidates,
+ size_t total_len)
+{
+ char *s = XALLOCAVEC (char, total_len);
+ candidates_to_string (s, &candidates);
+ const char *hint = find_closest_string (arg, &candidates);
+ if (hint)
+ inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
+ opt, s, hint);
+ else
+ inform (loc, "valid arguments to %qs are: %s", opt, s);
+}
+
/* Perform diagnostics for read_cmdline_option and control_warning_option
functions. Returns true if an error has been diagnosed.
LOC and LANG_MASK arguments like in read_cmdline_option.
@@ -1107,40 +1139,22 @@
if (errors & CL_ERR_ENUM_ARG)
{
const struct cl_enum *e = &cl_enums[option->var_enum];
- unsigned int i;
- size_t len;
- char *s, *p;
-
if (e->unknown_error)
error_at (loc, e->unknown_error, arg);
else
- error_at (loc, "unrecognized argument in option %qs", opt);
+ error_at (loc, "argument %qs to %qs not recognized", arg, opt);
- len = 0;
- for (i = 0; e->values[i].arg != NULL; i++)
- len += strlen (e->values[i].arg) + 1;
-
+ size_t len = 0;
+ unsigned int i;
auto_vec <const char *> candidates;
- s = XALLOCAVEC (char, len);
- p = s;
for (i = 0; e->values[i].arg != NULL; i++)
{
if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
- continue;
- size_t arglen = strlen (e->values[i].arg);
- memcpy (p, e->values[i].arg, arglen);
- p[arglen] = ' ';
- p += arglen + 1;
+ continue;
+ len += strlen (e->values[i].arg) + 1;
candidates.safe_push (e->values[i].arg);
}
- p[-1] = 0;
- const char *hint = find_closest_string (arg, &candidates);
- if (hint)
- inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
- option->opt_text, s, hint);
- else
- inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
-
+ unrecognized_argument_hint (loc, opt, arg, candidates, len);
return true;
}
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)
2016-09-05 19:42 ` Uros Bizjak
@ 2016-09-06 0:20 ` David Malcolm
2016-09-06 8:44 ` Jakub Jelinek
2016-09-06 20:41 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, i386 part) Jakub Jelinek
1 sibling, 1 reply; 12+ messages in thread
From: David Malcolm @ 2016-09-06 0:20 UTC (permalink / raw)
To: Uros Bizjak, Jakub Jelinek; +Cc: gcc-patches
On Mon, 2016-09-05 at 21:00 +0200, Uros Bizjak wrote:
> On Mon, Sep 5, 2016 at 7:25 PM, Jakub Jelinek <jakub@redhat.com>
> wrote:
> > Hi!
> >
> > While most of the i386.opt -m....= options have enum args and thus
> > cmdline_handle_error handles those, -march=/-mtune=/-m*-strategy=
> > (and also
> > -mrecip=) don't use that, with the CPU strings being maintained
> > inside of a
> > function rather than in some *.def file that could be also sourced
> > into the
> > *.opt or something (and similarly for the strategies).
> >
> > This patch adds inform calls that handle those similarly to what
> > cmdline_handle_error does for the options with enum values.
> > In addition, it adds %qs instead of %s in a couple of spaces, and
> > stops reporting incorrect attribute option("march=...") when it is
> > target("march=...") etc.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> > trunk?
> >
> > 2016-09-05 Jakub Jelinek <jakub@redhat.com>
> >
> > PR middle-end/77475
> > * config/i386/i386.c: Include spellcheck.h.
> > (ix86_parse_stringop_strategy_string): Simplify, use %qs
> > instead of %s
> > where desirable, use argument instead of arg in the
> > diagnostic
> > wording, add list of supported strategies and spellcheck
> > hint.
> > (ix86_option_override_internal): Emit target("m...")
> > instead of
> > option("m...") in the diagnostic. Use %qs instead of %s in
> > invalid
> > -march/-mtune option diagnostic. Add list of supported
> > arches/tunings
> > and spellcheck hint.
> >
> > * gcc.target/i386/pr65990.c: Adjust expected diagnostics.
>
> OK as far as x86 target is concerned, but please allow a day for
> David
> to eventually comment on the implementation.
The calls into spellcheck.h are minimal and look reasonable (I can't
really comment on the x86 aspects). So I have little to add beyond the
cleanups that Manu already observed.
One thing: shouldn't this have testcases that give test coverage for
emitting hints for -march and -mtune? Something like
gcc/testsuite/gcc.dg/spellcheck-options-11.c
(though obviously the new ones would be target-specific).
> Thanks,
> Uros.
>
> > --- gcc/config/i386/i386.c.jj 2016-09-05 12:41:03.000000000 +0200
> > +++ gcc/config/i386/i386.c 2016-09-05 16:44:45.184981211 +0200
> > @@ -76,6 +76,7 @@ along with GCC; see the file COPYING3.
> > #include "case-cfn-macros.h"
> > #include "regrename.h"
> > #include "dojump.h"
> > +#include "spellcheck.h"
> >
> > /* This file should be included last. */
> > #include "target-def.h"
> > @@ -4533,19 +4534,19 @@ ix86_parse_stringop_strategy_string (cha
> > next_range_str = strchr (curr_range_str, ',');
> > if (next_range_str)
> > *next_range_str++ = '\0';
> > + const char *opt
> > + = is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=";
> >
> > if (3 != sscanf (curr_range_str, "%20[^:]:%d:%10s",
> > alg_name, &maxs, align))
> > {
> > - error ("wrong arg %s to option %s", curr_range_str,
> > - is_memset ? "-mmemset_strategy=" : "
> > -mmemcpy_strategy=");
> > + error ("wrong argument %qs to option %qs",
> > curr_range_str, opt);
> > return;
> > }
> >
> > if (n > 0 && (maxs < (input_ranges[n - 1].max + 1) && maxs
> > != -1))
> > {
> > - error ("size ranges of option %s should be increasing",
> > - is_memset ? "-mmemset_strategy=" : "
> > -mmemcpy_strategy=");
> > + error ("size ranges of option %qs should be increasing",
> > opt);
> > return;
> > }
> >
> > @@ -4555,9 +4556,35 @@ ix86_parse_stringop_strategy_string (cha
> >
> > if (i == last_alg)
> > {
> > - error ("wrong stringop strategy name %s specified for
> > option %s",
> > - alg_name,
> > - is_memset ? "-mmemset_strategy=" : "
> > -mmemcpy_strategy=");
> > + error ("wrong stringop strategy name %qs specified for
> > option %s",
> > + alg_name, opt);
> > +
> > + size_t len = 0;
> > + for (i = 0; i < last_alg; i++)
> > + len += strlen (stringop_alg_names[i]) + 1;
> > +
> > + char *s, *p;
> > + auto_vec <const char *> candidates;
> > + s = XALLOCAVEC (char, len);
> > + p = s;
> > + for (i = 0; i < last_alg; i++)
> > + if ((stringop_alg) i != rep_prefix_8_byte ||
> > TARGET_64BIT)
> > + {
> > + size_t arglen = strlen (stringop_alg_names[i]);
> > + memcpy (p, stringop_alg_names[i], arglen);
> > + p[arglen] = ' ';
> > + p += arglen + 1;
> > + candidates.safe_push (stringop_alg_names[i]);
> > + }
> > + p[-1] = 0;
> > + const char *hint = find_closest_string (alg_name,
> > &candidates);
> > + if (hint)
> > + inform (input_location,
> > + "valid arguments to %qs are: %s; did you mean
> > %qs?",
> > + opt, s, hint);
> > + else
> > + inform (input_location, "valid arguments to %qs are:
> > %s",
> > + opt, s);
> > return;
> > }
> >
> > @@ -4565,10 +4592,8 @@ ix86_parse_stringop_strategy_string (cha
> > && !TARGET_64BIT)
> > {
> > /* rep; movq isn't available in 32-bit code. */
> > - error ("stringop strategy name %s specified for option %s
> > "
> > - "not supported for 32-bit code",
> > - alg_name,
> > - is_memset ? "-mmemset_strategy=" : "
> > -mmemcpy_strategy=");
> > + error ("stringop strategy name %qs specified for option
> > %qs "
> > + "not supported for 32-bit code", alg_name, opt);
> > return;
> > }
> >
> > @@ -4580,8 +4605,7 @@ ix86_parse_stringop_strategy_string (cha
> > input_ranges[n].noalign = true;
> > else
> > {
> > - error ("unknown alignment %s specified for option %s",
> > - align, is_memset ? "-mmemset_strategy=" : "
> > -mmemcpy_strategy=");
> > + error ("unknown alignment %qs specified for option %qs",
> > align, opt);
> > return;
> > }
> > n++;
> > @@ -5041,7 +5065,7 @@ ix86_option_override_internal (bool main
> > }
> > else
> > {
> > - prefix = "option(\"";
> > + prefix = "target(\"";
> > suffix = "\")";
> > sw = "attribute";
> > }
> > @@ -5480,8 +5504,41 @@ ix86_option_override_internal (bool main
> > error ("intel CPU can be used only for %stune=%s %s",
> > prefix, suffix, sw);
> > else if (i == pta_size)
> > - error ("bad value (%s) for %sarch=%s %s",
> > - opts->x_ix86_arch_string, prefix, suffix, sw);
> > + {
> > + error ("bad value (%qs) for %<%sarch=%s%> %s",
> > + opts->x_ix86_arch_string, prefix, suffix, sw);
> > +
> > + size_t len = 0;
> > + for (i = 0; i < pta_size; i++)
> > + len += strlen (processor_alias_table[i].name) + 1;
> > +
> > + char *s, *p;
> > + auto_vec <const char *> candidates;
> > + s = XALLOCAVEC (char, len);
> > + p = s;
> > + for (i = 0; i < pta_size; i++)
> > + if (strcmp (processor_alias_table[i].name, "generic")
> > + && strcmp (processor_alias_table[i].name, "intel")
> > + && (!TARGET_64BIT_P (opts->x_ix86_isa_flags)
> > + || (processor_alias_table[i].flags & PTA_64BIT)))
> > + {
> > + size_t arglen = strlen (processor_alias_table[i].name);
> > + memcpy (p, processor_alias_table[i].name, arglen);
> > + p[arglen] = ' ';
> > + p += arglen + 1;
> > + candidates.safe_push (processor_alias_table[i].name);
> > + }
> > + p[-1] = 0;
> > + const char *hint
> > + = find_closest_string (opts->x_ix86_arch_string,
> > &candidates);
> > + if (hint)
> > + inform (input_location,
> > + "valid arguments to %<%sarch=%s%> %s are: %s; "
> > + "did you mean %qs?", prefix, suffix, sw, s, hint);
> > + else
> > + inform (input_location, "valid arguments to %<%sarch=%s%>
> > %s are: %s",
> > + prefix, suffix, sw, s);
> > + }
> >
> > ix86_arch_mask = 1u << ix86_arch;
> > for (i = 0; i < X86_ARCH_LAST; ++i)
> > @@ -5523,8 +5580,40 @@ ix86_option_override_internal (bool main
> > }
> >
> > if (ix86_tune_specified && i == pta_size)
> > - error ("bad value (%s) for %stune=%s %s",
> > - opts->x_ix86_tune_string, prefix, suffix, sw);
> > + {
> > + error ("bad value (%qs) for %<%stune=%s%> %s",
> > + opts->x_ix86_tune_string, prefix, suffix, sw);
> > +
> > + size_t len = 0;
> > + for (i = 0; i < pta_size; i++)
> > + len += strlen (processor_alias_table[i].name) + 1;
> > +
> > + char *s, *p;
> > + auto_vec <const char *> candidates;
> > + s = XALLOCAVEC (char, len);
> > + p = s;
> > + for (i = 0; i < pta_size; i++)
> > + if (!TARGET_64BIT_P (opts->x_ix86_isa_flags)
> > + || (processor_alias_table[i].flags & PTA_64BIT))
> > + {
> > + size_t arglen = strlen (processor_alias_table[i].name);
> > + memcpy (p, processor_alias_table[i].name, arglen);
> > + p[arglen] = ' ';
> > + p += arglen + 1;
> > + candidates.safe_push (processor_alias_table[i].name);
> > + }
> > + p[-1] = 0;
> > + const char *hint
> > + = find_closest_string (opts->x_ix86_tune_string,
> > &candidates);
> > + if (hint)
> > + inform (input_location,
> > + "valid arguments to %<%stune=%s%> %s are: %s; "
> > + "did you mean %qs?", prefix, suffix, sw, s, hint);
> > + else
> > + inform (input_location, "valid arguments to %<%stune=%s%>
> > %s are: %s",
> > + prefix, suffix, sw, s);
> > +
> > + }
> >
> > set_ix86_tune_features (ix86_tune, opts->x_ix86_dump_tunes);
> >
> > --- gcc/testsuite/gcc.target/i386/pr65990.c.jj 2016-05-22
> > 12:20:14.000000000 +0200
> > +++ gcc/testsuite/gcc.target/i386/pr65990.c 2016-09-05
> > 19:00:48.000000000 +0200
> > @@ -1,7 +1,7 @@
> > /* { dg-do compile } */
> > /* { dg-options "-mtune=btver2 -mmemcpy-strategy=rep_8byte:
> > -1:noalign" }
> >
> > -/* { dg-error "stringop strategy name rep_8byte specified for
> > option -mmemcpy_strategy= not supported for 32-bit code" "" {
> > target ia32 } 0 } */
> > +/* { dg-error "stringop strategy name 'rep_8byte' specified for
> > option '-mmemcpy_strategy=' not supported for 32-bit code" "" {
> > target ia32 } 0 } */
> >
> > struct U9
> > {
> >
> > Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475)
2016-09-06 0:20 ` David Malcolm
@ 2016-09-06 8:44 ` Jakub Jelinek
0 siblings, 0 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-09-06 8:44 UTC (permalink / raw)
To: David Malcolm; +Cc: Uros Bizjak, gcc-patches
On Mon, Sep 05, 2016 at 07:54:15PM -0400, David Malcolm wrote:
> The calls into spellcheck.h are minimal and look reasonable (I can't
> really comment on the x86 aspects). So I have little to add beyond the
> cleanups that Manu already observed.
>
> One thing: shouldn't this have testcases that give test coverage for
> emitting hints for -march and -mtune? Something like
> gcc/testsuite/gcc.dg/spellcheck-options-11.c
> (though obviously the new ones would be target-specific).
I guess I could add one, but would prefer not to list all the CPUs, just
perhaps few first ones, because otherwise whenever a new CPU is added we'll
need to adjust the testcase.
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, generic part)
2016-09-05 20:00 ` Jakub Jelinek
@ 2016-09-06 20:40 ` Jakub Jelinek
2016-09-12 12:49 ` Bernd Schmidt
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2016-09-06 20:40 UTC (permalink / raw)
To: David Malcolm, Manuel López-Ibáñez
Cc: Uros Bizjak, GCC Patches
Hi!
On Mon, Sep 05, 2016 at 09:59:03PM +0200, Jakub Jelinek wrote:
> Plus obviously the unrecognized_argument_error needs to be declared in some
> header file.
>
> That said, for x86 -march/-mtune uses this is problematic, as it uses either
> %<-march=%> argument or target("march=") attribute wording there depending
> on whether it is a command line option or target attribute. Though, it is
> not good this way for translation anyway. Perhaps use XNEWVEC instead of
> XALLOCAVEC for the all options string, and have the helper function just
> return that + hint, inform by itself and then free the string?
Here is the generic part that does this (and it indeed simplifies quite a
lot the i386.c patch, while keeping the diagnostics in its hands for exact
wording etc.).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-09-06 Jakub Jelinek <jakub@redhat.com>
Manuel Lopez-Ibanez <manu@gcc.gnu.org>
PR middle-end/77475
* opts.h (candidates_list_and_hint): Declare.
* opts-common.c (candidates_list_and_hint): New function.
(cmdline_handle_error): Use it.
--- gcc/opts.h.jj 2016-06-30 19:38:45.000000000 +0200
+++ gcc/opts.h 2016-09-06 18:05:50.836734668 +0200
@@ -419,5 +420,8 @@ extern const struct sanitizer_opts_s
extern void add_misspelling_candidates (auto_vec<char *> *candidates,
const struct cl_option *option,
const char *base_option);
+extern const char *candidates_list_and_hint (const char *arg, char *&str,
+ const auto_vec <const char *> &
+ candidates);
#endif
--- gcc/opts-common.c.jj 2016-06-30 19:38:45.000000000 +0200
+++ gcc/opts-common.c 2016-09-06 19:36:22.339827679 +0200
@@ -1069,6 +1069,37 @@ generate_option_input_file (const char *
decoded->errors = 0;
}
+/* Helper function for listing valid choices and hint for misspelled
+ value. CANDIDATES is a vector containing all valid strings,
+ STR is set to a heap allocated string that contains all those
+ strings concatenated, separated by spaces, and the return value
+ is the closest string from those to ARG, or NULL if nothing is
+ close enough. */
+
+const char *
+candidates_list_and_hint (const char *arg, char *&str,
+ const auto_vec <const char *> &candidates)
+{
+ size_t len = 0;
+ int i;
+ const char *candidate;
+ char *p;
+
+ FOR_EACH_VEC_ELT (candidates, i, candidate)
+ len += strlen (candidate) + 1;
+
+ str = p = XNEWVEC (char, len);
+ FOR_EACH_VEC_ELT (candidates, i, candidate)
+ {
+ len = strlen (candidate);
+ memcpy (p, candidate, len);
+ p[len] = ' ';
+ p += len + 1;
+ }
+ p[-1] = '\0';
+ return find_closest_string (arg, &candidates);
+}
+
/* Perform diagnostics for read_cmdline_option and control_warning_option
functions. Returns true if an error has been diagnosed.
LOC and LANG_MASK arguments like in read_cmdline_option.
@@ -1108,38 +1139,27 @@ cmdline_handle_error (location_t loc, co
{
const struct cl_enum *e = &cl_enums[option->var_enum];
unsigned int i;
- size_t len;
- char *s, *p;
+ char *s;
if (e->unknown_error)
error_at (loc, e->unknown_error, arg);
else
error_at (loc, "unrecognized argument in option %qs", opt);
- len = 0;
- for (i = 0; e->values[i].arg != NULL; i++)
- len += strlen (e->values[i].arg) + 1;
-
auto_vec <const char *> candidates;
- s = XALLOCAVEC (char, len);
- p = s;
for (i = 0; e->values[i].arg != NULL; i++)
{
if (!enum_arg_ok_for_language (&e->values[i], lang_mask))
continue;
- size_t arglen = strlen (e->values[i].arg);
- memcpy (p, e->values[i].arg, arglen);
- p[arglen] = ' ';
- p += arglen + 1;
candidates.safe_push (e->values[i].arg);
}
- p[-1] = 0;
- const char *hint = find_closest_string (arg, &candidates);
+ const char *hint = candidates_list_and_hint (arg, s, candidates);
if (hint)
inform (loc, "valid arguments to %qs are: %s; did you mean %qs?",
option->opt_text, s, hint);
else
inform (loc, "valid arguments to %qs are: %s", option->opt_text, s);
+ XDELETEVEC (s);
return true;
}
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, i386 part)
2016-09-05 19:42 ` Uros Bizjak
2016-09-06 0:20 ` David Malcolm
@ 2016-09-06 20:41 ` Jakub Jelinek
2016-09-16 7:25 ` Rainer Orth
1 sibling, 1 reply; 12+ messages in thread
From: Jakub Jelinek @ 2016-09-06 20:41 UTC (permalink / raw)
To: Uros Bizjak, David Malcolm; +Cc: gcc-patches
Hi!
On Mon, Sep 05, 2016 at 09:00:30PM +0200, Uros Bizjak wrote:
> OK as far as x86 target is concerned, but please allow a day for David
> to eventually comment on the implementation.
And here is the updated i386 patch, on top of the generic patch I've just
posted. Compared to the last patch, this one now includes diagnostics,
uses the new opts-common.c helper and also stops using the perhaps fancy,
but absolutely l10n incompatible technique with prefix/suffix/sw variables
in various diagnostics - the words switch and attribute couldn't be
translated and would appear somewhere in a translated sequence.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-09-05 Jakub Jelinek <jakub@redhat.com>
PR middle-end/77475
* config/i386/i386.c (ix86_parse_stringop_strategy_string): Simplify,
use %qs instead of %s where desirable, use argument instead of arg in
the diagnostic wording, add list of supported strategies and
spellcheck hint.
(ix86_option_override_internal): Emit target("m...") instead of
option("m...") in the diagnostic. Use %qs instead of %s in invalid
-march/-mtune option diagnostic. Add list of supported arches/tunings
and spellcheck hint. Remove prefix, suffix and sw variables, use
main_args_p ? "..." : "..." in diagnostics to make translation
possible.
* gcc.target/i386/pr65990.c: Adjust expected diagnostics.
* gcc.dg/march-generic.c: Likewise.
* gcc.target/i386/spellcheck-options-1.c: New test.
* gcc.target/i386/spellcheck-options-2.c: New test.
* gcc.target/i386/spellcheck-options-3.c: New test.
* gcc.target/i386/spellcheck-options-4.c: New test.
--- gcc/config/i386/i386.c.jj 2016-09-06 16:55:35.524605779 +0200
+++ gcc/config/i386/i386.c 2016-09-06 19:45:03.126299652 +0200
@@ -4516,6 +4517,7 @@ ix86_parse_stringop_strategy_string (cha
const struct stringop_algs *default_algs;
stringop_size_range input_ranges[MAX_STRINGOP_ALGS];
char *curr_range_str, *next_range_str;
+ const char *opt = is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=";
int i = 0, n = 0;
if (is_memset)
@@ -4537,15 +4539,13 @@ ix86_parse_stringop_strategy_string (cha
if (3 != sscanf (curr_range_str, "%20[^:]:%d:%10s",
alg_name, &maxs, align))
{
- error ("wrong arg %s to option %s", curr_range_str,
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("wrong argument %qs to option %qs", curr_range_str, opt);
return;
}
if (n > 0 && (maxs < (input_ranges[n - 1].max + 1) && maxs != -1))
{
- error ("size ranges of option %s should be increasing",
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("size ranges of option %qs should be increasing", opt);
return;
}
@@ -4555,9 +4555,25 @@ ix86_parse_stringop_strategy_string (cha
if (i == last_alg)
{
- error ("wrong stringop strategy name %s specified for option %s",
- alg_name,
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("wrong strategy name %qs specified for option %qs",
+ alg_name, opt);
+
+ auto_vec <const char *> candidates;
+ for (i = 0; i < last_alg; i++)
+ if ((stringop_alg) i != rep_prefix_8_byte || TARGET_64BIT)
+ candidates.safe_push (stringop_alg_names[i]);
+
+ char *s;
+ const char *hint
+ = candidates_list_and_hint (alg_name, s, candidates);
+ if (hint)
+ inform (input_location,
+ "valid arguments to %qs are: %s; did you mean %qs?",
+ opt, s, hint);
+ else
+ inform (input_location, "valid arguments to %qs are: %s",
+ opt, s);
+ XDELETEVEC (s);
return;
}
@@ -4565,10 +4581,8 @@ ix86_parse_stringop_strategy_string (cha
&& !TARGET_64BIT)
{
/* rep; movq isn't available in 32-bit code. */
- error ("stringop strategy name %s specified for option %s "
- "not supported for 32-bit code",
- alg_name,
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("strategy name %qs specified for option %qs "
+ "not supported for 32-bit code", alg_name, opt);
return;
}
@@ -4580,8 +4594,7 @@ ix86_parse_stringop_strategy_string (cha
input_ranges[n].noalign = true;
else
{
- error ("unknown alignment %s specified for option %s",
- align, is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("unknown alignment %qs specified for option %qs", align, opt);
return;
}
n++;
@@ -4592,15 +4605,13 @@ ix86_parse_stringop_strategy_string (cha
if (input_ranges[n - 1].max != -1)
{
error ("the max value for the last size range should be -1"
- " for option %s",
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ " for option %qs", opt);
return;
}
if (n > MAX_STRINGOP_ALGS)
{
- error ("too many size ranges specified in option %s",
- is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
+ error ("too many size ranges specified in option %qs", opt);
return;
}
@@ -4731,9 +4742,6 @@ ix86_option_override_internal (bool main
int i;
unsigned int ix86_arch_mask;
const bool ix86_tune_specified = (opts->x_ix86_tune_string != NULL);
- const char *prefix;
- const char *suffix;
- const char *sw;
#define PTA_3DNOW (HOST_WIDE_INT_1 << 0)
#define PTA_3DNOW_A (HOST_WIDE_INT_1 << 1)
@@ -5031,21 +5039,6 @@ ix86_option_override_internal (bool main
int const pta_size = ARRAY_SIZE (processor_alias_table);
- /* Set up prefix/suffix so the error messages refer to either the command
- line argument, or the attribute(target). */
- if (main_args_p)
- {
- prefix = "-m";
- suffix = "";
- sw = "switch";
- }
- else
- {
- prefix = "option(\"";
- suffix = "\")";
- sw = "attribute";
- }
-
/* Turn off both OPTION_MASK_ABI_64 and OPTION_MASK_ABI_X32 if
TARGET_64BIT_DEFAULT is true and TARGET_64BIT is false. */
if (TARGET_64BIT_DEFAULT && !TARGET_64BIT_P (opts->x_ix86_isa_flags))
@@ -5118,9 +5111,13 @@ ix86_option_override_internal (bool main
opts->x_ix86_tune_string = "generic";
}
else if (!strcmp (opts->x_ix86_tune_string, "x86-64"))
- warning (OPT_Wdeprecated, "%stune=x86-64%s is deprecated; use "
- "%stune=k8%s or %stune=generic%s instead as appropriate",
- prefix, suffix, prefix, suffix, prefix, suffix);
+ warning (OPT_Wdeprecated,
+ main_args_p
+ ? "%<-mtune=x86-64%> is deprecated; use %<-mtune=k8%> "
+ "or %<-mtune=generic%> instead as appropriate"
+ : "%<target(\"tune=x86-64\")%> is deprecated; use "
+ "%<target(\"tune=k8\")%> or %<target(\"tune=generic\")%> "
+ "instead as appropriate");
}
else
{
@@ -5474,14 +5471,48 @@ ix86_option_override_internal (bool main
error ("Intel MPX does not support x32");
if (!strcmp (opts->x_ix86_arch_string, "generic"))
- error ("generic CPU can be used only for %stune=%s %s",
- prefix, suffix, sw);
+ error (main_args_p
+ ? "%<generic%> CPU can be used only for %<-mtune=%> switch"
+ : "%<generic%> CPU can be used only for "
+ "%<target(\"tune=\")%> attribute");
else if (!strcmp (opts->x_ix86_arch_string, "intel"))
- error ("intel CPU can be used only for %stune=%s %s",
- prefix, suffix, sw);
+ error (main_args_p
+ ? "%<intel%> CPU can be used only for %<-mtune=%> switch"
+ : "%<intel%> CPU can be used only for "
+ "%<target(\"tune=\")%> attribute");
else if (i == pta_size)
- error ("bad value (%s) for %sarch=%s %s",
- opts->x_ix86_arch_string, prefix, suffix, sw);
+ {
+ error (main_args_p
+ ? "bad value (%qs) for %<-march=%> switch"
+ : "bad value (%qs) for %<target(\"arch=\")%> attribute",
+ opts->x_ix86_arch_string);
+
+ auto_vec <const char *> candidates;
+ for (i = 0; i < pta_size; i++)
+ if (strcmp (processor_alias_table[i].name, "generic")
+ && strcmp (processor_alias_table[i].name, "intel")
+ && (!TARGET_64BIT_P (opts->x_ix86_isa_flags)
+ || (processor_alias_table[i].flags & PTA_64BIT)))
+ candidates.safe_push (processor_alias_table[i].name);
+
+ char *s;
+ const char *hint
+ = candidates_list_and_hint (opts->x_ix86_arch_string, s, candidates);
+ if (hint)
+ inform (input_location,
+ main_args_p
+ ? "valid arguments to %<-march=%> switch are: "
+ "%s; did you mean %qs?"
+ : "valid arguments to %<target(\"arch=\")%> attribute are: "
+ "%s; did you mean %qs?", s, hint);
+ else
+ inform (input_location,
+ main_args_p
+ ? "valid arguments to %<-march=%> switch are: %s"
+ : "valid arguments to %<target(\"arch=\")%> attribute are: %s",
+ s);
+ XDELETEVEC (s);
+ }
ix86_arch_mask = 1u << ix86_arch;
for (i = 0; i < X86_ARCH_LAST; ++i)
@@ -5523,8 +5554,36 @@ ix86_option_override_internal (bool main
}
if (ix86_tune_specified && i == pta_size)
- error ("bad value (%s) for %stune=%s %s",
- opts->x_ix86_tune_string, prefix, suffix, sw);
+ {
+ error (main_args_p
+ ? "bad value (%qs) for %<-mtune=%> switch"
+ : "bad value (%qs) for %<target(\"tune=\")%> attribute",
+ opts->x_ix86_tune_string);
+
+ auto_vec <const char *> candidates;
+ for (i = 0; i < pta_size; i++)
+ if (!TARGET_64BIT_P (opts->x_ix86_isa_flags)
+ || (processor_alias_table[i].flags & PTA_64BIT))
+ candidates.safe_push (processor_alias_table[i].name);
+
+ char *s;
+ const char *hint
+ = candidates_list_and_hint (opts->x_ix86_tune_string, s, candidates);
+ if (hint)
+ inform (input_location,
+ main_args_p
+ ? "valid arguments to %<-mtune=%> switch are: "
+ "%s; did you mean %qs?"
+ : "valid arguments to %<target(\"tune=\")%> attribute are: "
+ "%s; did you mean %qs?", s, hint);
+ else
+ inform (input_location,
+ main_args_p
+ ? "valid arguments to %<-mtune=%> switch are: %s"
+ : "valid arguments to %<target(\"tune=\")%> attribute are: %s",
+ s);
+ XDELETEVEC (s);
+ }
set_ix86_tune_features (ix86_tune, opts->x_ix86_dump_tunes);
@@ -5623,7 +5682,9 @@ ix86_option_override_internal (bool main
& ~opts->x_ix86_isa_flags_explicit);
if (TARGET_RTD_P (opts->x_target_flags))
- warning (0, "%srtd%s is ignored in 64bit mode", prefix, suffix);
+ warning (0,
+ main_args_p ? "%<-mrtd%> is ignored in 64bit mode"
+ : "%<target(\"rtd\")%> is ignored in 64bit mode");
}
else
{
@@ -5744,7 +5805,9 @@ ix86_option_override_internal (bool main
/* Accept -msseregparm only if at least SSE support is enabled. */
if (TARGET_SSEREGPARM_P (opts->x_target_flags)
&& ! TARGET_SSE_P (opts->x_ix86_isa_flags))
- error ("%ssseregparm%s used without SSE enabled", prefix, suffix);
+ error (main_args_p
+ ? "%<-msseregparm%> used without SSE enabled"
+ : "%<target(\"sseregparm\")%> used without SSE enabled");
if (opts_set->x_ix86_fpmath)
{
@@ -5809,8 +5872,12 @@ ix86_option_override_internal (bool main
&& !(opts->x_target_flags & MASK_ACCUMULATE_OUTGOING_ARGS))
{
if (opts_set->x_target_flags & MASK_ACCUMULATE_OUTGOING_ARGS)
- warning (0, "stack probing requires %saccumulate-outgoing-args%s "
- "for correctness", prefix, suffix);
+ warning (0,
+ main_args_p
+ ? "stack probing requires %<-maccumulate-outgoing-args%> "
+ "for correctness"
+ : "stack probing requires "
+ "%<target(\"accumulate-outgoing-args\")%> for correctness");
opts->x_target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;
}
@@ -5820,8 +5887,11 @@ ix86_option_override_internal (bool main
&& !(opts->x_target_flags & MASK_ACCUMULATE_OUTGOING_ARGS))
{
if (opts_set->x_target_flags & MASK_ACCUMULATE_OUTGOING_ARGS)
- warning (0, "fixed ebp register requires %saccumulate-outgoing-args%s",
- prefix, suffix);
+ warning (0,
+ main_args_p
+ ? "fixed ebp register requires %<-maccumulate-outgoing-args%>"
+ : "fixed ebp register requires "
+ "%<target(\"accumulate-outgoing-args\")%>");
opts->x_target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;
}
--- gcc/testsuite/gcc.target/i386/pr65990.c.jj 2016-09-05 19:27:03.016674736 +0200
+++ gcc/testsuite/gcc.target/i386/pr65990.c 2016-09-06 17:47:11.812776172 +0200
@@ -1,7 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-mtune=btver2 -mmemcpy-strategy=rep_8byte:-1:noalign" }
-/* { dg-error "stringop strategy name rep_8byte specified for option -mmemcpy_strategy= not supported for 32-bit code" "" { target ia32 } 0 } */
+/* { dg-error "stringop strategy name 'rep_8byte' specified for option '-mmemcpy_strategy=' not supported for 32-bit code" "" { target ia32 } 0 } */
struct U9
{
--- gcc/testsuite/gcc.dg/march-generic.c.jj 2014-11-11 00:06:08.000000000 +0100
+++ gcc/testsuite/gcc.dg/march-generic.c 2016-09-06 22:19:32.278111025 +0200
@@ -1,6 +1,6 @@
/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
/* { dg-options "-march=generic" } */
-/* { dg-error "generic CPU can be used only for -mtune" "" { target *-*-* } 0 } */
+/* { dg-error "'generic' CPU can be used only for '-mtune=' switch" "" { target *-*-* } 0 } */
/* { dg-bogus "march" "" { target *-*-* } 0 } */
int i;
--- gcc/testsuite/gcc.target/i386/spellcheck-options-1.c.jj 2016-09-06 19:33:10.134236964 +0200
+++ gcc/testsuite/gcc.target/i386/spellcheck-options-1.c 2016-09-06 19:39:21.623580368 +0200
@@ -0,0 +1,7 @@
+/* Verify that we provide a hint if the user misspells an option argument
+ (PR middle-end/77475). */
+
+/* { dg-do compile } */
+/* { dg-options "-march=hasvel" } */
+/* { dg-error "bad value .'hasvel'. for '-march=' switch" "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments to '-march=' switch are: \[^\n\r]*; did you mean 'haswell'?" "" { target *-*-* } 0 } */
--- gcc/testsuite/gcc.target/i386/spellcheck-options-2.c.jj 2016-09-06 19:39:39.433357123 +0200
+++ gcc/testsuite/gcc.target/i386/spellcheck-options-2.c 2016-09-06 19:40:02.367069650 +0200
@@ -0,0 +1,7 @@
+/* Verify that we provide a hint if the user misspells an option argument
+ (PR middle-end/77475). */
+
+/* { dg-do compile } */
+/* { dg-options "-mtune=hasvel" } */
+/* { dg-error "bad value .'hasvel'. for '-mtune=' switch" "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments to '-mtune=' switch are: \[^\n\r]*; did you mean 'haswell'?" "" { target *-*-* } 0 } */
--- gcc/testsuite/gcc.target/i386/spellcheck-options-3.c.jj 2016-09-06 19:40:13.177934137 +0200
+++ gcc/testsuite/gcc.target/i386/spellcheck-options-3.c 2016-09-06 19:46:25.200270859 +0200
@@ -0,0 +1,7 @@
+/* Verify that we provide a hint if the user misspells an option argument
+ (PR middle-end/77475). */
+
+/* { dg-do compile } */
+/* { dg-options "-mmemcpy-strategy=unroled_looop:8:align" } */
+/* { dg-error "wrong strategy name 'unroled_looop' specified for option '-mmemcpy_strategy='" "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments to '-mmemcpy_strategy=' are: \[^\n\r]*; did you mean 'unrolled_loop'?" "" { target *-*-* } 0 } */
--- gcc/testsuite/gcc.target/i386/spellcheck-options-4.c.jj 2016-09-06 19:46:37.500116681 +0200
+++ gcc/testsuite/gcc.target/i386/spellcheck-options-4.c 2016-09-06 19:48:04.486026318 +0200
@@ -0,0 +1,7 @@
+/* Verify that we provide a hint if the user misspells an option argument
+ (PR middle-end/77475). */
+
+/* { dg-do compile } */
+
+__attribute__((target ("arch=hasvel"))) void foo (void) {} /* { dg-error "bad value .'hasvel'. for 'target..arch=..' attribute" } */
+/* { dg-message "valid arguments to 'target..arch=..' attribute are: \[^\n\r]*; did you mean 'haswell'?" "" { target *-*-* } 6 } */
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, generic part)
2016-09-06 20:40 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, generic part) Jakub Jelinek
@ 2016-09-12 12:49 ` Bernd Schmidt
0 siblings, 0 replies; 12+ messages in thread
From: Bernd Schmidt @ 2016-09-12 12:49 UTC (permalink / raw)
To: Jakub Jelinek, David Malcolm, Manuel López-Ibáñez
Cc: Uros Bizjak, GCC Patches
On 09/06/2016 10:36 PM, Jakub Jelinek wrote:
> +/* Helper function for listing valid choices and hint for misspelled
> + value. CANDIDATES is a vector containing all valid strings,
> + STR is set to a heap allocated string that contains all those
> + strings concatenated, separated by spaces, and the return value
> + is the closest string from those to ARG, or NULL if nothing is
> + close enough. */
Maybe mention the return value should be freed with XDELETEVEC by the
caller. Ok with that change.
Bernd
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, i386 part)
2016-09-06 20:41 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, i386 part) Jakub Jelinek
@ 2016-09-16 7:25 ` Rainer Orth
2016-09-16 8:29 ` Jakub Jelinek
0 siblings, 1 reply; 12+ messages in thread
From: Rainer Orth @ 2016-09-16 7:25 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Uros Bizjak, David Malcolm, gcc-patches
Hi Jakub,
> 2016-09-05 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/77475
> * config/i386/i386.c (ix86_parse_stringop_strategy_string): Simplify,
> use %qs instead of %s where desirable, use argument instead of arg in
> the diagnostic wording, add list of supported strategies and
> spellcheck hint.
> (ix86_option_override_internal): Emit target("m...") instead of
> option("m...") in the diagnostic. Use %qs instead of %s in invalid
> -march/-mtune option diagnostic. Add list of supported arches/tunings
> and spellcheck hint. Remove prefix, suffix and sw variables, use
> main_args_p ? "..." : "..." in diagnostics to make translation
> possible.
>
> * gcc.target/i386/pr65990.c: Adjust expected diagnostics.
this test now fails on 32-bit Solaris/x86:
FAIL: gcc.target/i386/pr65990.c (test for errors, line )
FAIL: gcc.target/i386/pr65990.c (test for excess errors)
Excess errors:
cc1: error: strategy name 'rep_8byte' specified for option '-mmemcpy_strategy=' not supported for 32-bit code
> --- gcc/config/i386/i386.c.jj 2016-09-06 16:55:35.524605779 +0200
> +++ gcc/config/i386/i386.c 2016-09-06 19:45:03.126299652 +0200
[...]
> @@ -4555,9 +4555,25 @@ ix86_parse_stringop_strategy_string (cha
>
> if (i == last_alg)
> {
> - error ("wrong stringop strategy name %s specified for option %s",
> - alg_name,
> - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> + error ("wrong strategy name %qs specified for option %qs",
> + alg_name, opt);
The message lost the "stringop" ...
> --- gcc/testsuite/gcc.target/i386/pr65990.c.jj 2016-09-05 19:27:03.016674736 +0200
> +++ gcc/testsuite/gcc.target/i386/pr65990.c 2016-09-06 17:47:11.812776172 +0200
> @@ -1,7 +1,7 @@
> /* { dg-do compile } */
> /* { dg-options "-mtune=btver2 -mmemcpy-strategy=rep_8byte:-1:noalign" }
>
> -/* { dg-error "stringop strategy name rep_8byte specified for option -mmemcpy_strategy= not supported for 32-bit code" "" { target ia32 } 0 } */
> +/* { dg-error "stringop strategy name 'rep_8byte' specified for option '-mmemcpy_strategy=' not supported for 32-bit code" "" { target ia32 } 0 } */
... while the testcase still requires it. Can't tell for certain which
is intended.
Rainer
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, i386 part)
2016-09-16 7:25 ` Rainer Orth
@ 2016-09-16 8:29 ` Jakub Jelinek
0 siblings, 0 replies; 12+ messages in thread
From: Jakub Jelinek @ 2016-09-16 8:29 UTC (permalink / raw)
To: Rainer Orth; +Cc: Uros Bizjak, David Malcolm, gcc-patches
On Fri, Sep 16, 2016 at 09:12:42AM +0200, Rainer Orth wrote:
> Excess errors:
> cc1: error: strategy name 'rep_8byte' specified for option '-mmemcpy_strategy=' not supported for 32-bit code
>
> > --- gcc/config/i386/i386.c.jj 2016-09-06 16:55:35.524605779 +0200
> > +++ gcc/config/i386/i386.c 2016-09-06 19:45:03.126299652 +0200
> [...]
> > @@ -4555,9 +4555,25 @@ ix86_parse_stringop_strategy_string (cha
> >
> > if (i == last_alg)
> > {
> > - error ("wrong stringop strategy name %s specified for option %s",
> > - alg_name,
> > - is_memset ? "-mmemset_strategy=" : "-mmemcpy_strategy=");
> > + error ("wrong strategy name %qs specified for option %qs",
> > + alg_name, opt);
>
> The message lost the "stringop" ...
That was intentional, I think it is fine to use the term stringop internally in gcc,
but it isn't something we should present to users.
> > --- gcc/testsuite/gcc.target/i386/pr65990.c.jj 2016-09-05 19:27:03.016674736 +0200
> > +++ gcc/testsuite/gcc.target/i386/pr65990.c 2016-09-06 17:47:11.812776172 +0200
> > @@ -1,7 +1,7 @@
> > /* { dg-do compile } */
> > /* { dg-options "-mtune=btver2 -mmemcpy-strategy=rep_8byte:-1:noalign" }
> >
> > -/* { dg-error "stringop strategy name rep_8byte specified for option -mmemcpy_strategy= not supported for 32-bit code" "" { target ia32 } 0 } */
> > +/* { dg-error "stringop strategy name 'rep_8byte' specified for option '-mmemcpy_strategy=' not supported for 32-bit code" "" { target ia32 } 0 } */
>
> ... while the testcase still requires it. Can't tell for certain which
> is intended.
Dunno how I've managed to mess this up, and certainly I saw this FAIL last
night, meant to look at it in the morning.
Here is what I've committed.
2016-09-16 Jakub Jelinek <jakub@redhat.com>
PR middle-end/77475
* gcc.target/i386/pr65990.c: Adjust dg-error regexp.
--- gcc/testsuite/gcc.target/i386/pr65990.c.jj 2016-09-15 14:25:51.000000000 +0200
+++ gcc/testsuite/gcc.target/i386/pr65990.c 2016-09-16 10:10:15.745196917 +0200
@@ -1,7 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-mtune=btver2 -mmemcpy-strategy=rep_8byte:-1:noalign" }
-/* { dg-error "stringop strategy name 'rep_8byte' specified for option '-mmemcpy_strategy=' not supported for 32-bit code" "" { target ia32 } 0 } */
+/* { dg-error "strategy name 'rep_8byte' specified for option '-mmemcpy_strategy=' not supported for 32-bit code" "" { target ia32 } 0 } */
struct U9
{
Jakub
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-09-16 8:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-05 18:03 [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Jakub Jelinek
2016-09-05 19:42 ` Uros Bizjak
2016-09-06 0:20 ` David Malcolm
2016-09-06 8:44 ` Jakub Jelinek
2016-09-06 20:41 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, i386 part) Jakub Jelinek
2016-09-16 7:25 ` Rainer Orth
2016-09-16 8:29 ` Jakub Jelinek
2016-09-05 19:44 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Manuel López-Ibáñez
2016-09-05 20:00 ` Jakub Jelinek
2016-09-06 20:40 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475, take 2, generic part) Jakub Jelinek
2016-09-12 12:49 ` Bernd Schmidt
2016-09-05 20:02 ` [PATCH, i386] Spellcheck hints for the i386 backend option handling (PR middle-end/77475) Manuel López-Ibáñez
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).