diff mbox

[AArch64] Add spellchecking hints for -march,-mcpu,-mtune and their attributes

Message ID 57F7B6C3.6070708@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov Oct. 7, 2016, 2:52 p.m. UTC
Hi all,

This patch uses the spellcheck API from David and Jakub [1] to implement hints for the
march, mcpu and mtune options to suggest appropriate architectures and CPU names
for users.  It also adds such hints for the equivalent arch, cpu, tune attributes.
Architecture extensions like 'crc', 'crypto' are not handled in this patch as they
can be combined and modified with '+no' so it would be quite tricky. But if a user
misspells those we just give a proper "unknown modifier" error. The hints appear
only when the actual CPU or architecture name is misspelled.

Bootstrapped and tested on aarch64-none-linux-gnu.

Ok for trunk?

Thanks,
Kyrill

[1] https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00339.html

2016-10-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/aarch64/aarch64.c (aarch64_print_hint_for_core_or_arch):
     New function.
     (aarch64_print_hint_for_core): Likewise.
     (aarch64_print_hint_for_arch): Likewise.
     (aarch64_validate_march): Use it.  Fix indentation in type signature.
     (aarch64_validate_mcpu): Use aarch64_print_hint_for_core_or_arch.
     (aarch64_validate_mtune): Likewise.
     (aarch64_handle_attr_arch): Likewise.
     (aarch64_handle_attr_cpu): Likewise.
     (aarch64_handle_attr_tune): Likewise.

2016-10-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.target/aarch64/spellcheck_1.c: New test.
     * gcc.target/aarch64/spellcheck_2.c: Likewise.
     * gcc.target/aarch64/spellcheck_3.c: Likewise.
     * gcc.target/aarch64/spellcheck_4.c: Likewise.
     * gcc.target/aarch64/spellcheck_5.c: Likewise.
     * gcc.target/aarch64/spellcheck_6.c: Likewise.

Comments

Andrew Pinski Oct. 7, 2016, 8:34 p.m. UTC | #1
On Fri, Oct 7, 2016 at 7:52 AM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi all,
>
> This patch uses the spellcheck API from David and Jakub [1] to implement
> hints for the
> march, mcpu and mtune options to suggest appropriate architectures and CPU
> names
> for users.  It also adds such hints for the equivalent arch, cpu, tune
> attributes.
> Architecture extensions like 'crc', 'crypto' are not handled in this patch
> as they
> can be combined and modified with '+no' so it would be quite tricky. But if
> a user
> misspells those we just give a proper "unknown modifier" error. The hints
> appear
> only when the actual CPU or architecture name is misspelled.
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
>
> Ok for trunk?


I like this idea.  Though I need to check how it will interact with my
patch set which I am doing right now to support some more of Cavium's
SOCs.

Thanks,
Andrew Pinski

>
> Thanks,
> Kyrill
>
> [1] https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00339.html
>
> 2016-10-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * config/aarch64/aarch64.c (aarch64_print_hint_for_core_or_arch):
>     New function.
>     (aarch64_print_hint_for_core): Likewise.
>     (aarch64_print_hint_for_arch): Likewise.
>     (aarch64_validate_march): Use it.  Fix indentation in type signature.
>     (aarch64_validate_mcpu): Use aarch64_print_hint_for_core_or_arch.
>     (aarch64_validate_mtune): Likewise.
>     (aarch64_handle_attr_arch): Likewise.
>     (aarch64_handle_attr_cpu): Likewise.
>     (aarch64_handle_attr_tune): Likewise.
>
> 2016-10-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * gcc.target/aarch64/spellcheck_1.c: New test.
>     * gcc.target/aarch64/spellcheck_2.c: Likewise.
>     * gcc.target/aarch64/spellcheck_3.c: Likewise.
>     * gcc.target/aarch64/spellcheck_4.c: Likewise.
>     * gcc.target/aarch64/spellcheck_5.c: Likewise.
>     * gcc.target/aarch64/spellcheck_6.c: Likewise.
James Greenhalgh Oct. 11, 2016, 10:40 a.m. UTC | #2
On Fri, Oct 07, 2016 at 01:34:37PM -0700, Andrew Pinski wrote:
> On Fri, Oct 7, 2016 at 7:52 AM, Kyrill Tkachov
> <kyrylo.tkachov@foss.arm.com> wrote:
> > Hi all,
> >
> > This patch uses the spellcheck API from David and Jakub [1] to implement
> > hints for the
> > march, mcpu and mtune options to suggest appropriate architectures and CPU
> > names
> > for users.  It also adds such hints for the equivalent arch, cpu, tune
> > attributes.
> > Architecture extensions like 'crc', 'crypto' are not handled in this patch
> > as they
> > can be combined and modified with '+no' so it would be quite tricky. But if
> > a user
> > misspells those we just give a proper "unknown modifier" error. The hints
> > appear
> > only when the actual CPU or architecture name is misspelled.
> >
> > Bootstrapped and tested on aarch64-none-linux-gnu.
> >
> > Ok for trunk?
> 
> 
> I like this idea.  Though I need to check how it will interact with my
> patch set which I am doing right now to support some more of Cavium's
> SOCs.

I also like it. OK from me, but give Andrew another day to come back with
any objections.

Thanks,
James

> 
> Thanks,
> Andrew Pinski
> 
> >
> > Thanks,
> > Kyrill
> >
> > [1] https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00339.html
> >
> > 2016-10-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >
> >     * config/aarch64/aarch64.c (aarch64_print_hint_for_core_or_arch):
> >     New function.
> >     (aarch64_print_hint_for_core): Likewise.
> >     (aarch64_print_hint_for_arch): Likewise.
> >     (aarch64_validate_march): Use it.  Fix indentation in type signature.
> >     (aarch64_validate_mcpu): Use aarch64_print_hint_for_core_or_arch.
> >     (aarch64_validate_mtune): Likewise.
> >     (aarch64_handle_attr_arch): Likewise.
> >     (aarch64_handle_attr_cpu): Likewise.
> >     (aarch64_handle_attr_tune): Likewise.
> >
> > 2016-10-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >
> >     * gcc.target/aarch64/spellcheck_1.c: New test.
> >     * gcc.target/aarch64/spellcheck_2.c: Likewise.
> >     * gcc.target/aarch64/spellcheck_3.c: Likewise.
> >     * gcc.target/aarch64/spellcheck_4.c: Likewise.
> >     * gcc.target/aarch64/spellcheck_5.c: Likewise.
> >     * gcc.target/aarch64/spellcheck_6.c: Likewise.
>
Kyrill Tkachov Oct. 14, 2016, 8:34 a.m. UTC | #3
On 11/10/16 11:40, James Greenhalgh wrote:
> On Fri, Oct 07, 2016 at 01:34:37PM -0700, Andrew Pinski wrote:
>> On Fri, Oct 7, 2016 at 7:52 AM, Kyrill Tkachov
>> <kyrylo.tkachov@foss.arm.com> wrote:
>>> Hi all,
>>>
>>> This patch uses the spellcheck API from David and Jakub [1] to implement
>>> hints for the
>>> march, mcpu and mtune options to suggest appropriate architectures and CPU
>>> names
>>> for users.  It also adds such hints for the equivalent arch, cpu, tune
>>> attributes.
>>> Architecture extensions like 'crc', 'crypto' are not handled in this patch
>>> as they
>>> can be combined and modified with '+no' so it would be quite tricky. But if
>>> a user
>>> misspells those we just give a proper "unknown modifier" error. The hints
>>> appear
>>> only when the actual CPU or architecture name is misspelled.
>>>
>>> Bootstrapped and tested on aarch64-none-linux-gnu.
>>>
>>> Ok for trunk?
>>
>> I like this idea.  Though I need to check how it will interact with my
>> patch set which I am doing right now to support some more of Cavium's
>> SOCs.
> I also like it. OK from me, but give Andrew another day to come back with
> any objections.

Thanks, I'll commit this today.
Kyrill

> Thanks,
> James
>
>> Thanks,
>> Andrew Pinski
>>
>>> Thanks,
>>> Kyrill
>>>
>>> [1] https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00339.html
>>>
>>> 2016-10-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>      * config/aarch64/aarch64.c (aarch64_print_hint_for_core_or_arch):
>>>      New function.
>>>      (aarch64_print_hint_for_core): Likewise.
>>>      (aarch64_print_hint_for_arch): Likewise.
>>>      (aarch64_validate_march): Use it.  Fix indentation in type signature.
>>>      (aarch64_validate_mcpu): Use aarch64_print_hint_for_core_or_arch.
>>>      (aarch64_validate_mtune): Likewise.
>>>      (aarch64_handle_attr_arch): Likewise.
>>>      (aarch64_handle_attr_cpu): Likewise.
>>>      (aarch64_handle_attr_tune): Likewise.
>>>
>>> 2016-10-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>      * gcc.target/aarch64/spellcheck_1.c: New test.
>>>      * gcc.target/aarch64/spellcheck_2.c: Likewise.
>>>      * gcc.target/aarch64/spellcheck_3.c: Likewise.
>>>      * gcc.target/aarch64/spellcheck_4.c: Likewise.
>>>      * gcc.target/aarch64/spellcheck_5.c: Likewise.
>>>      * gcc.target/aarch64/spellcheck_6.c: Likewise.
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index eeaaa724edb1c70c6aaec32c51316336ef6ef01a..2bb7b8f021f1b67b005bec619f38ecc71ed9cd07 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -8276,6 +8276,44 @@  aarch64_override_options_internal (struct gcc_options *opts)
   aarch64_override_options_after_change_1 (opts);
 }
 
+/* Print a hint with a suggestion for a core or architecture name that
+   most closely resembles what the user passed in STR.  ARCH is true if
+   the user is asking for an architecture name.  ARCH is false if the user
+   is asking for a core name.  */
+
+static void
+aarch64_print_hint_for_core_or_arch (const char *str, bool arch)
+{
+  auto_vec<const char *> candidates;
+  const struct processor *entry = arch ? all_architectures : all_cores;
+  for (; entry->name != NULL; entry++)
+    candidates.safe_push (entry->name);
+  char *s;
+  const char *hint = candidates_list_and_hint (str, s, candidates);
+  if (hint)
+    inform (input_location, "valid arguments are: %s;"
+			     " did you mean %qs?", s, hint);
+  XDELETEVEC (s);
+}
+
+/* Print a hint with a suggestion for a core name that most closely resembles
+   what the user passed in STR.  */
+
+inline static void
+aarch64_print_hint_for_core (const char *str)
+{
+  aarch64_print_hint_for_core_or_arch (str, false);
+}
+
+/* Print a hint with a suggestion for an architecture name that most closely
+   resembles what the user passed in STR.  */
+
+inline static void
+aarch64_print_hint_for_arch (const char *str)
+{
+  aarch64_print_hint_for_core_or_arch (str, true);
+}
+
 /* Validate a command-line -mcpu option.  Parse the cpu and extensions (if any)
    specified in STR and throw errors if appropriate.  Put the results if
    they are valid in RES and ISA_FLAGS.  Return whether the option is
@@ -8298,6 +8336,7 @@  aarch64_validate_mcpu (const char *str, const struct processor **res,
 	break;
       case AARCH64_PARSE_INVALID_ARG:
 	error ("unknown value %qs for -mcpu", str);
+	aarch64_print_hint_for_core (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
 	error ("invalid feature modifier in -mcpu=%qs", str);
@@ -8316,7 +8355,7 @@  aarch64_validate_mcpu (const char *str, const struct processor **res,
 
 static bool
 aarch64_validate_march (const char *str, const struct processor **res,
-		       unsigned long *isa_flags)
+			 unsigned long *isa_flags)
 {
   enum aarch64_parse_opt_result parse_res
     = aarch64_parse_arch (str, res, isa_flags);
@@ -8331,6 +8370,7 @@  aarch64_validate_march (const char *str, const struct processor **res,
 	break;
       case AARCH64_PARSE_INVALID_ARG:
 	error ("unknown value %qs for -march", str);
+	aarch64_print_hint_for_arch (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
 	error ("invalid feature modifier in -march=%qs", str);
@@ -8363,6 +8403,7 @@  aarch64_validate_mtune (const char *str, const struct processor **res)
 	break;
       case AARCH64_PARSE_INVALID_ARG:
 	error ("unknown value %qs for -mtune", str);
+	aarch64_print_hint_for_core (str);
 	break;
       default:
 	gcc_unreachable ();
@@ -8736,6 +8777,7 @@  aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
 	break;
       case AARCH64_PARSE_INVALID_ARG:
 	error ("unknown value %qs for 'arch' target %s", str, pragma_or_attr);
+	aarch64_print_hint_for_arch (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
 	error ("invalid feature modifier %qs for 'arch' target %s",
@@ -8776,6 +8818,7 @@  aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
 	break;
       case AARCH64_PARSE_INVALID_ARG:
 	error ("unknown value %qs for 'cpu' target %s", str, pragma_or_attr);
+	aarch64_print_hint_for_core (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
 	error ("invalid feature modifier %qs for 'cpu' target %s",
@@ -8810,6 +8853,7 @@  aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
     {
       case AARCH64_PARSE_INVALID_ARG:
 	error ("unknown value %qs for 'tune' target %s", str, pragma_or_attr);
+	aarch64_print_hint_for_core (str);
 	break;
       default:
 	gcc_unreachable ();
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..76e54e8efe64fa2ccbee21ff75ad9c00d80042b4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+
+__attribute__((target ("arch=armv8-a-typo"))) void
+foo ()
+{
+}
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'armv8-a'?"  "" { target *-*-* } 5 } */
+/* { dg-error "unknown value 'armv8-a-typo' for 'arch' target attribute"  "" { target *-*-* } 5 } */
+/* { dg-error "target attribute 'arch=armv8-a-typo' is invalid"  "" { target *-*-* } 5 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
new file mode 100644
index 0000000000000000000000000000000000000000..80d4795c3f2cf59e5c9b31487e2066549a72371d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+
+__attribute__((target ("cpu=cortex-a57-typo"))) void
+foo ()
+{
+}
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57?"  "" { target *-*-* } 5 } */
+/* { dg-error "unknown value 'cortex-a57-typo' for 'cpu' target attribute"  "" { target *-*-* } 5 } */
+/* { dg-error "target attribute 'cpu=cortex-a57-typo' is invalid"  "" { target *-*-* } 5 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
new file mode 100644
index 0000000000000000000000000000000000000000..ab1940f515e2338cc6eea6f8dc217e97ea781085
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+
+__attribute__((target ("tune=cortex-a57-typo"))) void
+foo ()
+{
+}
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57?"  "" { target *-*-* } 5 } */
+/* { dg-error "unknown value 'cortex-a57-typo' for 'tune' target attribute"  "" { target *-*-* } 5 } */
+/* { dg-error "target attribute 'tune=cortex-a57-typo' is invalid"  "" { target *-*-* } 5 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_4.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_4.c
new file mode 100644
index 0000000000000000000000000000000000000000..6f66fdc98e28d32dec46d79747cb31a94e9d7529
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_4.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-options "-march=armv8-a-typo" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "unknown value 'armv8-a-typo' for -march"  "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'armv8-a'?"  "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_5.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_5.c
new file mode 100644
index 0000000000000000000000000000000000000000..8ec581f12822380f3c01200225ecf8339cec5161
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_5.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-mcpu=cortex-a17" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "unknown value 'cortex-a17' for -mcpu"  "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57'?"  "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_6.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_6.c
new file mode 100644
index 0000000000000000000000000000000000000000..8fa491b0ca4beae6c13aa90de84235495d8a9c33
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_6.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mtune=*" } { "" } } */
+/* { dg-options "-mtune=cortex-a72-typo" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "unknown value 'cortex-a72-typo' for -mtune"  "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a72'?"  "" { target *-*-* } 0 } */