Message ID | 57F7B6C3.6070708@foss.arm.com |
---|---|
State | New |
Headers | show |
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.
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. >
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 --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 } */