* [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
@ 2018-07-18 15:49 Martin Liška
2018-08-01 13:56 ` Martin Liška
2018-10-04 10:32 ` Kyrill Tkachov
0 siblings, 2 replies; 20+ messages in thread
From: Martin Liška @ 2018-07-18 15:49 UTC (permalink / raw)
To: gcc-patches; +Cc: Ramana Radhakrishnan, Kyrill Tkachov
[-- Attachment #1: Type: text/plain, Size: 1635 bytes --]
Hi.
This patch improves aarch64 feature modifier hints.
May I please ask ARM folks to test the patch?
Thanks,
Martin
gcc/ChangeLog:
2018-07-18 Martin Liska <mliska@suse.cz>
PR driver/83193
* common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
Set invalid_extension when there's any.
(aarch64_get_all_extension_candidates): New function.
(aarch64_rewrite_selected_cpu): Pass NULL as new argument.
* config/aarch64/aarch64-protos.h (aarch64_get_all_extension_candidates):
Declare new function.
* config/aarch64/aarch64.c (aarch64_parse_arch): Record
invalid_feature.
(aarch64_parse_cpu): Likewise.
(aarch64_print_hint_for_feature_modifier): New.
(aarch64_validate_mcpu): Record invalid feature modifier
and print hint for it.
(aarch64_validate_march): Likewise.
(aarch64_handle_attr_arch): Likewise.
(aarch64_handle_attr_cpu): Likewise.
(aarch64_handle_attr_isa_flags): Likewise.
gcc/testsuite/ChangeLog:
2018-07-18 Martin Liska <mliska@suse.cz>
PR driver/83193
* gcc.target/aarch64/spellcheck_7.c: New test.
* gcc.target/aarch64/spellcheck_8.c: New test.
---
gcc/common/config/aarch64/aarch64-common.c | 20 +++++-
gcc/config/aarch64/aarch64-protos.h | 4 +-
gcc/config/aarch64/aarch64.c | 67 +++++++++++++++----
.../gcc.target/aarch64/spellcheck_7.c | 11 +++
.../gcc.target/aarch64/spellcheck_8.c | 12 ++++
5 files changed, 97 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
[-- Attachment #2: 0001-Provide-extension-hint-for-aarch64-target-PR-driver-.patch --]
[-- Type: text/x-patch, Size: 10777 bytes --]
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index 292fb818705..c2994514004 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -175,7 +175,8 @@ static const struct arch_to_arch_name all_architectures[] =
aarch64_parse_opt_result describing the result. */
enum aarch64_parse_opt_result
-aarch64_parse_extension (const char *str, unsigned long *isa_flags)
+aarch64_parse_extension (const char *str, unsigned long *isa_flags,
+ char **invalid_extension)
{
/* The extension string is parsed left to right. */
const struct aarch64_option_extension *opt = NULL;
@@ -226,6 +227,11 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
if (opt->name == NULL)
{
/* Extension not found in list. */
+ if (invalid_extension)
+ {
+ *invalid_extension = xstrdup (str);
+ (*invalid_extension)[len] = '\0';
+ }
return AARCH64_PARSE_INVALID_FEATURE;
}
@@ -235,6 +241,16 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
return AARCH64_PARSE_OK;
}
+/* Append all extension candidates and put them to CANDIDATES vector. */
+
+void
+aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates)
+{
+ const struct aarch64_option_extension *opt;
+ for (opt = all_extensions; opt->name != NULL; opt++)
+ candidates->safe_push (opt->name);
+}
+
/* Return a string representation of ISA_FLAGS. DEFAULT_ARCH_FLAGS
gives the default set of flags which are implied by whatever -march
we'd put out. Our job is to figure out the minimal set of "+" and
@@ -322,7 +338,7 @@ aarch64_rewrite_selected_cpu (const char *name)
fatal_error (input_location, "unknown value %qs for -mcpu", name);
unsigned long extensions = p_to_a->flags;
- aarch64_parse_extension (extension_str.c_str (), &extensions);
+ aarch64_parse_extension (extension_str.c_str (), &extensions, NULL);
std::string outstr = a_to_an->arch_name
+ aarch64_get_extension_string_for_isa_flags (extensions,
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index bc11a781c4b..4db274fb85d 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -550,7 +550,9 @@ bool aarch64_handle_option (struct gcc_options *, struct gcc_options *,
const struct cl_decoded_option *, location_t);
const char *aarch64_rewrite_selected_cpu (const char *name);
enum aarch64_parse_opt_result aarch64_parse_extension (const char *,
- unsigned long *);
+ unsigned long *,
+ char **);
+void aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates);
std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
unsigned long);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1369704da3e..6fa03e4b091 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10229,7 +10229,7 @@ static void initialize_aarch64_code_model (struct gcc_options *);
static enum aarch64_parse_opt_result
aarch64_parse_arch (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, char **invalid_feature)
{
char *ext;
const struct processor *arch;
@@ -10260,7 +10260,7 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_feature);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -10284,7 +10284,7 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
static enum aarch64_parse_opt_result
aarch64_parse_cpu (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, char **invalid_feature)
{
char *ext;
const struct processor *cpu;
@@ -10316,7 +10316,7 @@ aarch64_parse_cpu (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_feature);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -10764,6 +10764,26 @@ aarch64_print_hint_for_arch (const char *str)
aarch64_print_hint_for_core_or_arch (str, true);
}
+
+/* Print a hint with a suggestion for a feature modifier name
+ that most closely resembles what the user passed in STR. */
+
+void
+aarch64_print_hint_for_feature_modifier (const char *str)
+{
+ auto_vec<const char *> candidates;
+ aarch64_get_all_extension_candidates (&candidates);
+ 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);
+ else
+ inform (input_location, "valid arguments are: %s;", s);
+
+ XDELETEVEC (s);
+}
+
/* 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
@@ -10773,8 +10793,9 @@ static bool
aarch64_validate_mcpu (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ char *invalid_feature;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, res, isa_flags);
+ = aarch64_parse_cpu (str, res, isa_flags, &invalid_feature);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -10789,7 +10810,10 @@ aarch64_validate_mcpu (const char *str, const struct processor **res,
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-mcpu=%s%>", str);
+ error ("invalid feature modifier %qs in %<-mcpu=%s%>",
+ invalid_feature, str);
+ aarch64_print_hint_for_feature_modifier (invalid_feature);
+ free (invalid_feature);
break;
default:
gcc_unreachable ();
@@ -10807,8 +10831,9 @@ static bool
aarch64_validate_march (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ char *invalid_feature;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, res, isa_flags);
+ = aarch64_parse_arch (str, res, isa_flags, &invalid_feature);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -10823,7 +10848,10 @@ aarch64_validate_march (const char *str, const struct processor **res,
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-march=%s%>", str);
+ error ("invalid feature modifier %qs in %<-march=%s%>",
+ invalid_feature, str);
+ aarch64_print_hint_for_feature_modifier (invalid_feature);
+ free (invalid_feature);
break;
default:
gcc_unreachable ();
@@ -11223,8 +11251,9 @@ static bool
aarch64_handle_attr_arch (const char *str)
{
const struct processor *tmp_arch = NULL;
+ char *invalid_feature;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags);
+ = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags, &invalid_feature);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11244,7 +11273,10 @@ aarch64_handle_attr_arch (const char *str)
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modified %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_feature, str);
+ aarch64_print_hint_for_feature_modifier (invalid_feature);
+ free (invalid_feature);
break;
default:
gcc_unreachable ();
@@ -11259,8 +11291,9 @@ static bool
aarch64_handle_attr_cpu (const char *str)
{
const struct processor *tmp_cpu = NULL;
+ char *invalid_feature;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags);
+ = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags, &invalid_feature);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11283,7 +11316,10 @@ aarch64_handle_attr_cpu (const char *str)
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modified %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_feature, str);
+ aarch64_print_hint_for_feature_modifier (invalid_feature);
+ free (invalid_feature);
break;
default:
gcc_unreachable ();
@@ -11341,7 +11377,8 @@ aarch64_handle_attr_isa_flags (char *str)
str += 8;
}
- parse_res = aarch64_parse_extension (str, &isa_flags);
+ char *invalid_feature;
+ parse_res = aarch64_parse_extension (str, &isa_flags, &invalid_feature);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11356,7 +11393,9 @@ aarch64_handle_attr_isa_flags (char *str)
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modified %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_feature, str);
+ free (invalid_feature);
break;
default:
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
new file mode 100644
index 00000000000..1350b865162
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-options "-march=armv8-a+typo" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .typo. in .-march=armv8-a\\+typo." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*;'?" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
new file mode 100644
index 00000000000..321678e5ef6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-options "-march=armv8-a+cripto" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .cripto. in .-march=armv8-a\\+cripto." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean .crypto.?" "" { target *-*-* } 0 } */
+
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-07-18 15:49 [PATCH] Provide extension hint for aarch64 target (PR driver/83193) Martin Liška
@ 2018-08-01 13:56 ` Martin Liška
2018-08-23 11:01 ` Martin Liška
2018-10-04 10:32 ` Kyrill Tkachov
1 sibling, 1 reply; 20+ messages in thread
From: Martin Liška @ 2018-08-01 13:56 UTC (permalink / raw)
To: gcc-patches; +Cc: Ramana Radhakrishnan, Kyrill Tkachov
PING^1
On 07/18/2018 05:49 PM, Martin Liška wrote:
> Hi.
>
> This patch improves aarch64 feature modifier hints.
>
> May I please ask ARM folks to test the patch?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2018-07-18 Martin Liska <mliska@suse.cz>
>
> PR driver/83193
> * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
> Set invalid_extension when there's any.
> (aarch64_get_all_extension_candidates): New function.
> (aarch64_rewrite_selected_cpu): Pass NULL as new argument.
> * config/aarch64/aarch64-protos.h (aarch64_get_all_extension_candidates):
> Declare new function.
> * config/aarch64/aarch64.c (aarch64_parse_arch): Record
> invalid_feature.
> (aarch64_parse_cpu): Likewise.
> (aarch64_print_hint_for_feature_modifier): New.
> (aarch64_validate_mcpu): Record invalid feature modifier
> and print hint for it.
> (aarch64_validate_march): Likewise.
> (aarch64_handle_attr_arch): Likewise.
> (aarch64_handle_attr_cpu): Likewise.
> (aarch64_handle_attr_isa_flags): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> 2018-07-18 Martin Liska <mliska@suse.cz>
>
> PR driver/83193
> * gcc.target/aarch64/spellcheck_7.c: New test.
> * gcc.target/aarch64/spellcheck_8.c: New test.
> ---
> gcc/common/config/aarch64/aarch64-common.c | 20 +++++-
> gcc/config/aarch64/aarch64-protos.h | 4 +-
> gcc/config/aarch64/aarch64.c | 67 +++++++++++++++----
> .../gcc.target/aarch64/spellcheck_7.c | 11 +++
> .../gcc.target/aarch64/spellcheck_8.c | 12 ++++
> 5 files changed, 97 insertions(+), 17 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-08-01 13:56 ` Martin Liška
@ 2018-08-23 11:01 ` Martin Liška
2018-09-19 18:11 ` Martin Liška
0 siblings, 1 reply; 20+ messages in thread
From: Martin Liška @ 2018-08-23 11:01 UTC (permalink / raw)
To: gcc-patches; +Cc: Ramana Radhakrishnan, Kyrill Tkachov
PING^2
On 08/01/2018 03:56 PM, Martin Liška wrote:
> PING^1
>
> On 07/18/2018 05:49 PM, Martin Liška wrote:
>> Hi.
>>
>> This patch improves aarch64 feature modifier hints.
>>
>> May I please ask ARM folks to test the patch?
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2018-07-18 Martin Liska <mliska@suse.cz>
>>
>> PR driver/83193
>> * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
>> Set invalid_extension when there's any.
>> (aarch64_get_all_extension_candidates): New function.
>> (aarch64_rewrite_selected_cpu): Pass NULL as new argument.
>> * config/aarch64/aarch64-protos.h (aarch64_get_all_extension_candidates):
>> Declare new function.
>> * config/aarch64/aarch64.c (aarch64_parse_arch): Record
>> invalid_feature.
>> (aarch64_parse_cpu): Likewise.
>> (aarch64_print_hint_for_feature_modifier): New.
>> (aarch64_validate_mcpu): Record invalid feature modifier
>> and print hint for it.
>> (aarch64_validate_march): Likewise.
>> (aarch64_handle_attr_arch): Likewise.
>> (aarch64_handle_attr_cpu): Likewise.
>> (aarch64_handle_attr_isa_flags): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-07-18 Martin Liska <mliska@suse.cz>
>>
>> PR driver/83193
>> * gcc.target/aarch64/spellcheck_7.c: New test.
>> * gcc.target/aarch64/spellcheck_8.c: New test.
>> ---
>> gcc/common/config/aarch64/aarch64-common.c | 20 +++++-
>> gcc/config/aarch64/aarch64-protos.h | 4 +-
>> gcc/config/aarch64/aarch64.c | 67 +++++++++++++++----
>> .../gcc.target/aarch64/spellcheck_7.c | 11 +++
>> .../gcc.target/aarch64/spellcheck_8.c | 12 ++++
>> 5 files changed, 97 insertions(+), 17 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>>
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-08-23 11:01 ` Martin Liška
@ 2018-09-19 18:11 ` Martin Liška
2018-10-04 8:16 ` Martin Liška
0 siblings, 1 reply; 20+ messages in thread
From: Martin Liška @ 2018-09-19 18:11 UTC (permalink / raw)
To: gcc-patches
Cc: Ramana Radhakrishnan, Kyrill Tkachov, Richard.Earnshaw, James Greenhalgh
PING^3
On 8/23/18 1:00 PM, Martin Liška wrote:
> PING^2
>
> On 08/01/2018 03:56 PM, Martin Liška wrote:
>> PING^1
>>
>> On 07/18/2018 05:49 PM, Martin Liška wrote:
>>> Hi.
>>>
>>> This patch improves aarch64 feature modifier hints.
>>>
>>> May I please ask ARM folks to test the patch?
>>> Thanks,
>>> Martin
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-07-18 Martin Liska <mliska@suse.cz>
>>>
>>> PR driver/83193
>>> * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
>>> Set invalid_extension when there's any.
>>> (aarch64_get_all_extension_candidates): New function.
>>> (aarch64_rewrite_selected_cpu): Pass NULL as new argument.
>>> * config/aarch64/aarch64-protos.h (aarch64_get_all_extension_candidates):
>>> Declare new function.
>>> * config/aarch64/aarch64.c (aarch64_parse_arch): Record
>>> invalid_feature.
>>> (aarch64_parse_cpu): Likewise.
>>> (aarch64_print_hint_for_feature_modifier): New.
>>> (aarch64_validate_mcpu): Record invalid feature modifier
>>> and print hint for it.
>>> (aarch64_validate_march): Likewise.
>>> (aarch64_handle_attr_arch): Likewise.
>>> (aarch64_handle_attr_cpu): Likewise.
>>> (aarch64_handle_attr_isa_flags): Likewise.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2018-07-18 Martin Liska <mliska@suse.cz>
>>>
>>> PR driver/83193
>>> * gcc.target/aarch64/spellcheck_7.c: New test.
>>> * gcc.target/aarch64/spellcheck_8.c: New test.
>>> ---
>>> gcc/common/config/aarch64/aarch64-common.c | 20 +++++-
>>> gcc/config/aarch64/aarch64-protos.h | 4 +-
>>> gcc/config/aarch64/aarch64.c | 67 +++++++++++++++----
>>> .../gcc.target/aarch64/spellcheck_7.c | 11 +++
>>> .../gcc.target/aarch64/spellcheck_8.c | 12 ++++
>>> 5 files changed, 97 insertions(+), 17 deletions(-)
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-09-19 18:11 ` Martin Liška
@ 2018-10-04 8:16 ` Martin Liška
0 siblings, 0 replies; 20+ messages in thread
From: Martin Liška @ 2018-10-04 8:16 UTC (permalink / raw)
To: gcc-patches
Cc: Ramana Radhakrishnan, Kyrill Tkachov, Richard.Earnshaw, James Greenhalgh
PING^4
On 9/19/18 8:02 PM, Martin Liška wrote:
> PING^3
>
> On 8/23/18 1:00 PM, Martin Liška wrote:
>> PING^2
>>
>> On 08/01/2018 03:56 PM, Martin Liška wrote:
>>> PING^1
>>>
>>> On 07/18/2018 05:49 PM, Martin Liška wrote:
>>>> Hi.
>>>>
>>>> This patch improves aarch64 feature modifier hints.
>>>>
>>>> May I please ask ARM folks to test the patch?
>>>> Thanks,
>>>> Martin
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2018-07-18 Martin Liska <mliska@suse.cz>
>>>>
>>>> Â Â Â Â Â Â Â Â PR driver/83193
>>>> Â Â Â Â * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
>>>> Â Â Â Â Â Â Â Â Set invalid_extension when there's any.
>>>> Â Â Â Â (aarch64_get_all_extension_candidates): New function.
>>>> Â Â Â Â (aarch64_rewrite_selected_cpu): Pass NULL as new argument.
>>>> Â Â Â Â * config/aarch64/aarch64-protos.h (aarch64_get_all_extension_candidates):
>>>> Â Â Â Â Â Â Â Â Declare new function.
>>>> Â Â Â Â * config/aarch64/aarch64.c (aarch64_parse_arch): Record
>>>> Â Â Â Â Â Â Â Â invalid_feature.
>>>> Â Â Â Â (aarch64_parse_cpu): Likewise.
>>>> Â Â Â Â (aarch64_print_hint_for_feature_modifier): New.
>>>> Â Â Â Â (aarch64_validate_mcpu): Record invalid feature modifier
>>>> Â Â Â Â Â Â Â Â and print hint for it.
>>>> Â Â Â Â (aarch64_validate_march): Likewise.
>>>> Â Â Â Â (aarch64_handle_attr_arch): Likewise.
>>>> Â Â Â Â (aarch64_handle_attr_cpu): Likewise.
>>>> Â Â Â Â (aarch64_handle_attr_isa_flags): Likewise.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2018-07-18 Martin Liska <mliska@suse.cz>
>>>>
>>>> Â Â Â Â Â Â Â Â PR driver/83193
>>>> Â Â Â Â * gcc.target/aarch64/spellcheck_7.c: New test.
>>>> Â Â Â Â * gcc.target/aarch64/spellcheck_8.c: New test.
>>>> ---
>>>>  gcc/common/config/aarch64/aarch64-common.c   | 20 +++++-
>>>>  gcc/config/aarch64/aarch64-protos.h          | 4 +-
>>>>  gcc/config/aarch64/aarch64.c                 | 67 +++++++++++++++----
>>>>  .../gcc.target/aarch64/spellcheck_7.c        | 11 +++
>>>>  .../gcc.target/aarch64/spellcheck_8.c        | 12 ++++
>>>> Â 5 files changed, 97 insertions(+), 17 deletions(-)
>>>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
>>>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>>>>
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-07-18 15:49 [PATCH] Provide extension hint for aarch64 target (PR driver/83193) Martin Liška
2018-08-01 13:56 ` Martin Liška
@ 2018-10-04 10:32 ` Kyrill Tkachov
2018-10-04 14:35 ` Martin Liška
2018-10-08 10:53 ` Martin Liška
1 sibling, 2 replies; 20+ messages in thread
From: Kyrill Tkachov @ 2018-10-04 10:32 UTC (permalink / raw)
To: Martin Liška, gcc-patches
Cc: Ramana Radhakrishnan, Richard Earnshaw (lists), James Greenhalgh
Hi Martin,
On 18/07/18 16:49, Martin Liška wrote:
> Hi.
>
> This patch improves aarch64 feature modifier hints.
>
> May I please ask ARM folks to test the patch?
> Thanks,
> Martin
>
I've bootstrapped and tested it on aarch64-none-linux-gnu and I didn't see any problems.
I like the functionality! It is definitely an improvement to the user experience.
I'm not an aarch64 maintainer but I have some comments on the patch below.
Thanks,
Kyrill
> gcc/ChangeLog:
>
> 2018-07-18 Martin Liska <mliska@suse.cz>
>
> PR driver/83193
> * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
> Set invalid_extension when there's any.
> (aarch64_get_all_extension_candidates): New function.
> (aarch64_rewrite_selected_cpu): Pass NULL as new argument.
> * config/aarch64/aarch64-protos.h (aarch64_get_all_extension_candidates):
> Declare new function.
> * config/aarch64/aarch64.c (aarch64_parse_arch): Record
> invalid_feature.
> (aarch64_parse_cpu): Likewise.
> (aarch64_print_hint_for_feature_modifier): New.
> (aarch64_validate_mcpu): Record invalid feature modifier
> and print hint for it.
> (aarch64_validate_march): Likewise.
> (aarch64_handle_attr_arch): Likewise.
> (aarch64_handle_attr_cpu): Likewise.
> (aarch64_handle_attr_isa_flags): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> 2018-07-18 Martin Liska <mliska@suse.cz>
>
> PR driver/83193
> * gcc.target/aarch64/spellcheck_7.c: New test.
> * gcc.target/aarch64/spellcheck_8.c: New test.
> ---
> gcc/common/config/aarch64/aarch64-common.c | 20 +++++-
> gcc/config/aarch64/aarch64-protos.h | 4 +-
> gcc/config/aarch64/aarch64.c | 67 +++++++++++++++----
> .../gcc.target/aarch64/spellcheck_7.c | 11 +++
> .../gcc.target/aarch64/spellcheck_8.c | 12 ++++
> 5 files changed, 97 insertions(+), 17 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>
>
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index 292fb818705..c2994514004 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -175,7 +175,8 @@ static const struct arch_to_arch_name all_architectures[] =
aarch64_parse_opt_result describing the result. */
enum aarch64_parse_opt_result
-aarch64_parse_extension (const char *str, unsigned long *isa_flags)
+aarch64_parse_extension (const char *str, unsigned long *isa_flags,
+ char **invalid_extension)
{
Please document the new argument in the function comment. Same for all the other parsing functions that you extend in the patch.
In particular, I'd like to see a comment on who allocates the memory for the string and who is responsible for freeing it.
+/* Append all extension candidates and put them to CANDIDATES vector. */
+
Given the implementation below, how about:
"Append all architecture extension candidates to the CANDIDATES vector." ?
+void
+aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates)
+{
+ const struct aarch64_option_extension *opt;
+ for (opt = all_extensions; opt->name != NULL; opt++)
+ candidates->safe_push (opt->name);
+}
+
<snip>
+
+/* Print a hint with a suggestion for a feature modifier name
+ that most closely resembles what the user passed in STR. */
+
+void
+aarch64_print_hint_for_feature_modifier (const char *str)
+{
Elsewhere in the backend and this patch as well we call them extensions rather than feature modifiers.
Let's be consistent: aarch64_print_hint_for_extension would is my suggestion
+ auto_vec<const char *> candidates;
+ aarch64_get_all_extension_candidates (&candidates);
+ 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);
+ else
+ inform (input_location, "valid arguments are: %s;", s);
+
+ XDELETEVEC (s);
+}
+
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
new file mode 100644
index 00000000000..1350b865162
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
Another way that the extensions are used is with -mcpu. For example -mcpu=cortex-a57+crypto.
So you'll need to skip if -mcpu is given as well.
And we'll want a test for -mcpu error-checking as well.
+/* { dg-options "-march=armv8-a+typo" } */
+
+void
+foo ()
+{
+}
+
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-04 10:32 ` Kyrill Tkachov
@ 2018-10-04 14:35 ` Martin Liška
2018-10-08 10:53 ` Martin Liška
1 sibling, 0 replies; 20+ messages in thread
From: Martin Liška @ 2018-10-04 14:35 UTC (permalink / raw)
To: Kyrill Tkachov, gcc-patches
Cc: Ramana Radhakrishnan, Richard Earnshaw (lists), James Greenhalgh
On 10/4/18 11:29 AM, Kyrill Tkachov wrote:
> Hi Martin,
>
> On 18/07/18 16:49, Martin Liška wrote:
>> Hi.
>>
>> This patch improves aarch64 feature modifier hints.
>>
>> May I please ask ARM folks to test the patch?
>> Thanks,
>> Martin
>>
>
> I've bootstrapped and tested it on aarch64-none-linux-gnu and I didn't see any problems.
> I like the functionality! It is definitely an improvement to the user experience.
>
> I'm not an aarch64 maintainer but I have some comments on the patch below.
Hi.
Thanks for it.
>
> Thanks,
> Kyrill
>
>> gcc/ChangeLog:
>>
>> 2018-07-18 Martin Liska <mliska@suse.cz>
>>
>> Â Â Â Â Â Â Â PR driver/83193
>> Â Â Â Â Â Â Â * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
>> Â Â Â Â Â Â Â Set invalid_extension when there's any.
>> Â Â Â Â Â Â Â (aarch64_get_all_extension_candidates): New function.
>> Â Â Â Â Â Â Â (aarch64_rewrite_selected_cpu): Pass NULL as new argument.
>> Â Â Â Â Â Â Â * config/aarch64/aarch64-protos.h (aarch64_get_all_extension_candidates):
>> Â Â Â Â Â Â Â Declare new function.
>> Â Â Â Â Â Â Â * config/aarch64/aarch64.c (aarch64_parse_arch): Record
>> Â Â Â Â Â Â Â invalid_feature.
>> Â Â Â Â Â Â Â (aarch64_parse_cpu): Likewise.
>> Â Â Â Â Â Â Â (aarch64_print_hint_for_feature_modifier): New.
>> Â Â Â Â Â Â Â (aarch64_validate_mcpu): Record invalid feature modifier
>> Â Â Â Â Â Â Â and print hint for it.
>> Â Â Â Â Â Â Â (aarch64_validate_march): Likewise.
>> Â Â Â Â Â Â Â (aarch64_handle_attr_arch): Likewise.
>> Â Â Â Â Â Â Â (aarch64_handle_attr_cpu): Likewise.
>> Â Â Â Â Â Â Â (aarch64_handle_attr_isa_flags): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-07-18 Martin Liska <mliska@suse.cz>
>>
>> Â Â Â Â Â Â Â PR driver/83193
>> Â Â Â Â Â Â Â * gcc.target/aarch64/spellcheck_7.c: New test.
>> Â Â Â Â Â Â Â * gcc.target/aarch64/spellcheck_8.c: New test.
>> ---
>>  gcc/common/config/aarch64/aarch64-common.c   | 20 +++++-
>>  gcc/config/aarch64/aarch64-protos.h          | 4 +-
>>  gcc/config/aarch64/aarch64.c                 | 67 +++++++++++++++----
>>  .../gcc.target/aarch64/spellcheck_7.c        | 11 +++
>>  .../gcc.target/aarch64/spellcheck_8.c        | 12 ++++
>> Â 5 files changed, 97 insertions(+), 17 deletions(-)
>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>>
>>
>
> diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
> index 292fb818705..c2994514004 100644
> --- a/gcc/common/config/aarch64/aarch64-common.c
> +++ b/gcc/common/config/aarch64/aarch64-common.c
> @@ -175,7 +175,8 @@ static const struct arch_to_arch_name all_architectures[] =
>    aarch64_parse_opt_result describing the result. */
> Â
> Â enum aarch64_parse_opt_result
> -aarch64_parse_extension (const char *str, unsigned long *isa_flags)
> +aarch64_parse_extension (const char *str, unsigned long *isa_flags,
> +Â Â Â Â Â Â Â Â Â Â Â Â char **invalid_extension)
> Â {
>
> Please document the new argument in the function comment. Same for all the other parsing functions that you extend in the patch.
> In particular, I'd like to see a comment on who allocates the memory for the string and who is responsible for freeing it.
>
> +/* Append all extension candidates and put them to CANDIDATES vector. */
> +
>
> Given the implementation below, how about:
> "Append all architecture extension candidates to the CANDIDATES vector." ?
>
> +void
> +aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates)
> +{
> +Â const struct aarch64_option_extension *opt;
> +Â for (opt = all_extensions; opt->name != NULL; opt++)
> +Â Â Â candidates->safe_push (opt->name);
> +}
> +
>
> <snip>
>
> +
> +/* Print a hint with a suggestion for a feature modifier name
> +  that most closely resembles what the user passed in STR. */
> +
> +void
> +aarch64_print_hint_for_feature_modifier (const char *str)
> +{
>
>
> Elsewhere in the backend and this patch as well we call them extensions rather than feature modifiers.
But currently following error message is printed to user:
cc1: error: invalid feature modifier in '-mcpu=thunderx+cripto'
?
I'm working on another iteration of the patch.
Martin
> Let's be consistent: aarch64_print_hint_for_extension would is my suggestion
>
> +Â auto_vec<const char *> candidates;
> +Â aarch64_get_all_extension_candidates (&candidates);
> +Â 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);
> +Â else
> +Â Â Â inform (input_location, "valid arguments are: %s;", s);
> +
> +Â XDELETEVEC (s);
> +}
> +
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
> new file mode 100644
> index 00000000000..1350b865162
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
>
> Another way that the extensions are used is with -mcpu. For example -mcpu=cortex-a57+crypto.
> So you'll need to skip if -mcpu is given as well.
> And we'll want a test for -mcpu error-checking as well.
>
> +/* { dg-options "-march=armv8-a+typo" } */
> +
> +void
> +foo ()
> +{
> +}
> +
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-04 10:32 ` Kyrill Tkachov
2018-10-04 14:35 ` Martin Liška
@ 2018-10-08 10:53 ` Martin Liška
2018-10-16 16:58 ` Kyrill Tkachov
2018-10-16 17:35 ` James Greenhalgh
1 sibling, 2 replies; 20+ messages in thread
From: Martin Liška @ 2018-10-08 10:53 UTC (permalink / raw)
To: Kyrill Tkachov, gcc-patches
Cc: Ramana Radhakrishnan, Richard Earnshaw (lists), James Greenhalgh
[-- Attachment #1: Type: text/plain, Size: 57 bytes --]
Hi.
I'm attaching updated version of the patch.
Martin
[-- Attachment #2: 0001-Provide-extension-hint-for-aarch64-target-PR-driver-.patch --]
[-- Type: text/x-patch, Size: 14751 bytes --]
From d36974540cda9fb0e159103fdcf92d26ef2f1b94 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 4 Oct 2018 16:31:49 +0200
Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
gcc/ChangeLog:
2018-10-05 Martin Liska <mliska@suse.cz>
PR driver/83193
* common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
Add new argument invalid_extension.
(aarch64_get_all_extension_candidates): New function.
(aarch64_rewrite_selected_cpu): Add NULL to function call.
* config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
new argument.
(aarch64_get_all_extension_candidates): New function.
* config/aarch64/aarch64.c (aarch64_parse_arch): Add new
argument invalid_extension.
(aarch64_parse_cpu): Likewise.
(aarch64_print_hint_for_extensions): New function.
(aarch64_validate_mcpu): Provide hint about invalid extension.
(aarch64_validate_march): Likewise.
(aarch64_handle_attr_arch): Pass new argument.
(aarch64_handle_attr_cpu): Provide hint about invalid extension.
(aarch64_handle_attr_isa_flags): Likewise.
gcc/testsuite/ChangeLog:
2018-10-05 Martin Liska <mliska@suse.cz>
PR driver/83193
* gcc.target/aarch64/spellcheck_7.c: New test.
* gcc.target/aarch64/spellcheck_8.c: New test.
* gcc.target/aarch64/spellcheck_9.c: New test.
---
gcc/common/config/aarch64/aarch64-common.c | 24 +++++-
gcc/config/aarch64/aarch64-protos.h | 4 +-
gcc/config/aarch64/aarch64.c | 75 +++++++++++++++----
.../gcc.target/aarch64/spellcheck_7.c | 12 +++
.../gcc.target/aarch64/spellcheck_8.c | 13 ++++
.../gcc.target/aarch64/spellcheck_9.c | 13 ++++
6 files changed, 121 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index ffddc4d16d8..2bebe70c021 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -220,10 +220,13 @@ static const struct arch_to_arch_name all_architectures[] =
/* Parse the architecture extension string STR and update ISA_FLAGS
with the architecture features turned on or off. Return a
- aarch64_parse_opt_result describing the result. */
+ aarch64_parse_opt_result describing the result.
+ When the STR string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
enum aarch64_parse_opt_result
-aarch64_parse_extension (const char *str, unsigned long *isa_flags)
+aarch64_parse_extension (const char *str, unsigned long *isa_flags,
+ char **invalid_extension)
{
/* The extension string is parsed left to right. */
const struct aarch64_option_extension *opt = NULL;
@@ -274,6 +277,11 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
if (opt->name == NULL)
{
/* Extension not found in list. */
+ if (invalid_extension)
+ {
+ *invalid_extension = xstrdup (str);
+ (*invalid_extension)[len] = '\0';
+ }
return AARCH64_PARSE_INVALID_FEATURE;
}
@@ -283,6 +291,16 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
return AARCH64_PARSE_OK;
}
+/* Append all architecture extension candidates to the CANDIDATES vector. */
+
+void
+aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates)
+{
+ const struct aarch64_option_extension *opt;
+ for (opt = all_extensions; opt->name != NULL; opt++)
+ candidates->safe_push (opt->name);
+}
+
/* Return a string representation of ISA_FLAGS. DEFAULT_ARCH_FLAGS
gives the default set of flags which are implied by whatever -march
we'd put out. Our job is to figure out the minimal set of "+" and
@@ -370,7 +388,7 @@ aarch64_rewrite_selected_cpu (const char *name)
fatal_error (input_location, "unknown value %qs for -mcpu", name);
unsigned long extensions = p_to_a->flags;
- aarch64_parse_extension (extension_str.c_str (), &extensions);
+ aarch64_parse_extension (extension_str.c_str (), &extensions, NULL);
std::string outstr = a_to_an->arch_name
+ aarch64_get_extension_string_for_isa_flags (extensions,
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 5f18837418e..2f1b0b10250 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -616,7 +616,9 @@ bool aarch64_handle_option (struct gcc_options *, struct gcc_options *,
const struct cl_decoded_option *, location_t);
const char *aarch64_rewrite_selected_cpu (const char *name);
enum aarch64_parse_opt_result aarch64_parse_extension (const char *,
- unsigned long *);
+ unsigned long *,
+ char **);
+void aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates);
std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
unsigned long);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d385b246a74..39895c7ede3 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10513,11 +10513,13 @@ static void initialize_aarch64_code_model (struct gcc_options *);
/* Parse the TO_PARSE string and put the architecture struct that it
selects into RES and the architectural features into ISA_FLAGS.
Return an aarch64_parse_opt_result describing the parse result.
- If there is an error parsing, RES and ISA_FLAGS are left unchanged. */
+ If there is an error parsing, RES and ISA_FLAGS are left unchanged.
+ When the TO_PARSE string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
static enum aarch64_parse_opt_result
aarch64_parse_arch (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, char **invalid_extension)
{
char *ext;
const struct processor *arch;
@@ -10548,7 +10550,7 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_extension);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -10568,11 +10570,13 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
/* Parse the TO_PARSE string and put the result tuning in RES and the
architecture flags in ISA_FLAGS. Return an aarch64_parse_opt_result
describing the parse result. If there is an error parsing, RES and
- ISA_FLAGS are left unchanged. */
+ ISA_FLAGS are left unchanged.
+ When the TO_PARSE string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
static enum aarch64_parse_opt_result
aarch64_parse_cpu (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, char **invalid_extension)
{
char *ext;
const struct processor *cpu;
@@ -10604,7 +10608,7 @@ aarch64_parse_cpu (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_extension);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -11086,6 +11090,26 @@ aarch64_print_hint_for_arch (const char *str)
aarch64_print_hint_for_core_or_arch (str, true);
}
+
+/* Print a hint with a suggestion for an extension name
+ that most closely resembles what the user passed in STR. */
+
+void
+aarch64_print_hint_for_extensions (const char *str)
+{
+ auto_vec<const char *> candidates;
+ aarch64_get_all_extension_candidates (&candidates);
+ 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);
+ else
+ inform (input_location, "valid arguments are: %s;", s);
+
+ XDELETEVEC (s);
+}
+
/* 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
@@ -11095,8 +11119,9 @@ static bool
aarch64_validate_mcpu (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ char *invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, res, isa_flags);
+ = aarch64_parse_cpu (str, res, isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -11111,7 +11136,10 @@ aarch64_validate_mcpu (const char *str, const struct processor **res,
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-mcpu=%s%>", str);
+ error ("invalid feature modifier %qs in %<-mcpu=%s%>",
+ invalid_extension, str);
+ aarch64_print_hint_for_extensions (invalid_extension);
+ free (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11129,8 +11157,9 @@ static bool
aarch64_validate_march (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ char *invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, res, isa_flags);
+ = aarch64_parse_arch (str, res, isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -11145,7 +11174,10 @@ aarch64_validate_march (const char *str, const struct processor **res,
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-march=%s%>", str);
+ error ("invalid feature modifier %qs in %<-march=%s%>",
+ invalid_extension, str);
+ aarch64_print_hint_for_extensions (invalid_extension);
+ free (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11545,8 +11577,9 @@ static bool
aarch64_handle_attr_arch (const char *str)
{
const struct processor *tmp_arch = NULL;
+ char *invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags);
+ = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11566,7 +11599,10 @@ aarch64_handle_attr_arch (const char *str)
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension, str);
+ aarch64_print_hint_for_extensions (invalid_extension);
+ free (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11581,8 +11617,9 @@ static bool
aarch64_handle_attr_cpu (const char *str)
{
const struct processor *tmp_cpu = NULL;
+ char *invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags);
+ = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11605,7 +11642,10 @@ aarch64_handle_attr_cpu (const char *str)
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension, str);
+ aarch64_print_hint_for_extensions (invalid_extension);
+ free (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11663,7 +11703,8 @@ aarch64_handle_attr_isa_flags (char *str)
str += 8;
}
- parse_res = aarch64_parse_extension (str, &isa_flags);
+ char *invalid_extension;
+ parse_res = aarch64_parse_extension (str, &isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11678,7 +11719,9 @@ aarch64_handle_attr_isa_flags (char *str)
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modified %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension, str);
+ free (invalid_extension);
break;
default:
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
new file mode 100644
index 00000000000..1d31950cb61
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-march=armv8-a+typo" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .typo. in .-march=armv8-a\\+typo." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*;'?" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
new file mode 100644
index 00000000000..1b8c5ebfeb1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-march=armv8-a+cripto" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .cripto. in .-march=armv8-a\\+cripto." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean .crypto.?" "" { target *-*-* } 0 } */
+
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
new file mode 100644
index 00000000000..ad5b82589c1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-mcpu=thunderx+cripto" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .cripto. in .-mcpu=thunderx\\+cripto." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean .crypto.?" "" { target *-*-* } 0 } */
+
--
2.19.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-08 10:53 ` Martin Liška
@ 2018-10-16 16:58 ` Kyrill Tkachov
2018-10-16 17:35 ` James Greenhalgh
1 sibling, 0 replies; 20+ messages in thread
From: Kyrill Tkachov @ 2018-10-16 16:58 UTC (permalink / raw)
To: Martin Liška, gcc-patches
Cc: Ramana Radhakrishnan, Richard Earnshaw (lists), James Greenhalgh
On 08/10/18 11:34, Martin Liška wrote:
> Hi.
>
> I'm attaching updated version of the patch.
>
> Martin
This looks ok to me, but you'll need approval from a maintainer.
Thanks for the iterations,
Kyrill
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-08 10:53 ` Martin Liška
2018-10-16 16:58 ` Kyrill Tkachov
@ 2018-10-16 17:35 ` James Greenhalgh
2018-10-22 14:03 ` Martin Liška
1 sibling, 1 reply; 20+ messages in thread
From: James Greenhalgh @ 2018-10-16 17:35 UTC (permalink / raw)
To: Martin Liška
Cc: Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
On Mon, Oct 08, 2018 at 05:34:52AM -0500, Martin Liška wrote:
> Hi.
>
> I'm attaching updated version of the patch.
Can't say I'm thrilled by the allocation/free (aarch64_parse_extension
allocates, everyone else has to free) responsibilities here.
If you can clean that up I'd be much happier. The overall patch is OK.
Thanks,
James
> From d36974540cda9fb0e159103fdcf92d26ef2f1b94 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Thu, 4 Oct 2018 16:31:49 +0200
> Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
>
> gcc/ChangeLog:
>
> 2018-10-05 Martin Liska <mliska@suse.cz>
>
> PR driver/83193
> * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
> Add new argument invalid_extension.
> (aarch64_get_all_extension_candidates): New function.
> (aarch64_rewrite_selected_cpu): Add NULL to function call.
> * config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
> new argument.
> (aarch64_get_all_extension_candidates): New function.
> * config/aarch64/aarch64.c (aarch64_parse_arch): Add new
> argument invalid_extension.
> (aarch64_parse_cpu): Likewise.
> (aarch64_print_hint_for_extensions): New function.
> (aarch64_validate_mcpu): Provide hint about invalid extension.
> (aarch64_validate_march): Likewise.
> (aarch64_handle_attr_arch): Pass new argument.
> (aarch64_handle_attr_cpu): Provide hint about invalid extension.
> (aarch64_handle_attr_isa_flags): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> 2018-10-05 Martin Liska <mliska@suse.cz>
>
> PR driver/83193
> * gcc.target/aarch64/spellcheck_7.c: New test.
> * gcc.target/aarch64/spellcheck_8.c: New test.
> * gcc.target/aarch64/spellcheck_9.c: New test.
> ---
> gcc/common/config/aarch64/aarch64-common.c | 24 +++++-
> gcc/config/aarch64/aarch64-protos.h | 4 +-
> gcc/config/aarch64/aarch64.c | 75 +++++++++++++++----
> .../gcc.target/aarch64/spellcheck_7.c | 12 +++
> .../gcc.target/aarch64/spellcheck_8.c | 13 ++++
> .../gcc.target/aarch64/spellcheck_9.c | 13 ++++
> 6 files changed, 121 insertions(+), 20 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-16 17:35 ` James Greenhalgh
@ 2018-10-22 14:03 ` Martin Liška
2018-10-23 13:48 ` [PATCH] Remove extra memory allocation of strings Martin Liška
2018-10-23 19:02 ` [PATCH] Provide extension hint for aarch64 target (PR driver/83193) Martin Sebor
0 siblings, 2 replies; 20+ messages in thread
From: Martin Liška @ 2018-10-22 14:03 UTC (permalink / raw)
To: James Greenhalgh
Cc: Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
[-- Attachment #1: Type: text/plain, Size: 2552 bytes --]
On 10/16/18 6:57 PM, James Greenhalgh wrote:
> On Mon, Oct 08, 2018 at 05:34:52AM -0500, Martin Liška wrote:
>> Hi.
>>
>> I'm attaching updated version of the patch.
>
> Can't say I'm thrilled by the allocation/free (aarch64_parse_extension
> allocates, everyone else has to free) responsibilities here.
Agreed.
>
> If you can clean that up I'd be much happier. The overall patch is OK.
I rewrote that to use std::string, hope it's improvement?
Martin
>
> Thanks,
> James
>
>> From d36974540cda9fb0e159103fdcf92d26ef2f1b94 Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Thu, 4 Oct 2018 16:31:49 +0200
>> Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
>>
>> gcc/ChangeLog:
>>
>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>
>> PR driver/83193
>> * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
>> Add new argument invalid_extension.
>> (aarch64_get_all_extension_candidates): New function.
>> (aarch64_rewrite_selected_cpu): Add NULL to function call.
>> * config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
>> new argument.
>> (aarch64_get_all_extension_candidates): New function.
>> * config/aarch64/aarch64.c (aarch64_parse_arch): Add new
>> argument invalid_extension.
>> (aarch64_parse_cpu): Likewise.
>> (aarch64_print_hint_for_extensions): New function.
>> (aarch64_validate_mcpu): Provide hint about invalid extension.
>> (aarch64_validate_march): Likewise.
>> (aarch64_handle_attr_arch): Pass new argument.
>> (aarch64_handle_attr_cpu): Provide hint about invalid extension.
>> (aarch64_handle_attr_isa_flags): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>
>> PR driver/83193
>> * gcc.target/aarch64/spellcheck_7.c: New test.
>> * gcc.target/aarch64/spellcheck_8.c: New test.
>> * gcc.target/aarch64/spellcheck_9.c: New test.
>> ---
>> gcc/common/config/aarch64/aarch64-common.c | 24 +++++-
>> gcc/config/aarch64/aarch64-protos.h | 4 +-
>> gcc/config/aarch64/aarch64.c | 75 +++++++++++++++----
>> .../gcc.target/aarch64/spellcheck_7.c | 12 +++
>> .../gcc.target/aarch64/spellcheck_8.c | 13 ++++
>> .../gcc.target/aarch64/spellcheck_9.c | 13 ++++
>> 6 files changed, 121 insertions(+), 20 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
>>
[-- Attachment #2: 0001-Provide-extension-hint-for-aarch64-target-PR-driver-.patch --]
[-- Type: text/x-patch, Size: 14728 bytes --]
From 07c1303836b953dc14aecbae475e88a9c88b4c12 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 4 Oct 2018 16:31:49 +0200
Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
gcc/ChangeLog:
2018-10-05 Martin Liska <mliska@suse.cz>
PR driver/83193
* common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
Add new argument invalid_extension.
(aarch64_get_all_extension_candidates): New function.
(aarch64_rewrite_selected_cpu): Add NULL to function call.
* config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
new argument.
(aarch64_get_all_extension_candidates): New function.
* config/aarch64/aarch64.c (aarch64_parse_arch): Add new
argument invalid_extension.
(aarch64_parse_cpu): Likewise.
(aarch64_print_hint_for_extensions): New function.
(aarch64_validate_mcpu): Provide hint about invalid extension.
(aarch64_validate_march): Likewise.
(aarch64_handle_attr_arch): Pass new argument.
(aarch64_handle_attr_cpu): Provide hint about invalid extension.
(aarch64_handle_attr_isa_flags): Likewise.
gcc/testsuite/ChangeLog:
2018-10-05 Martin Liska <mliska@suse.cz>
PR driver/83193
* gcc.target/aarch64/spellcheck_7.c: New test.
* gcc.target/aarch64/spellcheck_8.c: New test.
* gcc.target/aarch64/spellcheck_9.c: New test.
---
gcc/common/config/aarch64/aarch64-common.c | 24 ++++++-
gcc/config/aarch64/aarch64-protos.h | 4 +-
gcc/config/aarch64/aarch64.c | 70 ++++++++++++++-----
.../gcc.target/aarch64/spellcheck_7.c | 12 ++++
.../gcc.target/aarch64/spellcheck_8.c | 13 ++++
.../gcc.target/aarch64/spellcheck_9.c | 13 ++++
6 files changed, 116 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index ffddc4d16d8..2bcb3fb1ee1 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -220,10 +220,13 @@ static const struct arch_to_arch_name all_architectures[] =
/* Parse the architecture extension string STR and update ISA_FLAGS
with the architecture features turned on or off. Return a
- aarch64_parse_opt_result describing the result. */
+ aarch64_parse_opt_result describing the result.
+ When the STR string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
enum aarch64_parse_opt_result
-aarch64_parse_extension (const char *str, unsigned long *isa_flags)
+aarch64_parse_extension (const char *str, unsigned long *isa_flags,
+ std::string *invalid_extension)
{
/* The extension string is parsed left to right. */
const struct aarch64_option_extension *opt = NULL;
@@ -274,6 +277,11 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
if (opt->name == NULL)
{
/* Extension not found in list. */
+ if (invalid_extension)
+ {
+ *invalid_extension = std::string (str);
+ (*invalid_extension)[len] = '\0';
+ }
return AARCH64_PARSE_INVALID_FEATURE;
}
@@ -283,6 +291,16 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
return AARCH64_PARSE_OK;
}
+/* Append all architecture extension candidates to the CANDIDATES vector. */
+
+void
+aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates)
+{
+ const struct aarch64_option_extension *opt;
+ for (opt = all_extensions; opt->name != NULL; opt++)
+ candidates->safe_push (opt->name);
+}
+
/* Return a string representation of ISA_FLAGS. DEFAULT_ARCH_FLAGS
gives the default set of flags which are implied by whatever -march
we'd put out. Our job is to figure out the minimal set of "+" and
@@ -370,7 +388,7 @@ aarch64_rewrite_selected_cpu (const char *name)
fatal_error (input_location, "unknown value %qs for -mcpu", name);
unsigned long extensions = p_to_a->flags;
- aarch64_parse_extension (extension_str.c_str (), &extensions);
+ aarch64_parse_extension (extension_str.c_str (), &extensions, NULL);
std::string outstr = a_to_an->arch_name
+ aarch64_get_extension_string_for_isa_flags (extensions,
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 5f18837418e..0c7927aed7c 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -616,7 +616,9 @@ bool aarch64_handle_option (struct gcc_options *, struct gcc_options *,
const struct cl_decoded_option *, location_t);
const char *aarch64_rewrite_selected_cpu (const char *name);
enum aarch64_parse_opt_result aarch64_parse_extension (const char *,
- unsigned long *);
+ unsigned long *,
+ std::string *);
+void aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates);
std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
unsigned long);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d385b246a74..e3295419154 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10513,11 +10513,13 @@ static void initialize_aarch64_code_model (struct gcc_options *);
/* Parse the TO_PARSE string and put the architecture struct that it
selects into RES and the architectural features into ISA_FLAGS.
Return an aarch64_parse_opt_result describing the parse result.
- If there is an error parsing, RES and ISA_FLAGS are left unchanged. */
+ If there is an error parsing, RES and ISA_FLAGS are left unchanged.
+ When the TO_PARSE string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
static enum aarch64_parse_opt_result
aarch64_parse_arch (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, std::string *invalid_extension)
{
char *ext;
const struct processor *arch;
@@ -10548,7 +10550,7 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_extension);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -10568,11 +10570,13 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
/* Parse the TO_PARSE string and put the result tuning in RES and the
architecture flags in ISA_FLAGS. Return an aarch64_parse_opt_result
describing the parse result. If there is an error parsing, RES and
- ISA_FLAGS are left unchanged. */
+ ISA_FLAGS are left unchanged.
+ When the TO_PARSE string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
static enum aarch64_parse_opt_result
aarch64_parse_cpu (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, std::string *invalid_extension)
{
char *ext;
const struct processor *cpu;
@@ -10604,7 +10608,7 @@ aarch64_parse_cpu (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_extension);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -11086,6 +11090,26 @@ aarch64_print_hint_for_arch (const char *str)
aarch64_print_hint_for_core_or_arch (str, true);
}
+
+/* Print a hint with a suggestion for an extension name
+ that most closely resembles what the user passed in STR. */
+
+void
+aarch64_print_hint_for_extensions (const std::string &str)
+{
+ auto_vec<const char *> candidates;
+ aarch64_get_all_extension_candidates (&candidates);
+ char *s;
+ const char *hint = candidates_list_and_hint (str.c_str (), s, candidates);
+ if (hint)
+ inform (input_location, "valid arguments are: %s;"
+ " did you mean %qs?", s, hint);
+ else
+ inform (input_location, "valid arguments are: %s;", s);
+
+ XDELETEVEC (s);
+}
+
/* 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
@@ -11095,8 +11119,9 @@ static bool
aarch64_validate_mcpu (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, res, isa_flags);
+ = aarch64_parse_cpu (str, res, isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -11111,7 +11136,9 @@ aarch64_validate_mcpu (const char *str, const struct processor **res,
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-mcpu=%s%>", str);
+ error ("invalid feature modifier %qs in %<-mcpu=%s%>",
+ invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11129,8 +11156,9 @@ static bool
aarch64_validate_march (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, res, isa_flags);
+ = aarch64_parse_arch (str, res, isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -11145,7 +11173,9 @@ aarch64_validate_march (const char *str, const struct processor **res,
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-march=%s%>", str);
+ error ("invalid feature modifier %qs in %<-march=%s%>",
+ invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11545,8 +11575,9 @@ static bool
aarch64_handle_attr_arch (const char *str)
{
const struct processor *tmp_arch = NULL;
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags);
+ = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11566,7 +11597,9 @@ aarch64_handle_attr_arch (const char *str)
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11581,8 +11614,9 @@ static bool
aarch64_handle_attr_cpu (const char *str)
{
const struct processor *tmp_cpu = NULL;
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags);
+ = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11605,7 +11639,9 @@ aarch64_handle_attr_cpu (const char *str)
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11663,7 +11699,8 @@ aarch64_handle_attr_isa_flags (char *str)
str += 8;
}
- parse_res = aarch64_parse_extension (str, &isa_flags);
+ std::string invalid_extension;
+ parse_res = aarch64_parse_extension (str, &isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11678,7 +11715,8 @@ aarch64_handle_attr_isa_flags (char *str)
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modified %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
break;
default:
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
new file mode 100644
index 00000000000..1d31950cb61
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-march=armv8-a+typo" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .typo. in .-march=armv8-a\\+typo." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*;'?" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
new file mode 100644
index 00000000000..1b8c5ebfeb1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-march=armv8-a+cripto" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .cripto. in .-march=armv8-a\\+cripto." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean .crypto.?" "" { target *-*-* } 0 } */
+
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
new file mode 100644
index 00000000000..ad5b82589c1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-mcpu=thunderx+cripto" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .cripto. in .-mcpu=thunderx\\+cripto." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean .crypto.?" "" { target *-*-* } 0 } */
+
--
2.19.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] Remove extra memory allocation of strings.
2018-10-22 14:03 ` Martin Liška
@ 2018-10-23 13:48 ` Martin Liška
2018-11-01 10:13 ` Martin Liška
` (2 more replies)
2018-10-23 19:02 ` [PATCH] Provide extension hint for aarch64 target (PR driver/83193) Martin Sebor
1 sibling, 3 replies; 20+ messages in thread
From: Martin Liška @ 2018-10-23 13:48 UTC (permalink / raw)
To: James Greenhalgh
Cc: Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
[-- Attachment #1: Type: text/plain, Size: 168 bytes --]
Hello.
As a follow up patch I would like to remove redundant string allocation
on string which is not needed in my opinion.
That bootstrap on aarch64-linux.
Martin
[-- Attachment #2: 0001-Remove-extra-memory-allocation-of-strings.patch --]
[-- Type: text/x-patch, Size: 3370 bytes --]
From a21a626055442635057985323bb42ef29526e182 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Mon, 22 Oct 2018 15:18:23 +0200
Subject: [PATCH] Remove extra memory allocation of strings.
gcc/ChangeLog:
2018-10-22 Martin Liska <mliska@suse.cz>
* config/aarch64/aarch64.c (aarch64_parse_arch): Do not copy
string to a stack buffer.
(aarch64_parse_cpu): Likewise.
(aarch64_parse_tune): Likewise.
---
gcc/config/aarch64/aarch64.c | 32 ++++++++++++--------------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e3295419154..12c21dd74fb 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10521,19 +10521,16 @@ static enum aarch64_parse_opt_result
aarch64_parse_arch (const char *to_parse, const struct processor **res,
unsigned long *isa_flags, std::string *invalid_extension)
{
- char *ext;
+ const char *ext;
const struct processor *arch;
- char *str = (char *) alloca (strlen (to_parse) + 1);
size_t len;
- strcpy (str, to_parse);
-
- ext = strchr (str, '+');
+ ext = strchr (to_parse, '+');
if (ext != NULL)
- len = ext - str;
+ len = ext - to_parse;
else
- len = strlen (str);
+ len = strlen (to_parse);
if (len == 0)
return AARCH64_PARSE_MISSING_ARG;
@@ -10542,7 +10539,8 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
/* Loop through the list of supported ARCHes to find a match. */
for (arch = all_architectures; arch->name != NULL; arch++)
{
- if (strlen (arch->name) == len && strncmp (arch->name, str, len) == 0)
+ if (strlen (arch->name) == len
+ && strncmp (arch->name, to_parse, len) == 0)
{
unsigned long isa_temp = arch->flags;
@@ -10578,19 +10576,16 @@ static enum aarch64_parse_opt_result
aarch64_parse_cpu (const char *to_parse, const struct processor **res,
unsigned long *isa_flags, std::string *invalid_extension)
{
- char *ext;
+ const char *ext;
const struct processor *cpu;
- char *str = (char *) alloca (strlen (to_parse) + 1);
size_t len;
- strcpy (str, to_parse);
-
- ext = strchr (str, '+');
+ ext = strchr (to_parse, '+');
if (ext != NULL)
- len = ext - str;
+ len = ext - to_parse;
else
- len = strlen (str);
+ len = strlen (to_parse);
if (len == 0)
return AARCH64_PARSE_MISSING_ARG;
@@ -10599,7 +10594,7 @@ aarch64_parse_cpu (const char *to_parse, const struct processor **res,
/* Loop through the list of supported CPUs to find a match. */
for (cpu = all_cores; cpu->name != NULL; cpu++)
{
- if (strlen (cpu->name) == len && strncmp (cpu->name, str, len) == 0)
+ if (strlen (cpu->name) == len && strncmp (cpu->name, to_parse, len) == 0)
{
unsigned long isa_temp = cpu->flags;
@@ -10633,14 +10628,11 @@ static enum aarch64_parse_opt_result
aarch64_parse_tune (const char *to_parse, const struct processor **res)
{
const struct processor *cpu;
- char *str = (char *) alloca (strlen (to_parse) + 1);
-
- strcpy (str, to_parse);
/* Loop through the list of supported CPUs to find a match. */
for (cpu = all_cores; cpu->name != NULL; cpu++)
{
- if (strcmp (cpu->name, str) == 0)
+ if (strcmp (cpu->name, to_parse) == 0)
{
*res = cpu;
return AARCH64_PARSE_OK;
--
2.19.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-22 14:03 ` Martin Liška
2018-10-23 13:48 ` [PATCH] Remove extra memory allocation of strings Martin Liška
@ 2018-10-23 19:02 ` Martin Sebor
2018-10-24 11:48 ` Martin Liška
1 sibling, 1 reply; 20+ messages in thread
From: Martin Sebor @ 2018-10-23 19:02 UTC (permalink / raw)
To: Martin Liška, James Greenhalgh
Cc: Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
On 10/22/2018 07:05 AM, Martin Liška wrote:
> On 10/16/18 6:57 PM, James Greenhalgh wrote:
>> On Mon, Oct 08, 2018 at 05:34:52AM -0500, Martin Liška wrote:
>>> Hi.
>>>
>>> I'm attaching updated version of the patch.
>>
>> Can't say I'm thrilled by the allocation/free (aarch64_parse_extension
>> allocates, everyone else has to free) responsibilities here.
>
> Agreed.
>
>>
>> If you can clean that up I'd be much happier. The overall patch is OK.
>
> I rewrote that to use std::string, hope it's improvement?
If STR below is not nul-terminated the std::string ctor is not
safe. If it is nul-terminated but LEN is equal to its length
then the nul assignment should be unnecessary. If LEN is less
than its length and the goal is to truncate the string then
calling resize() would be the right way to do it. Otherwise,
assigning a nul to an element into the middle won't truncate
(it will leave the remaining elements there). (This may not
matter if the string isn't appended to after that.)
@@ -274,6 +277,11 @@
aarch64_parse_extension (const char *str, unsigned long *isa_flags)
if (opt->name == NULL)
{
/* Extension not found in list. */
+ if (invalid_extension)
+ {
+ *invalid_extension = std::string (str);
+ (*invalid_extension)[len] = '\0';
+ }
I also noticed a minor typo while quickly skimming the rest
of the patch:
@@ -11678,7 +11715,8 @@
aarch64_handle_attr_isa_flags (char *str)
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute",
str);
+ error ("invalid feature modified %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (),
str);
break;
default:
Based on the other messages in the patch the last word in "invalid
feature modified" should be "modifier"
Martin
>
> Martin
>
>>
>> Thanks,
>> James
>>
>>> From d36974540cda9fb0e159103fdcf92d26ef2f1b94 Mon Sep 17 00:00:00 2001
>>> From: marxin <mliska@suse.cz>
>>> Date: Thu, 4 Oct 2018 16:31:49 +0200
>>> Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>>
>>> PR driver/83193
>>> * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
>>> Add new argument invalid_extension.
>>> (aarch64_get_all_extension_candidates): New function.
>>> (aarch64_rewrite_selected_cpu): Add NULL to function call.
>>> * config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
>>> new argument.
>>> (aarch64_get_all_extension_candidates): New function.
>>> * config/aarch64/aarch64.c (aarch64_parse_arch): Add new
>>> argument invalid_extension.
>>> (aarch64_parse_cpu): Likewise.
>>> (aarch64_print_hint_for_extensions): New function.
>>> (aarch64_validate_mcpu): Provide hint about invalid extension.
>>> (aarch64_validate_march): Likewise.
>>> (aarch64_handle_attr_arch): Pass new argument.
>>> (aarch64_handle_attr_cpu): Provide hint about invalid extension.
>>> (aarch64_handle_attr_isa_flags): Likewise.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>>
>>> PR driver/83193
>>> * gcc.target/aarch64/spellcheck_7.c: New test.
>>> * gcc.target/aarch64/spellcheck_8.c: New test.
>>> * gcc.target/aarch64/spellcheck_9.c: New test.
>>> ---
>>> gcc/common/config/aarch64/aarch64-common.c | 24 +++++-
>>> gcc/config/aarch64/aarch64-protos.h | 4 +-
>>> gcc/config/aarch64/aarch64.c | 75 +++++++++++++++----
>>> .../gcc.target/aarch64/spellcheck_7.c | 12 +++
>>> .../gcc.target/aarch64/spellcheck_8.c | 13 ++++
>>> .../gcc.target/aarch64/spellcheck_9.c | 13 ++++
>>> 6 files changed, 121 insertions(+), 20 deletions(-)
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
>>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-23 19:02 ` [PATCH] Provide extension hint for aarch64 target (PR driver/83193) Martin Sebor
@ 2018-10-24 11:48 ` Martin Liška
2018-10-24 19:28 ` Martin Sebor
0 siblings, 1 reply; 20+ messages in thread
From: Martin Liška @ 2018-10-24 11:48 UTC (permalink / raw)
To: Martin Sebor, James Greenhalgh
Cc: Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
[-- Attachment #1: Type: text/plain, Size: 5374 bytes --]
On 10/23/18 6:31 PM, Martin Sebor wrote:
> On 10/22/2018 07:05 AM, Martin Liška wrote:
>> On 10/16/18 6:57 PM, James Greenhalgh wrote:
>>> On Mon, Oct 08, 2018 at 05:34:52AM -0500, Martin Liška wrote:
>>>> Hi.
>>>>
>>>> I'm attaching updated version of the patch.
>>>
>>> Can't say I'm thrilled by the allocation/free (aarch64_parse_extension
>>> allocates, everyone else has to free) responsibilities here.
>>
>> Agreed.
>>
>>>
>>> If you can clean that up I'd be much happier. The overall patch is OK.
>>
>> I rewrote that to use std::string, hope it's improvement?
>
Hi Martin
> If STR below is not nul-terminated the std::string ctor is not
> safe.
Appreciate the help. The string should be null-terminated, it either comes
from GCC command line or it's a valid of an attribute in source code.
If it is nul-terminated but LEN is equal to its length
> then the nul assignment should be unnecessary. If LEN is less
> than its length and the goal is to truncate the string then
> calling resize() would be the right way to do it. Otherwise,
> assigning a nul to an element into the middle won't truncate
> (it will leave the remaining elements there). (This may not
> matter if the string isn't appended to after that.)
That's new for me, I reworked the patch to use resize. Btw. it sounds
a candidate for a new warning ;) ? Must be quite common mistake?
>
> @@ -274,6 +277,11 @@
> Â aarch64_parse_extension (const char *str, unsigned long *isa_flags)
> Â Â Â Â Â Â if (opt->name == NULL)
> Â Â Â Â {
>       /* Extension not found in list. */
> +Â Â Â Â Â if (invalid_extension)
> +Â Â Â Â Â Â Â {
> +Â Â Â Â Â Â Â Â Â *invalid_extension = std::string (str);
> +Â Â Â Â Â Â Â Â Â (*invalid_extension)[len] = '\0';
> +Â Â Â Â Â Â Â }
>
> I also noticed a minor typo while quickly skimming the rest
> of the patch:
Fixed, thanks.
Martin
>
> @@ -11678,7 +11715,8 @@
> Â aarch64_handle_attr_isa_flags (char *str)
> Â Â Â Â break;
>
> Â Â Â Â Â Â case AARCH64_PARSE_INVALID_FEATURE:
> -Â Â Â error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
> +Â Â Â error ("invalid feature modified %s of value (\"%s\") in "
> +Â Â Â Â Â Â Â Â Â Â "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
> Â Â Â Â break;
>
> Â Â Â Â Â Â default:
>
> Based on the other messages in the patch the last word in "invalid
> feature modified" should be "modifier"
>
>
> Martin
>
>>
>> Martin
>>
>>>
>>> Thanks,
>>> James
>>>
>>>> From d36974540cda9fb0e159103fdcf92d26ef2f1b94 Mon Sep 17 00:00:00 2001
>>>> From: marxin <mliska@suse.cz>
>>>> Date: Thu, 4 Oct 2018 16:31:49 +0200
>>>> Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>>>
>>>> Â Â Â Â PR driver/83193
>>>> Â Â Â Â * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
>>>> Â Â Â Â Add new argument invalid_extension.
>>>> Â Â Â Â (aarch64_get_all_extension_candidates): New function.
>>>> Â Â Â Â (aarch64_rewrite_selected_cpu): Add NULL to function call.
>>>> Â Â Â Â * config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
>>>> Â Â Â Â new argument.
>>>> Â Â Â Â (aarch64_get_all_extension_candidates): New function.
>>>> Â Â Â Â * config/aarch64/aarch64.c (aarch64_parse_arch): Add new
>>>> Â Â Â Â argument invalid_extension.
>>>> Â Â Â Â (aarch64_parse_cpu): Likewise.
>>>> Â Â Â Â (aarch64_print_hint_for_extensions): New function.
>>>> Â Â Â Â (aarch64_validate_mcpu): Provide hint about invalid extension.
>>>> Â Â Â Â (aarch64_validate_march): Likewise.
>>>> Â Â Â Â (aarch64_handle_attr_arch): Pass new argument.
>>>> Â Â Â Â (aarch64_handle_attr_cpu): Provide hint about invalid extension.
>>>> Â Â Â Â (aarch64_handle_attr_isa_flags): Likewise.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>>>
>>>> Â Â Â Â PR driver/83193
>>>> Â Â Â Â * gcc.target/aarch64/spellcheck_7.c: New test.
>>>> Â Â Â Â * gcc.target/aarch64/spellcheck_8.c: New test.
>>>> Â Â Â Â * gcc.target/aarch64/spellcheck_9.c: New test.
>>>> ---
>>>>  gcc/common/config/aarch64/aarch64-common.c   | 24 +++++-
>>>>  gcc/config/aarch64/aarch64-protos.h          | 4 +-
>>>>  gcc/config/aarch64/aarch64.c                 | 75 +++++++++++++++----
>>>>  .../gcc.target/aarch64/spellcheck_7.c        | 12 +++
>>>>  .../gcc.target/aarch64/spellcheck_8.c        | 13 ++++
>>>>  .../gcc.target/aarch64/spellcheck_9.c        | 13 ++++
>>>> Â 6 files changed, 121 insertions(+), 20 deletions(-)
>>>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
>>>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>>>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
>>>>
>>
>
[-- Attachment #2: 0001-Provide-extension-hint-for-aarch64-target-PR-driver-.patch --]
[-- Type: text/x-patch, Size: 14727 bytes --]
From 2fcde1deaf6f3dec20ad4dfa68d13ed428497e87 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 4 Oct 2018 16:31:49 +0200
Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
gcc/ChangeLog:
2018-10-05 Martin Liska <mliska@suse.cz>
PR driver/83193
* common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
Add new argument invalid_extension.
(aarch64_get_all_extension_candidates): New function.
(aarch64_rewrite_selected_cpu): Add NULL to function call.
* config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
new argument.
(aarch64_get_all_extension_candidates): New function.
* config/aarch64/aarch64.c (aarch64_parse_arch): Add new
argument invalid_extension.
(aarch64_parse_cpu): Likewise.
(aarch64_print_hint_for_extensions): New function.
(aarch64_validate_mcpu): Provide hint about invalid extension.
(aarch64_validate_march): Likewise.
(aarch64_handle_attr_arch): Pass new argument.
(aarch64_handle_attr_cpu): Provide hint about invalid extension.
(aarch64_handle_attr_isa_flags): Likewise.
gcc/testsuite/ChangeLog:
2018-10-05 Martin Liska <mliska@suse.cz>
PR driver/83193
* gcc.target/aarch64/spellcheck_7.c: New test.
* gcc.target/aarch64/spellcheck_8.c: New test.
* gcc.target/aarch64/spellcheck_9.c: New test.
---
gcc/common/config/aarch64/aarch64-common.c | 24 ++++++-
gcc/config/aarch64/aarch64-protos.h | 4 +-
gcc/config/aarch64/aarch64.c | 70 ++++++++++++++-----
.../gcc.target/aarch64/spellcheck_7.c | 12 ++++
.../gcc.target/aarch64/spellcheck_8.c | 13 ++++
.../gcc.target/aarch64/spellcheck_9.c | 13 ++++
6 files changed, 116 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index ffddc4d16d8..4eede92892f 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -220,10 +220,13 @@ static const struct arch_to_arch_name all_architectures[] =
/* Parse the architecture extension string STR and update ISA_FLAGS
with the architecture features turned on or off. Return a
- aarch64_parse_opt_result describing the result. */
+ aarch64_parse_opt_result describing the result.
+ When the STR string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
enum aarch64_parse_opt_result
-aarch64_parse_extension (const char *str, unsigned long *isa_flags)
+aarch64_parse_extension (const char *str, unsigned long *isa_flags,
+ std::string *invalid_extension)
{
/* The extension string is parsed left to right. */
const struct aarch64_option_extension *opt = NULL;
@@ -274,6 +277,11 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
if (opt->name == NULL)
{
/* Extension not found in list. */
+ if (invalid_extension)
+ {
+ *invalid_extension = std::string (str);
+ invalid_extension->resize (len);
+ }
return AARCH64_PARSE_INVALID_FEATURE;
}
@@ -283,6 +291,16 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
return AARCH64_PARSE_OK;
}
+/* Append all architecture extension candidates to the CANDIDATES vector. */
+
+void
+aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates)
+{
+ const struct aarch64_option_extension *opt;
+ for (opt = all_extensions; opt->name != NULL; opt++)
+ candidates->safe_push (opt->name);
+}
+
/* Return a string representation of ISA_FLAGS. DEFAULT_ARCH_FLAGS
gives the default set of flags which are implied by whatever -march
we'd put out. Our job is to figure out the minimal set of "+" and
@@ -370,7 +388,7 @@ aarch64_rewrite_selected_cpu (const char *name)
fatal_error (input_location, "unknown value %qs for -mcpu", name);
unsigned long extensions = p_to_a->flags;
- aarch64_parse_extension (extension_str.c_str (), &extensions);
+ aarch64_parse_extension (extension_str.c_str (), &extensions, NULL);
std::string outstr = a_to_an->arch_name
+ aarch64_get_extension_string_for_isa_flags (extensions,
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 5f18837418e..0c7927aed7c 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -616,7 +616,9 @@ bool aarch64_handle_option (struct gcc_options *, struct gcc_options *,
const struct cl_decoded_option *, location_t);
const char *aarch64_rewrite_selected_cpu (const char *name);
enum aarch64_parse_opt_result aarch64_parse_extension (const char *,
- unsigned long *);
+ unsigned long *,
+ std::string *);
+void aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates);
std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
unsigned long);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d385b246a74..5bc1f4c1f64 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10513,11 +10513,13 @@ static void initialize_aarch64_code_model (struct gcc_options *);
/* Parse the TO_PARSE string and put the architecture struct that it
selects into RES and the architectural features into ISA_FLAGS.
Return an aarch64_parse_opt_result describing the parse result.
- If there is an error parsing, RES and ISA_FLAGS are left unchanged. */
+ If there is an error parsing, RES and ISA_FLAGS are left unchanged.
+ When the TO_PARSE string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
static enum aarch64_parse_opt_result
aarch64_parse_arch (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, std::string *invalid_extension)
{
char *ext;
const struct processor *arch;
@@ -10548,7 +10550,7 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_extension);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -10568,11 +10570,13 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
/* Parse the TO_PARSE string and put the result tuning in RES and the
architecture flags in ISA_FLAGS. Return an aarch64_parse_opt_result
describing the parse result. If there is an error parsing, RES and
- ISA_FLAGS are left unchanged. */
+ ISA_FLAGS are left unchanged.
+ When the TO_PARSE string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
static enum aarch64_parse_opt_result
aarch64_parse_cpu (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, std::string *invalid_extension)
{
char *ext;
const struct processor *cpu;
@@ -10604,7 +10608,7 @@ aarch64_parse_cpu (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_extension);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -11086,6 +11090,26 @@ aarch64_print_hint_for_arch (const char *str)
aarch64_print_hint_for_core_or_arch (str, true);
}
+
+/* Print a hint with a suggestion for an extension name
+ that most closely resembles what the user passed in STR. */
+
+void
+aarch64_print_hint_for_extensions (const std::string &str)
+{
+ auto_vec<const char *> candidates;
+ aarch64_get_all_extension_candidates (&candidates);
+ char *s;
+ const char *hint = candidates_list_and_hint (str.c_str (), s, candidates);
+ if (hint)
+ inform (input_location, "valid arguments are: %s;"
+ " did you mean %qs?", s, hint);
+ else
+ inform (input_location, "valid arguments are: %s;", s);
+
+ XDELETEVEC (s);
+}
+
/* 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
@@ -11095,8 +11119,9 @@ static bool
aarch64_validate_mcpu (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, res, isa_flags);
+ = aarch64_parse_cpu (str, res, isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -11111,7 +11136,9 @@ aarch64_validate_mcpu (const char *str, const struct processor **res,
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-mcpu=%s%>", str);
+ error ("invalid feature modifier %qs in %<-mcpu=%s%>",
+ invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11129,8 +11156,9 @@ static bool
aarch64_validate_march (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, res, isa_flags);
+ = aarch64_parse_arch (str, res, isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -11145,7 +11173,9 @@ aarch64_validate_march (const char *str, const struct processor **res,
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-march=%s%>", str);
+ error ("invalid feature modifier %qs in %<-march=%s%>",
+ invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11545,8 +11575,9 @@ static bool
aarch64_handle_attr_arch (const char *str)
{
const struct processor *tmp_arch = NULL;
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags);
+ = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11566,7 +11597,9 @@ aarch64_handle_attr_arch (const char *str)
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11581,8 +11614,9 @@ static bool
aarch64_handle_attr_cpu (const char *str)
{
const struct processor *tmp_cpu = NULL;
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags);
+ = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11605,7 +11639,9 @@ aarch64_handle_attr_cpu (const char *str)
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11663,7 +11699,8 @@ aarch64_handle_attr_isa_flags (char *str)
str += 8;
}
- parse_res = aarch64_parse_extension (str, &isa_flags);
+ std::string invalid_extension;
+ parse_res = aarch64_parse_extension (str, &isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11678,7 +11715,8 @@ aarch64_handle_attr_isa_flags (char *str)
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
break;
default:
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
new file mode 100644
index 00000000000..1d31950cb61
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-march=armv8-a+typo" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .typo. in .-march=armv8-a\\+typo." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*;'?" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
new file mode 100644
index 00000000000..1b8c5ebfeb1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-march=armv8-a+cripto" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .cripto. in .-march=armv8-a\\+cripto." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean .crypto.?" "" { target *-*-* } 0 } */
+
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
new file mode 100644
index 00000000000..ad5b82589c1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-mcpu=thunderx+cripto" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .cripto. in .-mcpu=thunderx\\+cripto." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean .crypto.?" "" { target *-*-* } 0 } */
+
--
2.19.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-24 11:48 ` Martin Liška
@ 2018-10-24 19:28 ` Martin Sebor
2018-10-25 12:36 ` Martin Liška
0 siblings, 1 reply; 20+ messages in thread
From: Martin Sebor @ 2018-10-24 19:28 UTC (permalink / raw)
To: Martin Liška, James Greenhalgh
Cc: Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
On 10/24/2018 03:52 AM, Martin Liška wrote:
> On 10/23/18 6:31 PM, Martin Sebor wrote:
>> On 10/22/2018 07:05 AM, Martin Liška wrote:
>>> On 10/16/18 6:57 PM, James Greenhalgh wrote:
>>>> On Mon, Oct 08, 2018 at 05:34:52AM -0500, Martin Liška wrote:
>>>>> Hi.
>>>>>
>>>>> I'm attaching updated version of the patch.
>>>>
>>>> Can't say I'm thrilled by the allocation/free (aarch64_parse_extension
>>>> allocates, everyone else has to free) responsibilities here.
>>>
>>> Agreed.
>>>
>>>>
>>>> If you can clean that up I'd be much happier. The overall patch is OK.
>>>
>>> I rewrote that to use std::string, hope it's improvement?
>>
>
> Hi Martin
>
>> If STR below is not nul-terminated the std::string ctor is not
>> safe.
>
> Appreciate the help. The string should be null-terminated, it either comes
> from GCC command line or it's a valid of an attribute in source code.
>
> If it is nul-terminated but LEN is equal to its length
>> then the nul assignment should be unnecessary. If LEN is less
>> than its length and the goal is to truncate the string then
>> calling resize() would be the right way to do it. Otherwise,
>> assigning a nul to an element into the middle won't truncate
>> (it will leave the remaining elements there). (This may not
>> matter if the string isn't appended to after that.)
>
> That's new for me, I reworked the patch to use resize. Btw. it sounds
> a candidate for a new warning ;) ? Must be quite common mistake?
I should have also mentioned that there is constructor that
takes a pointer and a count:
*invalid_extension = std::string (str, len);
That would be even better than calling resize (sorry about that).
There are lots of opportunities for warnings about misuses of
the standard library. I think we need to first solve
the -Wno-system-headers problem (which disables most warnings
for standard library headers).
Martin
>
>>
>> @@ -274,6 +277,11 @@
>> aarch64_parse_extension (const char *str, unsigned long *isa_flags)
>> if (opt->name == NULL)
>> {
>> /* Extension not found in list. */
>> + if (invalid_extension)
>> + {
>> + *invalid_extension = std::string (str);
>> + (*invalid_extension)[len] = '\0';
>> + }
>>
>> I also noticed a minor typo while quickly skimming the rest
>> of the patch:
>
> Fixed, thanks.
>
> Martin
>
>>
>> @@ -11678,7 +11715,8 @@
>> aarch64_handle_attr_isa_flags (char *str)
>> break;
>>
>> case AARCH64_PARSE_INVALID_FEATURE:
>> - error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
>> + error ("invalid feature modified %s of value (\"%s\") in "
>> + "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
>> break;
>>
>> default:
>>
>> Based on the other messages in the patch the last word in "invalid
>> feature modified" should be "modifier"
>>
>>
>> Martin
>>
>>>
>>> Martin
>>>
>>>>
>>>> Thanks,
>>>> James
>>>>
>>>>> From d36974540cda9fb0e159103fdcf92d26ef2f1b94 Mon Sep 17 00:00:00 2001
>>>>> From: marxin <mliska@suse.cz>
>>>>> Date: Thu, 4 Oct 2018 16:31:49 +0200
>>>>> Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>>>>
>>>>> PR driver/83193
>>>>> * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
>>>>> Add new argument invalid_extension.
>>>>> (aarch64_get_all_extension_candidates): New function.
>>>>> (aarch64_rewrite_selected_cpu): Add NULL to function call.
>>>>> * config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
>>>>> new argument.
>>>>> (aarch64_get_all_extension_candidates): New function.
>>>>> * config/aarch64/aarch64.c (aarch64_parse_arch): Add new
>>>>> argument invalid_extension.
>>>>> (aarch64_parse_cpu): Likewise.
>>>>> (aarch64_print_hint_for_extensions): New function.
>>>>> (aarch64_validate_mcpu): Provide hint about invalid extension.
>>>>> (aarch64_validate_march): Likewise.
>>>>> (aarch64_handle_attr_arch): Pass new argument.
>>>>> (aarch64_handle_attr_cpu): Provide hint about invalid extension.
>>>>> (aarch64_handle_attr_isa_flags): Likewise.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>>>>
>>>>> PR driver/83193
>>>>> * gcc.target/aarch64/spellcheck_7.c: New test.
>>>>> * gcc.target/aarch64/spellcheck_8.c: New test.
>>>>> * gcc.target/aarch64/spellcheck_9.c: New test.
>>>>> ---
>>>>> gcc/common/config/aarch64/aarch64-common.c | 24 +++++-
>>>>> gcc/config/aarch64/aarch64-protos.h | 4 +-
>>>>> gcc/config/aarch64/aarch64.c | 75 +++++++++++++++----
>>>>> .../gcc.target/aarch64/spellcheck_7.c | 12 +++
>>>>> .../gcc.target/aarch64/spellcheck_8.c | 13 ++++
>>>>> .../gcc.target/aarch64/spellcheck_9.c | 13 ++++
>>>>> 6 files changed, 121 insertions(+), 20 deletions(-)
>>>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
>>>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>>>>> create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
>>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-24 19:28 ` Martin Sebor
@ 2018-10-25 12:36 ` Martin Liška
2018-10-30 19:50 ` James Greenhalgh
0 siblings, 1 reply; 20+ messages in thread
From: Martin Liška @ 2018-10-25 12:36 UTC (permalink / raw)
To: Martin Sebor, James Greenhalgh
Cc: Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
[-- Attachment #1: Type: text/plain, Size: 6240 bytes --]
On 10/24/18 7:48 PM, Martin Sebor wrote:
> On 10/24/2018 03:52 AM, Martin Liška wrote:
>> On 10/23/18 6:31 PM, Martin Sebor wrote:
>>> On 10/22/2018 07:05 AM, Martin Liška wrote:
>>>> On 10/16/18 6:57 PM, James Greenhalgh wrote:
>>>>> On Mon, Oct 08, 2018 at 05:34:52AM -0500, Martin Liška wrote:
>>>>>> Hi.
>>>>>>
>>>>>> I'm attaching updated version of the patch.
>>>>>
>>>>> Can't say I'm thrilled by the allocation/free (aarch64_parse_extension
>>>>> allocates, everyone else has to free) responsibilities here.
>>>>
>>>> Agreed.
>>>>
>>>>>
>>>>> If you can clean that up I'd be much happier. The overall patch is OK.
>>>>
>>>> I rewrote that to use std::string, hope it's improvement?
>>>
>>
>> Hi Martin
>>
>>> If STR below is not nul-terminated the std::string ctor is not
>>> safe.
>>
>> Appreciate the help. The string should be null-terminated, it either comes
>> from GCC command line or it's a valid of an attribute in source code.
>>
>> Â If it is nul-terminated but LEN is equal to its length
>>> then the nul assignment should be unnecessary. If LEN is less
>>> than its length and the goal is to truncate the string then
>>> calling resize() would be the right way to do it. Otherwise,
>>> assigning a nul to an element into the middle won't truncate
>>> (it will leave the remaining elements there). (This may not
>>> matter if the string isn't appended to after that.)
>>
>> That's new for me, I reworked the patch to use resize. Btw. it sounds
>> a candidate for a new warning ;) ? Must be quite common mistake?
>
> I should have also mentioned that there is constructor that
> takes a pointer and a count:
>
> Â *invalid_extension = std::string (str, len);
>
> That would be even better than calling resize (sorry about that).
That's fine, I'm sending updated patch. Tested just locally as cross compiler
in valgind.
>
> There are lots of opportunities for warnings about misuses of
> the standard library. I think we need to first solve
> the -Wno-system-headers problem (which disables most warnings
> for standard library headers).
I see!
Martin
>
> Martin
>
>>
>>>
>>> @@ -274,6 +277,11 @@
>>> Â aarch64_parse_extension (const char *str, unsigned long *isa_flags)
>>> Â Â Â Â Â Â if (opt->name == NULL)
>>> Â Â Â Â {
>>>       /* Extension not found in list. */
>>> +Â Â Â Â Â if (invalid_extension)
>>> +Â Â Â Â Â Â Â {
>>> +Â Â Â Â Â Â Â Â Â *invalid_extension = std::string (str);
>>> +Â Â Â Â Â Â Â Â Â (*invalid_extension)[len] = '\0';
>>> +Â Â Â Â Â Â Â }
>>>
>>> I also noticed a minor typo while quickly skimming the rest
>>> of the patch:
>>
>> Fixed, thanks.
>>
>> Martin
>>
>>>
>>> @@ -11678,7 +11715,8 @@
>>> Â aarch64_handle_attr_isa_flags (char *str)
>>> Â Â Â Â break;
>>>
>>> Â Â Â Â Â Â case AARCH64_PARSE_INVALID_FEATURE:
>>> -Â Â Â error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
>>> +Â Â Â error ("invalid feature modified %s of value (\"%s\") in "
>>> +Â Â Â Â Â Â Â Â Â Â "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
>>> Â Â Â Â break;
>>>
>>> Â Â Â Â Â Â default:
>>>
>>> Based on the other messages in the patch the last word in "invalid
>>> feature modified" should be "modifier"
>>>
>>>
>>> Martin
>>>
>>>>
>>>> Martin
>>>>
>>>>>
>>>>> Thanks,
>>>>> James
>>>>>
>>>>>> From d36974540cda9fb0e159103fdcf92d26ef2f1b94 Mon Sep 17 00:00:00 2001
>>>>>> From: marxin <mliska@suse.cz>
>>>>>> Date: Thu, 4 Oct 2018 16:31:49 +0200
>>>>>> Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
>>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>>>>>
>>>>>> Â Â Â PR driver/83193
>>>>>> Â Â Â * common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
>>>>>> Â Â Â Add new argument invalid_extension.
>>>>>> Â Â Â (aarch64_get_all_extension_candidates): New function.
>>>>>> Â Â Â (aarch64_rewrite_selected_cpu): Add NULL to function call.
>>>>>> Â Â Â * config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
>>>>>> Â Â Â new argument.
>>>>>> Â Â Â (aarch64_get_all_extension_candidates): New function.
>>>>>> Â Â Â * config/aarch64/aarch64.c (aarch64_parse_arch): Add new
>>>>>> Â Â Â argument invalid_extension.
>>>>>> Â Â Â (aarch64_parse_cpu): Likewise.
>>>>>> Â Â Â (aarch64_print_hint_for_extensions): New function.
>>>>>> Â Â Â (aarch64_validate_mcpu): Provide hint about invalid extension.
>>>>>> Â Â Â (aarch64_validate_march): Likewise.
>>>>>> Â Â Â (aarch64_handle_attr_arch): Pass new argument.
>>>>>> Â Â Â (aarch64_handle_attr_cpu): Provide hint about invalid extension.
>>>>>> Â Â Â (aarch64_handle_attr_isa_flags): Likewise.
>>>>>>
>>>>>> gcc/testsuite/ChangeLog:
>>>>>>
>>>>>> 2018-10-05 Martin Liska <mliska@suse.cz>
>>>>>>
>>>>>> Â Â Â PR driver/83193
>>>>>> Â Â Â * gcc.target/aarch64/spellcheck_7.c: New test.
>>>>>> Â Â Â * gcc.target/aarch64/spellcheck_8.c: New test.
>>>>>> Â Â Â * gcc.target/aarch64/spellcheck_9.c: New test.
>>>>>> ---
>>>>>>  gcc/common/config/aarch64/aarch64-common.c   | 24 +++++-
>>>>>>  gcc/config/aarch64/aarch64-protos.h          | 4 +-
>>>>>>  gcc/config/aarch64/aarch64.c                 | 75 +++++++++++++++----
>>>>>>  .../gcc.target/aarch64/spellcheck_7.c        | 12 +++
>>>>>>  .../gcc.target/aarch64/spellcheck_8.c        | 13 ++++
>>>>>>  .../gcc.target/aarch64/spellcheck_9.c        | 13 ++++
>>>>>> Â 6 files changed, 121 insertions(+), 20 deletions(-)
>>>>>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
>>>>>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
>>>>>> Â create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
>>>>>>
>>>>
>>>
>>
>
[-- Attachment #2: 0001-Provide-extension-hint-for-aarch64-target-PR-driver-.patch --]
[-- Type: text/x-patch, Size: 14671 bytes --]
From d0e7be2ef3b70957516498acbb39de19ff5bcd1f Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 4 Oct 2018 16:31:49 +0200
Subject: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
gcc/ChangeLog:
2018-10-05 Martin Liska <mliska@suse.cz>
PR driver/83193
* common/config/aarch64/aarch64-common.c (aarch64_parse_extension):
Add new argument invalid_extension.
(aarch64_get_all_extension_candidates): New function.
(aarch64_rewrite_selected_cpu): Add NULL to function call.
* config/aarch64/aarch64-protos.h (aarch64_parse_extension): Add
new argument.
(aarch64_get_all_extension_candidates): New function.
* config/aarch64/aarch64.c (aarch64_parse_arch): Add new
argument invalid_extension.
(aarch64_parse_cpu): Likewise.
(aarch64_print_hint_for_extensions): New function.
(aarch64_validate_mcpu): Provide hint about invalid extension.
(aarch64_validate_march): Likewise.
(aarch64_handle_attr_arch): Pass new argument.
(aarch64_handle_attr_cpu): Provide hint about invalid extension.
(aarch64_handle_attr_isa_flags): Likewise.
gcc/testsuite/ChangeLog:
2018-10-05 Martin Liska <mliska@suse.cz>
PR driver/83193
* gcc.target/aarch64/spellcheck_7.c: New test.
* gcc.target/aarch64/spellcheck_8.c: New test.
* gcc.target/aarch64/spellcheck_9.c: New test.
---
gcc/common/config/aarch64/aarch64-common.c | 21 +++++-
gcc/config/aarch64/aarch64-protos.h | 4 +-
gcc/config/aarch64/aarch64.c | 70 ++++++++++++++-----
.../gcc.target/aarch64/spellcheck_7.c | 12 ++++
.../gcc.target/aarch64/spellcheck_8.c | 13 ++++
.../gcc.target/aarch64/spellcheck_9.c | 13 ++++
6 files changed, 113 insertions(+), 20 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index ffddc4d16d8..dd7d4267340 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -220,10 +220,13 @@ static const struct arch_to_arch_name all_architectures[] =
/* Parse the architecture extension string STR and update ISA_FLAGS
with the architecture features turned on or off. Return a
- aarch64_parse_opt_result describing the result. */
+ aarch64_parse_opt_result describing the result.
+ When the STR string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
enum aarch64_parse_opt_result
-aarch64_parse_extension (const char *str, unsigned long *isa_flags)
+aarch64_parse_extension (const char *str, unsigned long *isa_flags,
+ std::string *invalid_extension)
{
/* The extension string is parsed left to right. */
const struct aarch64_option_extension *opt = NULL;
@@ -274,6 +277,8 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
if (opt->name == NULL)
{
/* Extension not found in list. */
+ if (invalid_extension)
+ *invalid_extension = std::string (str, len);
return AARCH64_PARSE_INVALID_FEATURE;
}
@@ -283,6 +288,16 @@ aarch64_parse_extension (const char *str, unsigned long *isa_flags)
return AARCH64_PARSE_OK;
}
+/* Append all architecture extension candidates to the CANDIDATES vector. */
+
+void
+aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates)
+{
+ const struct aarch64_option_extension *opt;
+ for (opt = all_extensions; opt->name != NULL; opt++)
+ candidates->safe_push (opt->name);
+}
+
/* Return a string representation of ISA_FLAGS. DEFAULT_ARCH_FLAGS
gives the default set of flags which are implied by whatever -march
we'd put out. Our job is to figure out the minimal set of "+" and
@@ -370,7 +385,7 @@ aarch64_rewrite_selected_cpu (const char *name)
fatal_error (input_location, "unknown value %qs for -mcpu", name);
unsigned long extensions = p_to_a->flags;
- aarch64_parse_extension (extension_str.c_str (), &extensions);
+ aarch64_parse_extension (extension_str.c_str (), &extensions, NULL);
std::string outstr = a_to_an->arch_name
+ aarch64_get_extension_string_for_isa_flags (extensions,
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 5f18837418e..0c7927aed7c 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -616,7 +616,9 @@ bool aarch64_handle_option (struct gcc_options *, struct gcc_options *,
const struct cl_decoded_option *, location_t);
const char *aarch64_rewrite_selected_cpu (const char *name);
enum aarch64_parse_opt_result aarch64_parse_extension (const char *,
- unsigned long *);
+ unsigned long *,
+ std::string *);
+void aarch64_get_all_extension_candidates (auto_vec<const char *> *candidates);
std::string aarch64_get_extension_string_for_isa_flags (unsigned long,
unsigned long);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d385b246a74..5bc1f4c1f64 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10513,11 +10513,13 @@ static void initialize_aarch64_code_model (struct gcc_options *);
/* Parse the TO_PARSE string and put the architecture struct that it
selects into RES and the architectural features into ISA_FLAGS.
Return an aarch64_parse_opt_result describing the parse result.
- If there is an error parsing, RES and ISA_FLAGS are left unchanged. */
+ If there is an error parsing, RES and ISA_FLAGS are left unchanged.
+ When the TO_PARSE string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
static enum aarch64_parse_opt_result
aarch64_parse_arch (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, std::string *invalid_extension)
{
char *ext;
const struct processor *arch;
@@ -10548,7 +10550,7 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_extension);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -10568,11 +10570,13 @@ aarch64_parse_arch (const char *to_parse, const struct processor **res,
/* Parse the TO_PARSE string and put the result tuning in RES and the
architecture flags in ISA_FLAGS. Return an aarch64_parse_opt_result
describing the parse result. If there is an error parsing, RES and
- ISA_FLAGS are left unchanged. */
+ ISA_FLAGS are left unchanged.
+ When the TO_PARSE string contains an invalid extension,
+ a copy of the string is created and stored to INVALID_EXTENSION. */
static enum aarch64_parse_opt_result
aarch64_parse_cpu (const char *to_parse, const struct processor **res,
- unsigned long *isa_flags)
+ unsigned long *isa_flags, std::string *invalid_extension)
{
char *ext;
const struct processor *cpu;
@@ -10604,7 +10608,7 @@ aarch64_parse_cpu (const char *to_parse, const struct processor **res,
{
/* TO_PARSE string contains at least one extension. */
enum aarch64_parse_opt_result ext_res
- = aarch64_parse_extension (ext, &isa_temp);
+ = aarch64_parse_extension (ext, &isa_temp, invalid_extension);
if (ext_res != AARCH64_PARSE_OK)
return ext_res;
@@ -11086,6 +11090,26 @@ aarch64_print_hint_for_arch (const char *str)
aarch64_print_hint_for_core_or_arch (str, true);
}
+
+/* Print a hint with a suggestion for an extension name
+ that most closely resembles what the user passed in STR. */
+
+void
+aarch64_print_hint_for_extensions (const std::string &str)
+{
+ auto_vec<const char *> candidates;
+ aarch64_get_all_extension_candidates (&candidates);
+ char *s;
+ const char *hint = candidates_list_and_hint (str.c_str (), s, candidates);
+ if (hint)
+ inform (input_location, "valid arguments are: %s;"
+ " did you mean %qs?", s, hint);
+ else
+ inform (input_location, "valid arguments are: %s;", s);
+
+ XDELETEVEC (s);
+}
+
/* 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
@@ -11095,8 +11119,9 @@ static bool
aarch64_validate_mcpu (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, res, isa_flags);
+ = aarch64_parse_cpu (str, res, isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -11111,7 +11136,9 @@ aarch64_validate_mcpu (const char *str, const struct processor **res,
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-mcpu=%s%>", str);
+ error ("invalid feature modifier %qs in %<-mcpu=%s%>",
+ invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11129,8 +11156,9 @@ static bool
aarch64_validate_march (const char *str, const struct processor **res,
unsigned long *isa_flags)
{
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, res, isa_flags);
+ = aarch64_parse_arch (str, res, isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
return true;
@@ -11145,7 +11173,9 @@ aarch64_validate_march (const char *str, const struct processor **res,
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid feature modifier in %<-march=%s%>", str);
+ error ("invalid feature modifier %qs in %<-march=%s%>",
+ invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11545,8 +11575,9 @@ static bool
aarch64_handle_attr_arch (const char *str)
{
const struct processor *tmp_arch = NULL;
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags);
+ = aarch64_parse_arch (str, &tmp_arch, &aarch64_isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11566,7 +11597,9 @@ aarch64_handle_attr_arch (const char *str)
aarch64_print_hint_for_arch (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11581,8 +11614,9 @@ static bool
aarch64_handle_attr_cpu (const char *str)
{
const struct processor *tmp_cpu = NULL;
+ std::string invalid_extension;
enum aarch64_parse_opt_result parse_res
- = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags);
+ = aarch64_parse_cpu (str, &tmp_cpu, &aarch64_isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11605,7 +11639,9 @@ aarch64_handle_attr_cpu (const char *str)
aarch64_print_hint_for_core (str);
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
+ aarch64_print_hint_for_extensions (invalid_extension);
break;
default:
gcc_unreachable ();
@@ -11663,7 +11699,8 @@ aarch64_handle_attr_isa_flags (char *str)
str += 8;
}
- parse_res = aarch64_parse_extension (str, &isa_flags);
+ std::string invalid_extension;
+ parse_res = aarch64_parse_extension (str, &isa_flags, &invalid_extension);
if (parse_res == AARCH64_PARSE_OK)
{
@@ -11678,7 +11715,8 @@ aarch64_handle_attr_isa_flags (char *str)
break;
case AARCH64_PARSE_INVALID_FEATURE:
- error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
+ error ("invalid feature modifier %s of value (\"%s\") in "
+ "%<target()%> pragma or attribute", invalid_extension.c_str (), str);
break;
default:
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
new file mode 100644
index 00000000000..1d31950cb61
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_7.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-march=armv8-a+typo" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .typo. in .-march=armv8-a\\+typo." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*;'?" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
new file mode 100644
index 00000000000..1b8c5ebfeb1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_8.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-march=armv8-a+cripto" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .cripto. in .-march=armv8-a\\+cripto." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean .crypto.?" "" { target *-*-* } 0 } */
+
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
new file mode 100644
index 00000000000..ad5b82589c1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_9.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-march=*" } { "" } } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=*" } { "" } } */
+/* { dg-options "-mcpu=thunderx+cripto" } */
+
+void
+foo ()
+{
+}
+
+/* { dg-error "invalid feature modifier .cripto. in .-mcpu=thunderx\\+cripto." "" { target *-*-* } 0 } */
+/* { dg-message "valid arguments are: \[^\n\r]*; did you mean .crypto.?" "" { target *-*-* } 0 } */
+
--
2.19.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).
2018-10-25 12:36 ` Martin Liška
@ 2018-10-30 19:50 ` James Greenhalgh
0 siblings, 0 replies; 20+ messages in thread
From: James Greenhalgh @ 2018-10-30 19:50 UTC (permalink / raw)
To: Martin Liška
Cc: Martin Sebor, Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan,
Richard Earnshaw, nd
On Thu, Oct 25, 2018 at 05:53:22AM -0500, Martin Liška wrote:
> On 10/24/18 7:48 PM, Martin Sebor wrote:
> > On 10/24/2018 03:52 AM, Martin Liška wrote:
> >> On 10/23/18 6:31 PM, Martin Sebor wrote:
> >>> On 10/22/2018 07:05 AM, Martin Liška wrote:
> >>>> On 10/16/18 6:57 PM, James Greenhalgh wrote:
> >>>>> On Mon, Oct 08, 2018 at 05:34:52AM -0500, Martin Liška wrote:
> >>>>>> Hi.
> >>>>>>
> >>>>>> I'm attaching updated version of the patch.
> >>>>>
> >>>>> Can't say I'm thrilled by the allocation/free (aarch64_parse_extension
> >>>>> allocates, everyone else has to free) responsibilities here.
> >>>>
> >>>> Agreed.
> >>>>
> >>>>>
> >>>>> If you can clean that up I'd be much happier. The overall patch is OK.
> >>>>
> >>>> I rewrote that to use std::string, hope it's improvement?
> >>>
> >>
> >> Hi Martin
> >>
> >>> If STR below is not nul-terminated the std::string ctor is not
> >>> safe.
> >>
> >> Appreciate the help. The string should be null-terminated, it either comes
> >> from GCC command line or it's a valid of an attribute in source code.
> >>
> >> Â If it is nul-terminated but LEN is equal to its length
> >>> then the nul assignment should be unnecessary. If LEN is less
> >>> than its length and the goal is to truncate the string then
> >>> calling resize() would be the right way to do it. Otherwise,
> >>> assigning a nul to an element into the middle won't truncate
> >>> (it will leave the remaining elements there). (This may not
> >>> matter if the string isn't appended to after that.)
> >>
> >> That's new for me, I reworked the patch to use resize. Btw. it sounds
> >> a candidate for a new warning ;) ? Must be quite common mistake?
> >
> > I should have also mentioned that there is constructor that
> > takes a pointer and a count:
> >
> > Â *invalid_extension = std::string (str, len);
> >
> > That would be even better than calling resize (sorry about that).
>
> That's fine, I'm sending updated patch. Tested just locally as cross compiler
> in valgind.
>
> >
> > There are lots of opportunities for warnings about misuses of
> > the standard library. I think we need to first solve
> > the -Wno-system-headers problem (which disables most warnings
> > for standard library headers).
>
> I see!
OK.
Thanks,
James
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Remove extra memory allocation of strings.
2018-10-23 13:48 ` [PATCH] Remove extra memory allocation of strings Martin Liška
@ 2018-11-01 10:13 ` Martin Liška
2018-11-08 12:20 ` Kyrill Tkachov
2018-11-08 15:22 ` James Greenhalgh
2 siblings, 0 replies; 20+ messages in thread
From: Martin Liška @ 2018-11-01 10:13 UTC (permalink / raw)
To: James Greenhalgh
Cc: Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
On 10/23/18 3:17 PM, Martin Liška wrote:
> Hello.
>
> As a follow up patch I would like to remove redundant string allocation
> on string which is not needed in my opinion.
>
> That bootstrap on aarch64-linux.
>
> Martin
>
James may I please remind this small patch?
Thanks,
Martin
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Remove extra memory allocation of strings.
2018-10-23 13:48 ` [PATCH] Remove extra memory allocation of strings Martin Liška
2018-11-01 10:13 ` Martin Liška
@ 2018-11-08 12:20 ` Kyrill Tkachov
2018-11-08 15:22 ` James Greenhalgh
2 siblings, 0 replies; 20+ messages in thread
From: Kyrill Tkachov @ 2018-11-08 12:20 UTC (permalink / raw)
To: Martin Liška, James Greenhalgh
Cc: gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
Hi Martin,
On 23/10/18 14:17, Martin Liška wrote:
> Hello.
>
> As a follow up patch I would like to remove redundant string allocation
> on string which is not needed in my opinion.
I think this change is correct, as these functions don't modify the string,
just read it in different ways.
You'll still need approval from a maintainer.
Thanks,
Kyrill
> That bootstrap on aarch64-linux.
>
> Martin
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Remove extra memory allocation of strings.
2018-10-23 13:48 ` [PATCH] Remove extra memory allocation of strings Martin Liška
2018-11-01 10:13 ` Martin Liška
2018-11-08 12:20 ` Kyrill Tkachov
@ 2018-11-08 15:22 ` James Greenhalgh
2 siblings, 0 replies; 20+ messages in thread
From: James Greenhalgh @ 2018-11-08 15:22 UTC (permalink / raw)
To: Martin Liška
Cc: Kyrill Tkachov, gcc-patches, Ramana Radhakrishnan, Richard Earnshaw, nd
On Tue, Oct 23, 2018 at 08:17:43AM -0500, Martin Liška wrote:
> Hello.
>
> As a follow up patch I would like to remove redundant string allocation
> on string which is not needed in my opinion.
>
> That bootstrap on aarch64-linux.
OK,
Thanks,
James
> From a21a626055442635057985323bb42ef29526e182 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Mon, 22 Oct 2018 15:18:23 +0200
> Subject: [PATCH] Remove extra memory allocation of strings.
>
> gcc/ChangeLog:
>
> 2018-10-22 Martin Liska <mliska@suse.cz>
>
> * config/aarch64/aarch64.c (aarch64_parse_arch): Do not copy
> string to a stack buffer.
> (aarch64_parse_cpu): Likewise.
> (aarch64_parse_tune): Likewise.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-11-08 15:22 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-18 15:49 [PATCH] Provide extension hint for aarch64 target (PR driver/83193) Martin Liška
2018-08-01 13:56 ` Martin Liška
2018-08-23 11:01 ` Martin Liška
2018-09-19 18:11 ` Martin Liška
2018-10-04 8:16 ` Martin Liška
2018-10-04 10:32 ` Kyrill Tkachov
2018-10-04 14:35 ` Martin Liška
2018-10-08 10:53 ` Martin Liška
2018-10-16 16:58 ` Kyrill Tkachov
2018-10-16 17:35 ` James Greenhalgh
2018-10-22 14:03 ` Martin Liška
2018-10-23 13:48 ` [PATCH] Remove extra memory allocation of strings Martin Liška
2018-11-01 10:13 ` Martin Liška
2018-11-08 12:20 ` Kyrill Tkachov
2018-11-08 15:22 ` James Greenhalgh
2018-10-23 19:02 ` [PATCH] Provide extension hint for aarch64 target (PR driver/83193) Martin Sebor
2018-10-24 11:48 ` Martin Liška
2018-10-24 19:28 ` Martin Sebor
2018-10-25 12:36 ` Martin Liška
2018-10-30 19:50 ` James Greenhalgh
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).