Re: [PATCH] Provide extension hint for aarch64 target (PR driver/83193).

[Martin Sebor <msebor@gmail.com>]

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
>>>
>