[PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Steve Ellcey]

[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]

This is a new version of my patch to fix PR target/79868, where some
error messages are impossible to translate correctly due to how the
strings are dynamically constructed.  It also includes some format
changes in the error messags to make the messages more consistent with
each other and with other GCC errors.  This was worked out with help
from Martin Sebor.  I also had to fix some tests to match the new error
string formats.

Tested on Aarch64 with no regressions, OK to checkin?

Steve Ellcey
sellcey@cavium.com


2017-09-25  Steve Ellcey  <sellcey@cavium.com>

	PR target/79868
	* config/aarch64/aarch64-c.c (aarch64_pragma_target_parse):
	Change argument type on aarch64_process_target_attr call.
	* config/aarch64/aarch64-protos.h (aarch64_process_target_attr):
	Change argument type.
	* config/aarch64/aarch64.c (aarch64_attribute_info): Change
	field type.
	(aarch64_handle_attr_arch): Change argument type, use boolean
	argument to use different strings in error calls.
	(aarch64_handle_attr_cpu): Ditto.
	(aarch64_handle_attr_tune): Ditto.
	(aarch64_handle_attr_isa_flags): Ditto.
	(aarch64_process_one_target_attr): Ditto.
	(aarch64_process_target_attr): Ditto.
	(aarch64_option_valid_attribute_p): Change argument type on
	aarch64_process_target_attr call.


2017-09-25  Steve Ellcey  <sellcey@cavium.com>

	PR target/79868
	* gcc.target/aarch64/spellcheck_1.c: Update dg-error string to match
	new format.
	* gcc.target/aarch64/spellcheck_2.c: Ditto.
	* gcc.target/aarch64/spellcheck_3.c: Ditto.
	* gcc.target/aarch64/target_attr_11.c: Ditto.
	* gcc.target/aarch64/target_attr_12.c: Ditto.
	* gcc.target/aarch64/target_attr_17.c: Ditto.

[-- Attachment #2: pr79868.patch --]
[-- Type: text/x-patch, Size: 12620 bytes --]

diff --git a/gcc/config/aarch64/aarch64-c.c b/gcc/config/aarch64/aarch64-c.c
index 177e638..c9945db 100644
--- a/gcc/config/aarch64/aarch64-c.c
+++ b/gcc/config/aarch64/aarch64-c.c
@@ -165,7 +165,7 @@ aarch64_pragma_target_parse (tree args, tree pop_target)
      information that it specifies.  */
   if (args)
     {
-      if (!aarch64_process_target_attr (args, "pragma"))
+      if (!aarch64_process_target_attr (args, true))
 	return false;
 
       aarch64_override_options_internal (&global_options);
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index e67c2ed..4323e9e 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -445,7 +445,7 @@ bool aarch64_gen_adjusted_ldpstp (rtx *, bool, scalar_mode, RTX_CODE);
 
 void aarch64_init_builtins (void);
 
-bool aarch64_process_target_attr (tree, const char*);
+bool aarch64_process_target_attr (tree, bool);
 void aarch64_override_options_internal (struct gcc_options *);
 
 rtx aarch64_expand_builtin (tree exp,
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1c14008..122ed5e 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -67,6 +67,7 @@
 #include "common/common-target.h"
 #include "selftest.h"
 #include "selftest-rtl.h"
+#include "intl.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -9554,15 +9555,15 @@ struct aarch64_attribute_info
   const char *name;
   enum aarch64_attr_opt_type attr_type;
   bool allow_neg;
-  bool (*handler) (const char *, const char *);
+  bool (*handler) (const char *, bool);
   enum opt_code opt_num;
 };
 
 /* Handle the ARCH_STR argument to the arch= target attribute.
-   PRAGMA_OR_ATTR is used in potential error messages.  */
+   IS_PRAGMA is used in potential error messages.  */
 
 static bool
-aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
+aarch64_handle_attr_arch (const char *str, bool is_pragma)
 {
   const struct processor *tmp_arch = NULL;
   enum aarch64_parse_opt_result parse_res
@@ -9579,15 +9580,22 @@ aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
   switch (parse_res)
     {
       case AARCH64_PARSE_MISSING_ARG:
-	error ("missing architecture name in 'arch' target %s", pragma_or_attr);
+	error (is_pragma
+	       ? G_("missing name in %<target(\"arch=\")%> pragma")
+	       : G_("missing name in %<target(\"arch=\")%> attribute"));
 	break;
       case AARCH64_PARSE_INVALID_ARG:
-	error ("unknown value %qs for 'arch' target %s", str, pragma_or_attr);
+	error (is_pragma
+	       ? G_("invalid name (\"%s\") in %<target(\"arch=\")%> pragma")
+	       : G_("invalid name (\"%s\") in %<target(\"arch=\")%> attribute"),
+	       str);
 	aarch64_print_hint_for_arch (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
-	error ("invalid feature modifier %qs for 'arch' target %s",
-	       str, pragma_or_attr);
+	error (is_pragma
+	       ? G_("invalid value (\"%s\") in %<target()%> pragma")
+	       : G_("invalid value (\"%s\") in %<target()%> attribute"),
+	       str);
 	break;
       default:
 	gcc_unreachable ();
@@ -9597,10 +9605,10 @@ aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
 }
 
 /* Handle the argument CPU_STR to the cpu= target attribute.
-   PRAGMA_OR_ATTR is used in potential error messages.  */
+   IS_PRAGMA is used in potential error messages.  */
 
 static bool
-aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
+aarch64_handle_attr_cpu (const char *str, bool is_pragma)
 {
   const struct processor *tmp_cpu = NULL;
   enum aarch64_parse_opt_result parse_res
@@ -9620,15 +9628,22 @@ aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
   switch (parse_res)
     {
       case AARCH64_PARSE_MISSING_ARG:
-	error ("missing cpu name in 'cpu' target %s", pragma_or_attr);
+	error (is_pragma
+	       ? G_("missing name in %<target(\"cpu=\")%> pragma")
+	       : G_("missing name in %<target(\"cpu=\")%> attribute"));
 	break;
       case AARCH64_PARSE_INVALID_ARG:
-	error ("unknown value %qs for 'cpu' target %s", str, pragma_or_attr);
+	error (is_pragma
+	       ? G_("invalid name (\"%s\") in %<target(\"cpu=\")%> pragma")
+	       : G_("invalid name (\"%s\") in %<target(\"cpu=\")%> attribute"),
+	       str);
 	aarch64_print_hint_for_core (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
-	error ("invalid feature modifier %qs for 'cpu' target %s",
-	       str, pragma_or_attr);
+	error (is_pragma
+	       ? G_("invalid value (\"%s\") in %<target()%> pragma")
+	       : G_("invalid value (\"%s\") in %<target()%> attribute"),
+	       str);
 	break;
       default:
 	gcc_unreachable ();
@@ -9638,10 +9653,10 @@ aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
 }
 
 /* Handle the argument STR to the tune= target attribute.
-   PRAGMA_OR_ATTR is used in potential error messages.  */
+   IS_PRAGMA is used in potential error messages.  */
 
 static bool
-aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
+aarch64_handle_attr_tune (const char *str, bool is_pragma)
 {
   const struct processor *tmp_tune = NULL;
   enum aarch64_parse_opt_result parse_res
@@ -9658,7 +9673,10 @@ aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
   switch (parse_res)
     {
       case AARCH64_PARSE_INVALID_ARG:
-	error ("unknown value %qs for 'tune' target %s", str, pragma_or_attr);
+	error (is_pragma
+	       ? G_("invalid name (\"%s\") in %<target(\"tune=\")%> pragma")
+	       : G_("invalid name (\"%s\") in %<target(\"tune=\")%> attribute"),
+	       str);
 	aarch64_print_hint_for_core (str);
 	break;
       default:
@@ -9672,10 +9690,10 @@ aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
    For example "+fp+nosimd".  Show any errors if needed.  Return TRUE
    if successful.  Update aarch64_isa_flags to reflect the ISA features
    modified.
-   PRAGMA_OR_ATTR is used in potential error messages.  */
+   IS_PRAGMA is used in potential error messages.  */
 
 static bool
-aarch64_handle_attr_isa_flags (char *str, const char *pragma_or_attr)
+aarch64_handle_attr_isa_flags (char *str, bool is_pragma)
 {
   enum aarch64_parse_opt_result parse_res;
   unsigned long isa_flags = aarch64_isa_flags;
@@ -9699,13 +9717,16 @@ aarch64_handle_attr_isa_flags (char *str, const char *pragma_or_attr)
   switch (parse_res)
     {
       case AARCH64_PARSE_MISSING_ARG:
-	error ("missing feature modifier in target %s %qs",
-	       pragma_or_attr, str);
+	error (is_pragma
+	       ? G_("missing value in %<target()%> pragma")
+	       : G_("missing value in %<target()%> attribute"));
 	break;
 
       case AARCH64_PARSE_INVALID_FEATURE:
-	error ("invalid feature modifier in target %s %qs",
-	       pragma_or_attr, str);
+	error (is_pragma
+	       ? G_("invalid value (\"%s\") in %<target()%> pragma")
+	       : G_("invalid value (\"%s\") in %<target()%> attribute"),
+	       str);
 	break;
 
       default:
@@ -9744,11 +9765,11 @@ static const struct aarch64_attribute_info aarch64_attributes[] =
 
 /* Parse ARG_STR which contains the definition of one target attribute.
    Show appropriate errors if any or return true if the attribute is valid.
-   PRAGMA_OR_ATTR holds the string to use in error messages about whether
+   IS_PRAGMA holds the boolean to tell error messages about whether
    we're processing a target attribute or pragma.  */
 
 static bool
-aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
+aarch64_process_one_target_attr (char *arg_str, bool is_pragma)
 {
   bool invert = false;
 
@@ -9756,7 +9777,9 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 
   if (len == 0)
     {
-      error ("malformed target %s", pragma_or_attr);
+      error (is_pragma
+	     ? G_("malformed %<target()%> pragma")
+	     : G_("malformed %<target()%> attribute"));
       return false;
     }
 
@@ -9772,7 +9795,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
      through the machinery for the rest of the target attributes in this
      function.  */
   if (*str_to_check == '+')
-    return aarch64_handle_attr_isa_flags (str_to_check, pragma_or_attr);
+    return aarch64_handle_attr_isa_flags (str_to_check, is_pragma);
 
   if (len > 3 && strncmp (str_to_check, "no-", 3) == 0)
     {
@@ -9804,8 +9827,10 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 
       if (attr_need_arg_p ^ (arg != NULL))
 	{
-	  error ("target %s %qs does not accept an argument",
-		  pragma_or_attr, str_to_check);
+	  error (is_pragma
+		 ? G_("pragma %<target(\"%s\")%> does not accept an argument")
+		 : G_("attribute %<target(\"%s\")%> does not accept an argument"),
+		 str_to_check);
 	  return false;
 	}
 
@@ -9813,8 +9838,10 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 	 then we can't match.  */
       if (invert && !p_attr->allow_neg)
 	{
-	  error ("target %s %qs does not allow a negated form",
-		  pragma_or_attr, str_to_check);
+	  error (is_pragma
+		 ? G_("pragma %<target(\"%s\")%> does not allow a negated form")
+		 : G_("attribute %<target(\"%s\")%> does not allow a negated form"),
+		 str_to_check);
 	  return false;
 	}
 
@@ -9824,7 +9851,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 	   For example, cpu=, arch=, tune=.  */
 	  case aarch64_attr_custom:
 	    gcc_assert (p_attr->handler);
-	    if (!p_attr->handler (arg, pragma_or_attr))
+	    if (!p_attr->handler (arg, is_pragma))
 	      return false;
 	    break;
 
@@ -9868,8 +9895,10 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 		}
 	      else
 		{
-		  error ("target %s %s=%s is not valid",
-			 pragma_or_attr, str_to_check, arg);
+		  error (is_pragma
+			 ? G_("pragma %<target(\"%s=%s\")%> is not valid")
+			 : G_("attribute %<target(\"%s=%s\")%> is not valid"),
+			 str_to_check, arg);
 		}
 	      break;
 	    }
@@ -9903,12 +9932,12 @@ num_occurences_in_str (char c, char *str)
 }
 
 /* Parse the tree in ARGS that contains the target attribute information
-   and update the global target options space.  PRAGMA_OR_ATTR is a string
-   to be used in error messages, specifying whether this is processing
+   and update the global target options space.  IS_PRAGMA is a boolean
+   to be used by error messages, specifying whether this is processing
    a target attribute or a target pragma.  */
 
 bool
-aarch64_process_target_attr (tree args, const char* pragma_or_attr)
+aarch64_process_target_attr (tree args, bool is_pragma)
 {
   if (TREE_CODE (args) == TREE_LIST)
     {
@@ -9917,7 +9946,7 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
 	  tree head = TREE_VALUE (args);
 	  if (head)
 	    {
-	      if (!aarch64_process_target_attr (head, pragma_or_attr))
+	      if (!aarch64_process_target_attr (head, is_pragma))
 		return false;
 	    }
 	  args = TREE_CHAIN (args);
@@ -9938,7 +9967,9 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
 
   if (len == 0)
     {
-      error ("malformed target %s value", pragma_or_attr);
+      error (is_pragma
+	     ? G_("malformed %<target()%> pragma")
+	     : G_("malformed %<target()%> attribute"));
       return false;
     }
 
@@ -9953,9 +9984,12 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
   while (token)
     {
       num_attrs++;
-      if (!aarch64_process_one_target_attr (token, pragma_or_attr))
+      if (!aarch64_process_one_target_attr (token, is_pragma))
 	{
-	  error ("target %s %qs is invalid", pragma_or_attr, token);
+	  error (is_pragma
+		 ? G_("pragma %<target(\"%s\")%> is not valid")
+		 : G_("attribute %<target(\"%s\")%> is not valid"),
+		 token);
 	  return false;
 	}
 
@@ -9964,8 +9998,10 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
 
   if (num_attrs != num_commas + 1)
     {
-      error ("malformed target %s list %qs",
-	      pragma_or_attr, TREE_STRING_POINTER (args));
+      error (is_pragma
+	? G_("malformed %<target(\"%s\")%> pragma")
+	: G_("malformed %<target(\"%s\")%> attribute"),
+	TREE_STRING_POINTER (args));
       return false;
     }
 
@@ -10025,7 +10061,7 @@ aarch64_option_valid_attribute_p (tree fndecl, tree, tree args, int)
 			TREE_TARGET_OPTION (target_option_current_node));
 
 
-  ret = aarch64_process_target_attr (args, "attribute");
+  ret = aarch64_process_target_attr (args, false);
 
   /* Set up any additional state.  */
   if (ret)

[-- Attachment #3: pr79868-tests.patch --]
[-- Type: text/x-patch, Size: 4328 bytes --]

diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
index ccfe417..33ea312 100644
--- a/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
@@ -4,6 +4,6 @@ __attribute__((target ("arch=armv8-a-typo"))) void
 foo ()
 {
   /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'armv8-a'?"  "" { target *-*-* } .-1 } */
-  /* { dg-error "unknown value 'armv8-a-typo' for 'arch' target attribute"  "" { target *-*-* } .-2 } */
-  /* { dg-error "target attribute 'arch=armv8-a-typo' is invalid"  "" { target *-*-* } .-3 } */
+  /* { dg-error "invalid name \\(\"armv8-a-typo\"\\) in 'target\\(\"arch=\"\\)' attribute"  "" { target *-*-* } .-2 } */
+  /* { dg-error "attribute 'target\\(\"arch=armv8-a-typo\"\\)' is not valid"  "" { target *-*-* } .-3 } */
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
index 42ba51a..533b949 100644
--- a/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
@@ -3,7 +3,7 @@
 __attribute__((target ("cpu=cortex-a57-typo"))) void
 foo ()
 {
-  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57?"  "" { target *-*-* } .-1 } */
-  /* { dg-error "unknown value 'cortex-a57-typo' for 'cpu' target attribute"  "" { target *-*-* } .-2 } */
-  /* { dg-error "target attribute 'cpu=cortex-a57-typo' is invalid"  "" { target *-*-* } .-3 } */
+  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57'?"  "" { target *-*-* } .-1 } */
+  /* { dg-error "invalid name \\(\"cortex-a57-typo\"\\) in 'target\\(\"cpu=\"\\)' attribute"  "" { target *-*-* } .-2 } */
+  /* { dg-error "attribute 'target\\(\"cpu=cortex-a57-typo\"\\)' is not valid"  "" { target *-*-* } .-3 } */
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
index 03d2bbf..017f110 100644
--- a/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
@@ -3,7 +3,7 @@
 __attribute__((target ("tune=cortex-a57-typo"))) void
 foo ()
 {
-  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57?"  "" { target *-*-* } .-1 } */
-  /* { dg-error "unknown value 'cortex-a57-typo' for 'tune' target attribute"  "" { target *-*-* } .-2 } */
-  /* { dg-error "target attribute 'tune=cortex-a57-typo' is invalid"  "" { target *-*-* } .-3 } */
+  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57'?"  "" { target *-*-* } .-1 } */
+  /* { dg-error "invalid name \\(\"cortex-a57-typo\"\\) in 'target\\(\"tune=\"\\)' attribute"  "" { target *-*-* } .-2 } */
+  /* { dg-error "attribute 'target\\(\"tune=cortex-a57-typo\"\\)' is not valid"  "" { target *-*-* } .-3 } */
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_11.c b/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
index 7cfb826..a3df438 100644
--- a/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
+++ b/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
@@ -10,4 +10,4 @@ foo (int a)
 }
 
 /* { dg-error "does not allow a negated form" "" { target *-*-* } 0 } */
-/* { dg-error "is invalid" "" { target *-*-* } 0 } */
+/* { dg-error "is not valid" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_12.c b/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
index 39cb996..8a3a25b 100644
--- a/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
+++ b/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
@@ -10,4 +10,4 @@ foo (int a)
 }
 
 /* { dg-error "does not accept an argument" "" { target *-*-* } 0 } */
-/* { dg-error "is invalid" "" { target *-*-* } 0 } */
+/* { dg-error "is not valid" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_17.c b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
index 483cc6d..2a7a751 100644
--- a/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
+++ b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
@@ -5,4 +5,4 @@ foo (int a)
   return a + 5;
 }
 
-/* { dg-error "target attribute.*is invalid" "" { target *-*-* } 0 } */
\ No newline at end of file
+/* { dg-error "attribute 'target\\(\"invalid-attr-string\"\\)' is not valid" "" { target *-*-* } 0 } */

Re: [PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Steve Ellcey]

Ping.

Steve Ellcey
sellcey@cavium.com

On Mon, 2017-09-25 at 16:25 -0700, Steve Ellcey wrote:
> This is a new version of my patch to fix PR target/79868, where some
> error messages are impossible to translate correctly due to how the
> strings are dynamically constructed.  It also includes some format
> changes in the error messags to make the messages more consistent with
> each other and with other GCC errors.  This was worked out with help
> from Martin Sebor.  I also had to fix some tests to match the new error
> string formats.
> 
> Tested on Aarch64 with no regressions, OK to checkin?
> 
> Steve Ellcey
> sellcey@cavium.com
> 
> 
> 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
> 
> 	PR target/79868
> 	* config/aarch64/aarch64-c.c (aarch64_pragma_target_parse):
> 	Change argument type on aarch64_process_target_attr call.
> 	* config/aarch64/aarch64-protos.h
> (aarch64_process_target_attr):
> 	Change argument type.
> 	* config/aarch64/aarch64.c (aarch64_attribute_info): Change
> 	field type.
> 	(aarch64_handle_attr_arch): Change argument type, use boolean
> 	argument to use different strings in error calls.
> 	(aarch64_handle_attr_cpu): Ditto.
> 	(aarch64_handle_attr_tune): Ditto.
> 	(aarch64_handle_attr_isa_flags): Ditto.
> 	(aarch64_process_one_target_attr): Ditto.
> 	(aarch64_process_target_attr): Ditto.
> 	(aarch64_option_valid_attribute_p): Change argument type on
> 	aarch64_process_target_attr call.
> 
> 
> 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
> 
> 	PR target/79868
> 	* gcc.target/aarch64/spellcheck_1.c: Update dg-error string to
> match
> 	new format.
> 	* gcc.target/aarch64/spellcheck_2.c: Ditto.
> 	* gcc.target/aarch64/spellcheck_3.c: Ditto.
> 	* gcc.target/aarch64/target_attr_11.c: Ditto.
> 	* gcc.target/aarch64/target_attr_12.c: Ditto.
> 	* gcc.target/aarch64/target_attr_17.c: Ditto.

Re: [PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Frédéric Marchal]

Sorry for the delay. I lost track of this discussion.

The patch looks good to me.

I haven't checked the unit tests because I don't understand them. I 
assume they all succeed when you run the tests so they are all ok.

I don't remember seeing a definite answer about the C-only syntax used in 
error messages. If nobody complains, then I don't mind. I have no solution 
to propose anyway.

Thanks for you patch,

Frederic


On Friday 06 October 2017 15:23:38 Steve Ellcey wrote:
> Ping.
> 
> Steve Ellcey
> sellcey@cavium.com
> 
> On Mon, 2017-09-25 at 16:25 -0700, Steve Ellcey wrote:
> > This is a new version of my patch to fix PR target/79868, where some
> > error messages are impossible to translate correctly due to how the
> > strings are dynamically constructed.  It also includes some format
> > changes in the error messags to make the messages more consistent 
with
> > each other and with other GCC errors.  This was worked out with help
> > from Martin Sebor.  I also had to fix some tests to match the new error
> > string formats.
> > 
> > Tested on Aarch64 with no regressions, OK to checkin?
> > 
> > Steve Ellcey
> > sellcey@cavium.com
> > 
> > 
> > 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
> > 
> > 	PR target/79868
> > 	* config/aarch64/aarch64-c.c 
(aarch64_pragma_target_parse):
> > 	Change argument type on aarch64_process_target_attr call.
> > 	* config/aarch64/aarch64-protos.h
> > 
> > (aarch64_process_target_attr):
> > 	Change argument type.
> > 	* config/aarch64/aarch64.c (aarch64_attribute_info): Change
> > 	field type.
> > 	(aarch64_handle_attr_arch): Change argument type, use 
boolean
> > 	argument to use different strings in error calls.
> > 	(aarch64_handle_attr_cpu): Ditto.
> > 	(aarch64_handle_attr_tune): Ditto.
> > 	(aarch64_handle_attr_isa_flags): Ditto.
> > 	(aarch64_process_one_target_attr): Ditto.
> > 	(aarch64_process_target_attr): Ditto.
> > 	(aarch64_option_valid_attribute_p): Change argument type on
> > 	aarch64_process_target_attr call.
> > 
> > 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
> > 
> > 	PR target/79868
> > 	* gcc.target/aarch64/spellcheck_1.c: Update dg-error string to
> > 
> > match
> > 
> > 	new format.
> > 	* gcc.target/aarch64/spellcheck_2.c: Ditto.
> > 	* gcc.target/aarch64/spellcheck_3.c: Ditto.
> > 	* gcc.target/aarch64/target_attr_11.c: Ditto.
> > 	* gcc.target/aarch64/target_attr_12.c: Ditto.
> > 	* gcc.target/aarch64/target_attr_17.c: Ditto.

Re: [PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Steve Ellcey]

Thanks for looking at this Frederic,  I don't see your name in the
MAINTAINERS list so I assume I still need approval from one of the
diagnostic message maintainers (Dodji or David) or maybe an Aarch64
maintainer or a global maintainer.

Ping?

Steve Ellcey
sellcey@cavium.com

On Sun, 2017-10-08 at 13:17 +0200, Frédéric Marchal wrote:
> Sorry for the delay. I lost track of this discussion.
>  
> The patch looks good to me.
>  
> I haven't checked the unit tests because I don't understand them. I
> assume they all succeed when you run the tests so they are all ok.
>  
> I don't remember seeing a definite answer about the C-only syntax
> used in error messages. If nobody complains, then I don't mind. I
> have no solution to propose anyway.
>  
> Thanks for you patch,
>  
> Frederic
>  
>  
> On Friday 06 October 2017 15:23:38 Steve Ellcey wrote:
> > Ping.
> > 
> > Steve Ellcey
> > sellcey@cavium.com
> > 
> > On Mon, 2017-09-25 at 16:25 -0700, Steve Ellcey wrote:
> > > This is a new version of my patch to fix PR target/79868, where
> some
> > > error messages are impossible to translate correctly due to how
> the
> > > strings are dynamically constructed.  It also includes some
> format
> > > changes in the error messags to make the messages more consistent
> with
> > > each other and with other GCC errors.  This was worked out with
> help
> > > from Martin Sebor.  I also had to fix some tests to match the new
> error
> > > string formats.
> > > 
> > > Tested on Aarch64 with no regressions, OK to checkin?
> > > 
> > > Steve Ellcey
> > > sellcey@cavium.com
> > > 
> > > 
> > > 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
> > > 
> > > 	PR target/79868
> > > 	* config/aarch64/aarch64-c.c
> (aarch64_pragma_target_parse):
> > > 	Change argument type on aarch64_process_target_attr call.
> > > 	* config/aarch64/aarch64-protos.h
> > > 
> > > (aarch64_process_target_attr):
> > > 	Change argument type.
> > > 	* config/aarch64/aarch64.c (aarch64_attribute_info):
> Change
> > > 	field type.
> > > 	(aarch64_handle_attr_arch): Change argument type, use
> boolean
> > > 	argument to use different strings in error calls.
> > > 	(aarch64_handle_attr_cpu): Ditto.
> > > 	(aarch64_handle_attr_tune): Ditto.
> > > 	(aarch64_handle_attr_isa_flags): Ditto.
> > > 	(aarch64_process_one_target_attr): Ditto.
> > > 	(aarch64_process_target_attr): Ditto.
> > > 	(aarch64_option_valid_attribute_p): Change argument type
> on
> > > 	aarch64_process_target_attr call.
> > > 
> > > 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
> > > 
> > > 	PR target/79868
> > > 	* gcc.target/aarch64/spellcheck_1.c: Update dg-error
> string to
> > > 
> > > match
> > > 
> > > 	new format.
> > > 	* gcc.target/aarch64/spellcheck_2.c: Ditto.
> > > 	* gcc.target/aarch64/spellcheck_3.c: Ditto.
> > > 	* gcc.target/aarch64/target_attr_11.c: Ditto.
> > > 	* gcc.target/aarch64/target_attr_12.c: Ditto.
> > > 	* gcc.target/aarch64/target_attr_17.c: Ditto.
>  

Re: [PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Kyrill Tkachov]

Hi Steve,

On 26/09/17 00:25, Steve Ellcey wrote:
> This is a new version of my patch to fix PR target/79868, where some
> error messages are impossible to translate correctly due to how the
> strings are dynamically constructed.  It also includes some format
> changes in the error messags to make the messages more consistent with
> each other and with other GCC errors.  This was worked out with help
> from Martin Sebor.  I also had to fix some tests to match the new error
> string formats.
>
> Tested on Aarch64 with no regressions, OK to checkin?
>
> Steve Ellcey
> sellcey@cavium.com
>

This looks ok to me (but I can't approve) with a nit below.
I assume this has been bootstrapped as well as tested on aarch64.

>
> 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
>
>         PR target/79868
>         * config/aarch64/aarch64-c.c (aarch64_pragma_target_parse):
>         Change argument type on aarch64_process_target_attr call.
>         * config/aarch64/aarch64-protos.h (aarch64_process_target_attr):
>         Change argument type.
>         * config/aarch64/aarch64.c (aarch64_attribute_info): Change
>         field type.
>         (aarch64_handle_attr_arch): Change argument type, use boolean
>         argument to use different strings in error calls.
>         (aarch64_handle_attr_cpu): Ditto.
>         (aarch64_handle_attr_tune): Ditto.
>         (aarch64_handle_attr_isa_flags): Ditto.
>         (aarch64_process_one_target_attr): Ditto.
>         (aarch64_process_target_attr): Ditto.
>         (aarch64_option_valid_attribute_p): Change argument type on
>         aarch64_process_target_attr call.
>
>
> 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
>
>         PR target/79868
>         * gcc.target/aarch64/spellcheck_1.c: Update dg-error string to 
> match
>         new format.
>         * gcc.target/aarch64/spellcheck_2.c: Ditto.
>         * gcc.target/aarch64/spellcheck_3.c: Ditto.
>         * gcc.target/aarch64/target_attr_11.c: Ditto.
>         * gcc.target/aarch64/target_attr_12.c: Ditto.
>         * gcc.target/aarch64/target_attr_17.c: Ditto.

  /* Handle the ARCH_STR argument to the arch= target attribute.
-   PRAGMA_OR_ATTR is used in potential error messages.  */
+   IS_PRAGMA is used in potential error messages.  */
  

Please reword this comment to say something along the lines of "IS_PRAGMA is true if processing a target pragma rather than a target attribute". Same with other occurrences of this change in other functions in this patch. I think the comment above aarch64_process_one_target_attr is a pretty good template now.

Thanks,
Kyrill

Re: [PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Richard Earnshaw (lists)]

On 26/09/17 00:25, Steve Ellcey wrote:
> This is a new version of my patch to fix PR target/79868, where some
> error messages are impossible to translate correctly due to how the
> strings are dynamically constructed.  It also includes some format
> changes in the error messags to make the messages more consistent with
> each other and with other GCC errors.  This was worked out with help
> from Martin Sebor.  I also had to fix some tests to match the new error
> string formats.
> 
> Tested on Aarch64 with no regressions, OK to checkin?

I can't help feeling that all this logic is somewhat excessive and
changing the wording of each message to include "pragma or attribute"
would solve it equally well.  With the new context highlighting it's
trivial to tell which subcase of usage is being referred to.

R.

> 
> Steve Ellcey
> sellcey@cavium.com
> 
> 
> 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
> 
> 	PR target/79868
> 	* config/aarch64/aarch64-c.c (aarch64_pragma_target_parse):
> 	Change argument type on aarch64_process_target_attr call.
> 	* config/aarch64/aarch64-protos.h (aarch64_process_target_attr):
> 	Change argument type.
> 	* config/aarch64/aarch64.c (aarch64_attribute_info): Change
> 	field type.
> 	(aarch64_handle_attr_arch): Change argument type, use boolean
> 	argument to use different strings in error calls.
> 	(aarch64_handle_attr_cpu): Ditto.
> 	(aarch64_handle_attr_tune): Ditto.
> 	(aarch64_handle_attr_isa_flags): Ditto.
> 	(aarch64_process_one_target_attr): Ditto.
> 	(aarch64_process_target_attr): Ditto.
> 	(aarch64_option_valid_attribute_p): Change argument type on
> 	aarch64_process_target_attr call.
> 
> 
> 2017-09-25  Steve Ellcey  <sellcey@cavium.com>
> 
> 	PR target/79868
> 	* gcc.target/aarch64/spellcheck_1.c: Update dg-error string to match
> 	new format.
> 	* gcc.target/aarch64/spellcheck_2.c: Ditto.
> 	* gcc.target/aarch64/spellcheck_3.c: Ditto.
> 	* gcc.target/aarch64/target_attr_11.c: Ditto.
> 	* gcc.target/aarch64/target_attr_12.c: Ditto.
> 	* gcc.target/aarch64/target_attr_17.c: Ditto.
> 
> 
> pr79868.patch
> 
> 
> diff --git a/gcc/config/aarch64/aarch64-c.c b/gcc/config/aarch64/aarch64-c.c
> index 177e638..c9945db 100644
> --- a/gcc/config/aarch64/aarch64-c.c
> +++ b/gcc/config/aarch64/aarch64-c.c
> @@ -165,7 +165,7 @@ aarch64_pragma_target_parse (tree args, tree pop_target)
>       information that it specifies.  */
>    if (args)
>      {
> -      if (!aarch64_process_target_attr (args, "pragma"))
> +      if (!aarch64_process_target_attr (args, true))
>  	return false;
>  
>        aarch64_override_options_internal (&global_options);
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index e67c2ed..4323e9e 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -445,7 +445,7 @@ bool aarch64_gen_adjusted_ldpstp (rtx *, bool, scalar_mode, RTX_CODE);
>  
>  void aarch64_init_builtins (void);
>  
> -bool aarch64_process_target_attr (tree, const char*);
> +bool aarch64_process_target_attr (tree, bool);
>  void aarch64_override_options_internal (struct gcc_options *);
>  
>  rtx aarch64_expand_builtin (tree exp,
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 1c14008..122ed5e 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -67,6 +67,7 @@
>  #include "common/common-target.h"
>  #include "selftest.h"
>  #include "selftest-rtl.h"
> +#include "intl.h"
>  
>  /* This file should be included last.  */
>  #include "target-def.h"
> @@ -9554,15 +9555,15 @@ struct aarch64_attribute_info
>    const char *name;
>    enum aarch64_attr_opt_type attr_type;
>    bool allow_neg;
> -  bool (*handler) (const char *, const char *);
> +  bool (*handler) (const char *, bool);
>    enum opt_code opt_num;
>  };
>  
>  /* Handle the ARCH_STR argument to the arch= target attribute.
> -   PRAGMA_OR_ATTR is used in potential error messages.  */
> +   IS_PRAGMA is used in potential error messages.  */
>  
>  static bool
> -aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
> +aarch64_handle_attr_arch (const char *str, bool is_pragma)
>  {
>    const struct processor *tmp_arch = NULL;
>    enum aarch64_parse_opt_result parse_res
> @@ -9579,15 +9580,22 @@ aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
>    switch (parse_res)
>      {
>        case AARCH64_PARSE_MISSING_ARG:
> -	error ("missing architecture name in 'arch' target %s", pragma_or_attr);
> +	error (is_pragma
> +	       ? G_("missing name in %<target(\"arch=\")%> pragma")
> +	       : G_("missing name in %<target(\"arch=\")%> attribute"));
>  	break;
>        case AARCH64_PARSE_INVALID_ARG:
> -	error ("unknown value %qs for 'arch' target %s", str, pragma_or_attr);
> +	error (is_pragma
> +	       ? G_("invalid name (\"%s\") in %<target(\"arch=\")%> pragma")
> +	       : G_("invalid name (\"%s\") in %<target(\"arch=\")%> attribute"),
> +	       str);
>  	aarch64_print_hint_for_arch (str);
>  	break;
>        case AARCH64_PARSE_INVALID_FEATURE:
> -	error ("invalid feature modifier %qs for 'arch' target %s",
> -	       str, pragma_or_attr);
> +	error (is_pragma
> +	       ? G_("invalid value (\"%s\") in %<target()%> pragma")
> +	       : G_("invalid value (\"%s\") in %<target()%> attribute"),
> +	       str);
>  	break;
>        default:
>  	gcc_unreachable ();
> @@ -9597,10 +9605,10 @@ aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
>  }
>  
>  /* Handle the argument CPU_STR to the cpu= target attribute.
> -   PRAGMA_OR_ATTR is used in potential error messages.  */
> +   IS_PRAGMA is used in potential error messages.  */
>  
>  static bool
> -aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
> +aarch64_handle_attr_cpu (const char *str, bool is_pragma)
>  {
>    const struct processor *tmp_cpu = NULL;
>    enum aarch64_parse_opt_result parse_res
> @@ -9620,15 +9628,22 @@ aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
>    switch (parse_res)
>      {
>        case AARCH64_PARSE_MISSING_ARG:
> -	error ("missing cpu name in 'cpu' target %s", pragma_or_attr);
> +	error (is_pragma
> +	       ? G_("missing name in %<target(\"cpu=\")%> pragma")
> +	       : G_("missing name in %<target(\"cpu=\")%> attribute"));
>  	break;
>        case AARCH64_PARSE_INVALID_ARG:
> -	error ("unknown value %qs for 'cpu' target %s", str, pragma_or_attr);
> +	error (is_pragma
> +	       ? G_("invalid name (\"%s\") in %<target(\"cpu=\")%> pragma")
> +	       : G_("invalid name (\"%s\") in %<target(\"cpu=\")%> attribute"),
> +	       str);
>  	aarch64_print_hint_for_core (str);
>  	break;
>        case AARCH64_PARSE_INVALID_FEATURE:
> -	error ("invalid feature modifier %qs for 'cpu' target %s",
> -	       str, pragma_or_attr);
> +	error (is_pragma
> +	       ? G_("invalid value (\"%s\") in %<target()%> pragma")
> +	       : G_("invalid value (\"%s\") in %<target()%> attribute"),
> +	       str);
>  	break;
>        default:
>  	gcc_unreachable ();
> @@ -9638,10 +9653,10 @@ aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
>  }
>  
>  /* Handle the argument STR to the tune= target attribute.
> -   PRAGMA_OR_ATTR is used in potential error messages.  */
> +   IS_PRAGMA is used in potential error messages.  */
>  
>  static bool
> -aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
> +aarch64_handle_attr_tune (const char *str, bool is_pragma)
>  {
>    const struct processor *tmp_tune = NULL;
>    enum aarch64_parse_opt_result parse_res
> @@ -9658,7 +9673,10 @@ aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
>    switch (parse_res)
>      {
>        case AARCH64_PARSE_INVALID_ARG:
> -	error ("unknown value %qs for 'tune' target %s", str, pragma_or_attr);
> +	error (is_pragma
> +	       ? G_("invalid name (\"%s\") in %<target(\"tune=\")%> pragma")
> +	       : G_("invalid name (\"%s\") in %<target(\"tune=\")%> attribute"),
> +	       str);
>  	aarch64_print_hint_for_core (str);
>  	break;
>        default:
> @@ -9672,10 +9690,10 @@ aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
>     For example "+fp+nosimd".  Show any errors if needed.  Return TRUE
>     if successful.  Update aarch64_isa_flags to reflect the ISA features
>     modified.
> -   PRAGMA_OR_ATTR is used in potential error messages.  */
> +   IS_PRAGMA is used in potential error messages.  */
>  
>  static bool
> -aarch64_handle_attr_isa_flags (char *str, const char *pragma_or_attr)
> +aarch64_handle_attr_isa_flags (char *str, bool is_pragma)
>  {
>    enum aarch64_parse_opt_result parse_res;
>    unsigned long isa_flags = aarch64_isa_flags;
> @@ -9699,13 +9717,16 @@ aarch64_handle_attr_isa_flags (char *str, const char *pragma_or_attr)
>    switch (parse_res)
>      {
>        case AARCH64_PARSE_MISSING_ARG:
> -	error ("missing feature modifier in target %s %qs",
> -	       pragma_or_attr, str);
> +	error (is_pragma
> +	       ? G_("missing value in %<target()%> pragma")
> +	       : G_("missing value in %<target()%> attribute"));
>  	break;
>  
>        case AARCH64_PARSE_INVALID_FEATURE:
> -	error ("invalid feature modifier in target %s %qs",
> -	       pragma_or_attr, str);
> +	error (is_pragma
> +	       ? G_("invalid value (\"%s\") in %<target()%> pragma")
> +	       : G_("invalid value (\"%s\") in %<target()%> attribute"),
> +	       str);
>  	break;
>  
>        default:
> @@ -9744,11 +9765,11 @@ static const struct aarch64_attribute_info aarch64_attributes[] =
>  
>  /* Parse ARG_STR which contains the definition of one target attribute.
>     Show appropriate errors if any or return true if the attribute is valid.
> -   PRAGMA_OR_ATTR holds the string to use in error messages about whether
> +   IS_PRAGMA holds the boolean to tell error messages about whether
>     we're processing a target attribute or pragma.  */
>  
>  static bool
> -aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
> +aarch64_process_one_target_attr (char *arg_str, bool is_pragma)
>  {
>    bool invert = false;
>  
> @@ -9756,7 +9777,9 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
>  
>    if (len == 0)
>      {
> -      error ("malformed target %s", pragma_or_attr);
> +      error (is_pragma
> +	     ? G_("malformed %<target()%> pragma")
> +	     : G_("malformed %<target()%> attribute"));
>        return false;
>      }
>  
> @@ -9772,7 +9795,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
>       through the machinery for the rest of the target attributes in this
>       function.  */
>    if (*str_to_check == '+')
> -    return aarch64_handle_attr_isa_flags (str_to_check, pragma_or_attr);
> +    return aarch64_handle_attr_isa_flags (str_to_check, is_pragma);
>  
>    if (len > 3 && strncmp (str_to_check, "no-", 3) == 0)
>      {
> @@ -9804,8 +9827,10 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
>  
>        if (attr_need_arg_p ^ (arg != NULL))
>  	{
> -	  error ("target %s %qs does not accept an argument",
> -		  pragma_or_attr, str_to_check);
> +	  error (is_pragma
> +		 ? G_("pragma %<target(\"%s\")%> does not accept an argument")
> +		 : G_("attribute %<target(\"%s\")%> does not accept an argument"),
> +		 str_to_check);
>  	  return false;
>  	}
>  
> @@ -9813,8 +9838,10 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
>  	 then we can't match.  */
>        if (invert && !p_attr->allow_neg)
>  	{
> -	  error ("target %s %qs does not allow a negated form",
> -		  pragma_or_attr, str_to_check);
> +	  error (is_pragma
> +		 ? G_("pragma %<target(\"%s\")%> does not allow a negated form")
> +		 : G_("attribute %<target(\"%s\")%> does not allow a negated form"),
> +		 str_to_check);
>  	  return false;
>  	}
>  
> @@ -9824,7 +9851,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
>  	   For example, cpu=, arch=, tune=.  */
>  	  case aarch64_attr_custom:
>  	    gcc_assert (p_attr->handler);
> -	    if (!p_attr->handler (arg, pragma_or_attr))
> +	    if (!p_attr->handler (arg, is_pragma))
>  	      return false;
>  	    break;
>  
> @@ -9868,8 +9895,10 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
>  		}
>  	      else
>  		{
> -		  error ("target %s %s=%s is not valid",
> -			 pragma_or_attr, str_to_check, arg);
> +		  error (is_pragma
> +			 ? G_("pragma %<target(\"%s=%s\")%> is not valid")
> +			 : G_("attribute %<target(\"%s=%s\")%> is not valid"),
> +			 str_to_check, arg);
>  		}
>  	      break;
>  	    }
> @@ -9903,12 +9932,12 @@ num_occurences_in_str (char c, char *str)
>  }
>  
>  /* Parse the tree in ARGS that contains the target attribute information
> -   and update the global target options space.  PRAGMA_OR_ATTR is a string
> -   to be used in error messages, specifying whether this is processing
> +   and update the global target options space.  IS_PRAGMA is a boolean
> +   to be used by error messages, specifying whether this is processing
>     a target attribute or a target pragma.  */
>  
>  bool
> -aarch64_process_target_attr (tree args, const char* pragma_or_attr)
> +aarch64_process_target_attr (tree args, bool is_pragma)
>  {
>    if (TREE_CODE (args) == TREE_LIST)
>      {
> @@ -9917,7 +9946,7 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
>  	  tree head = TREE_VALUE (args);
>  	  if (head)
>  	    {
> -	      if (!aarch64_process_target_attr (head, pragma_or_attr))
> +	      if (!aarch64_process_target_attr (head, is_pragma))
>  		return false;
>  	    }
>  	  args = TREE_CHAIN (args);
> @@ -9938,7 +9967,9 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
>  
>    if (len == 0)
>      {
> -      error ("malformed target %s value", pragma_or_attr);
> +      error (is_pragma
> +	     ? G_("malformed %<target()%> pragma")
> +	     : G_("malformed %<target()%> attribute"));
>        return false;
>      }
>  
> @@ -9953,9 +9984,12 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
>    while (token)
>      {
>        num_attrs++;
> -      if (!aarch64_process_one_target_attr (token, pragma_or_attr))
> +      if (!aarch64_process_one_target_attr (token, is_pragma))
>  	{
> -	  error ("target %s %qs is invalid", pragma_or_attr, token);
> +	  error (is_pragma
> +		 ? G_("pragma %<target(\"%s\")%> is not valid")
> +		 : G_("attribute %<target(\"%s\")%> is not valid"),
> +		 token);
>  	  return false;
>  	}
>  
> @@ -9964,8 +9998,10 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
>  
>    if (num_attrs != num_commas + 1)
>      {
> -      error ("malformed target %s list %qs",
> -	      pragma_or_attr, TREE_STRING_POINTER (args));
> +      error (is_pragma
> +	? G_("malformed %<target(\"%s\")%> pragma")
> +	: G_("malformed %<target(\"%s\")%> attribute"),
> +	TREE_STRING_POINTER (args));
>        return false;
>      }
>  
> @@ -10025,7 +10061,7 @@ aarch64_option_valid_attribute_p (tree fndecl, tree, tree args, int)
>  			TREE_TARGET_OPTION (target_option_current_node));
>  
>  
> -  ret = aarch64_process_target_attr (args, "attribute");
> +  ret = aarch64_process_target_attr (args, false);
>  
>    /* Set up any additional state.  */
>    if (ret)
> 
> 
> pr79868-tests.patch
> 
> 
> diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
> index ccfe417..33ea312 100644
> --- a/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
> @@ -4,6 +4,6 @@ __attribute__((target ("arch=armv8-a-typo"))) void
>  foo ()
>  {
>    /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'armv8-a'?"  "" { target *-*-* } .-1 } */
> -  /* { dg-error "unknown value 'armv8-a-typo' for 'arch' target attribute"  "" { target *-*-* } .-2 } */
> -  /* { dg-error "target attribute 'arch=armv8-a-typo' is invalid"  "" { target *-*-* } .-3 } */
> +  /* { dg-error "invalid name \\(\"armv8-a-typo\"\\) in 'target\\(\"arch=\"\\)' attribute"  "" { target *-*-* } .-2 } */
> +  /* { dg-error "attribute 'target\\(\"arch=armv8-a-typo\"\\)' is not valid"  "" { target *-*-* } .-3 } */
>  }
> diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
> index 42ba51a..533b949 100644
> --- a/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
> @@ -3,7 +3,7 @@
>  __attribute__((target ("cpu=cortex-a57-typo"))) void
>  foo ()
>  {
> -  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57?"  "" { target *-*-* } .-1 } */
> -  /* { dg-error "unknown value 'cortex-a57-typo' for 'cpu' target attribute"  "" { target *-*-* } .-2 } */
> -  /* { dg-error "target attribute 'cpu=cortex-a57-typo' is invalid"  "" { target *-*-* } .-3 } */
> +  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57'?"  "" { target *-*-* } .-1 } */
> +  /* { dg-error "invalid name \\(\"cortex-a57-typo\"\\) in 'target\\(\"cpu=\"\\)' attribute"  "" { target *-*-* } .-2 } */
> +  /* { dg-error "attribute 'target\\(\"cpu=cortex-a57-typo\"\\)' is not valid"  "" { target *-*-* } .-3 } */
>  }
> diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
> index 03d2bbf..017f110 100644
> --- a/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
> @@ -3,7 +3,7 @@
>  __attribute__((target ("tune=cortex-a57-typo"))) void
>  foo ()
>  {
> -  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57?"  "" { target *-*-* } .-1 } */
> -  /* { dg-error "unknown value 'cortex-a57-typo' for 'tune' target attribute"  "" { target *-*-* } .-2 } */
> -  /* { dg-error "target attribute 'tune=cortex-a57-typo' is invalid"  "" { target *-*-* } .-3 } */
> +  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57'?"  "" { target *-*-* } .-1 } */
> +  /* { dg-error "invalid name \\(\"cortex-a57-typo\"\\) in 'target\\(\"tune=\"\\)' attribute"  "" { target *-*-* } .-2 } */
> +  /* { dg-error "attribute 'target\\(\"tune=cortex-a57-typo\"\\)' is not valid"  "" { target *-*-* } .-3 } */
>  }
> diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_11.c b/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
> index 7cfb826..a3df438 100644
> --- a/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
> +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
> @@ -10,4 +10,4 @@ foo (int a)
>  }
>  
>  /* { dg-error "does not allow a negated form" "" { target *-*-* } 0 } */
> -/* { dg-error "is invalid" "" { target *-*-* } 0 } */
> +/* { dg-error "is not valid" "" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_12.c b/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
> index 39cb996..8a3a25b 100644
> --- a/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
> +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
> @@ -10,4 +10,4 @@ foo (int a)
>  }
>  
>  /* { dg-error "does not accept an argument" "" { target *-*-* } 0 } */
> -/* { dg-error "is invalid" "" { target *-*-* } 0 } */
> +/* { dg-error "is not valid" "" { target *-*-* } 0 } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_17.c b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
> index 483cc6d..2a7a751 100644
> --- a/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
> +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
> @@ -5,4 +5,4 @@ foo (int a)
>    return a + 5;
>  }
>  
> -/* { dg-error "target attribute.*is invalid" "" { target *-*-* } 0 } */
> \ No newline at end of file
> +/* { dg-error "attribute 'target\\(\"invalid-attr-string\"\\)' is not valid" "" { target *-*-* } 0 } */
> 

Re: [PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Steve Ellcey]

[-- Attachment #1: Type: text/plain, Size: 1659 bytes --]

On Thu, 2017-10-26 at 13:56 +0100, Richard Earnshaw (lists) wrote:
> 
> I can't help feeling that all this logic is somewhat excessive and
> changing the wording of each message to include "pragma or attribute"
> would solve it equally well.  With the new context highlighting it's
> trivial to tell which subcase of usage is being referred to.
> 
> R.

I have no problem with that.  Here is a version that uses "pragma or
attribute".

Tested on ToT with no regressions.  Ok to checkin?

Steve Ellcey
sellcey@cavium.com



ChangeLog:

2017-10-30  Steve Ellcey  <sellcey@cavium.com>

	PR target/79868
	* config/aarch64/aarch64-c.c (aarch64_pragma_target_parse):
	Remove second argument from aarch64_process_target_attr call.
	* config/aarch64/aarch64-protos.h (aarch64_process_target_attr):
	Ditto.
	* config/aarch64/aarch64.c (aarch64_attribute_info): Change
	field type.
	(aarch64_handle_attr_arch): Remove second argument.
	(aarch64_handle_attr_cpu): Ditto.
	(aarch64_handle_attr_tune): Ditto.
	(aarch64_handle_attr_isa_flags): Ditto.
	(aarch64_process_one_target_attr): Ditto.
	(aarch64_process_target_attr): Ditto.
	(aarch64_option_valid_attribute_p): Remove second argument.
	on aarch64_process_target_attr call.


Testsuite ChangeLog:

2017-10-30  Steve Ellcey  <sellcey@cavium.com>

	PR target/79868
	* gcc.target/aarch64/spellcheck_1.c: Update dg-error string to match
	new format.
	* gcc.target/aarch64/spellcheck_2.c: Ditto.
	* gcc.target/aarch64/spellcheck_3.c: Ditto.
	* gcc.target/aarch64/target_attr_11.c: Ditto.
	* gcc.target/aarch64/target_attr_12.c: Ditto.
	* gcc.target/aarch64/target_attr_17.c: Ditto.

[-- Attachment #2: pr79868.patch --]
[-- Type: text/x-patch, Size: 11902 bytes --]

diff --git a/gcc/config/aarch64/aarch64-c.c b/gcc/config/aarch64/aarch64-c.c
index c7d866f..e18ec4a 100644
--- a/gcc/config/aarch64/aarch64-c.c
+++ b/gcc/config/aarch64/aarch64-c.c
@@ -166,7 +166,7 @@ aarch64_pragma_target_parse (tree args, tree pop_target)
      information that it specifies.  */
   if (args)
     {
-      if (!aarch64_process_target_attr (args, "pragma"))
+      if (!aarch64_process_target_attr (args))
 	return false;
 
       aarch64_override_options_internal (&global_options);
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 5d7c5df..345bfe8 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -457,7 +457,7 @@ bool aarch64_gen_adjusted_ldpstp (rtx *, bool, scalar_mode, RTX_CODE);
 
 void aarch64_init_builtins (void);
 
-bool aarch64_process_target_attr (tree, const char*);
+bool aarch64_process_target_attr (tree);
 void aarch64_override_options_internal (struct gcc_options *);
 
 rtx aarch64_expand_builtin (tree exp,
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1cc1043..fa2f86e 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9554,9 +9554,8 @@ enum aarch64_attr_opt_type
    ATTR_TYPE specifies the type of behavior of the attribute as described
    in the definition of enum aarch64_attr_opt_type.
    ALLOW_NEG is true if the attribute supports a "no-" form.
-   HANDLER is the function that takes the attribute string and whether
-   it is a pragma or attribute and handles the option.  It is needed only
-   when the ATTR_TYPE is aarch64_attr_custom.
+   HANDLER is the function that takes the attribute string as an argument
+   It is needed only when the ATTR_TYPE is aarch64_attr_custom.
    OPT_NUM is the enum specifying the option that the attribute modifies.
    This is needed for attributes that mirror the behavior of a command-line
    option, that is it has ATTR_TYPE aarch64_attr_mask, aarch64_attr_bool or
@@ -9567,15 +9566,14 @@ struct aarch64_attribute_info
   const char *name;
   enum aarch64_attr_opt_type attr_type;
   bool allow_neg;
-  bool (*handler) (const char *, const char *);
+  bool (*handler) (const char *);
   enum opt_code opt_num;
 };
 
-/* Handle the ARCH_STR argument to the arch= target attribute.
-   PRAGMA_OR_ATTR is used in potential error messages.  */
+/* Handle the ARCH_STR argument to the arch= target attribute.  */
 
 static bool
-aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
+aarch64_handle_attr_arch (const char *str)
 {
   const struct processor *tmp_arch = NULL;
   enum aarch64_parse_opt_result parse_res
@@ -9592,15 +9590,14 @@ aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
   switch (parse_res)
     {
       case AARCH64_PARSE_MISSING_ARG:
-	error ("missing architecture name in 'arch' target %s", pragma_or_attr);
+	error ("missing name in %<target(\"arch=\")%> pragma or attribute");
 	break;
       case AARCH64_PARSE_INVALID_ARG:
-	error ("unknown value %qs for 'arch' target %s", str, pragma_or_attr);
+	error ("invalid name (\"%s\") in %<target(\"arch=\")%> pragma or attribute", str);
 	aarch64_print_hint_for_arch (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
-	error ("invalid feature modifier %qs for 'arch' target %s",
-	       str, pragma_or_attr);
+	error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
 	break;
       default:
 	gcc_unreachable ();
@@ -9609,11 +9606,10 @@ aarch64_handle_attr_arch (const char *str, const char *pragma_or_attr)
   return false;
 }
 
-/* Handle the argument CPU_STR to the cpu= target attribute.
-   PRAGMA_OR_ATTR is used in potential error messages.  */
+/* Handle the argument CPU_STR to the cpu= target attribute.  */
 
 static bool
-aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
+aarch64_handle_attr_cpu (const char *str)
 {
   const struct processor *tmp_cpu = NULL;
   enum aarch64_parse_opt_result parse_res
@@ -9633,15 +9629,14 @@ aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
   switch (parse_res)
     {
       case AARCH64_PARSE_MISSING_ARG:
-	error ("missing cpu name in 'cpu' target %s", pragma_or_attr);
+	error ("missing name in %<target(\"cpu=\")%> pragma or attribute");
 	break;
       case AARCH64_PARSE_INVALID_ARG:
-	error ("unknown value %qs for 'cpu' target %s", str, pragma_or_attr);
+	error ("invalid name (\"%s\") in %<target(\"cpu=\")%> pragma or attribute", str);
 	aarch64_print_hint_for_core (str);
 	break;
       case AARCH64_PARSE_INVALID_FEATURE:
-	error ("invalid feature modifier %qs for 'cpu' target %s",
-	       str, pragma_or_attr);
+	error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
 	break;
       default:
 	gcc_unreachable ();
@@ -9650,11 +9645,10 @@ aarch64_handle_attr_cpu (const char *str, const char *pragma_or_attr)
   return false;
 }
 
-/* Handle the argument STR to the tune= target attribute.
-   PRAGMA_OR_ATTR is used in potential error messages.  */
+/* Handle the argument STR to the tune= target attribute.  */
 
 static bool
-aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
+aarch64_handle_attr_tune (const char *str)
 {
   const struct processor *tmp_tune = NULL;
   enum aarch64_parse_opt_result parse_res
@@ -9671,7 +9665,7 @@ aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
   switch (parse_res)
     {
       case AARCH64_PARSE_INVALID_ARG:
-	error ("unknown value %qs for 'tune' target %s", str, pragma_or_attr);
+	error ("invalid name (\"%s\") in %<target(\"tune=\")%> pragma or attribute", str);
 	aarch64_print_hint_for_core (str);
 	break;
       default:
@@ -9684,11 +9678,10 @@ aarch64_handle_attr_tune (const char *str, const char *pragma_or_attr)
 /* Parse an architecture extensions target attribute string specified in STR.
    For example "+fp+nosimd".  Show any errors if needed.  Return TRUE
    if successful.  Update aarch64_isa_flags to reflect the ISA features
-   modified.
-   PRAGMA_OR_ATTR is used in potential error messages.  */
+   modified.  */
 
 static bool
-aarch64_handle_attr_isa_flags (char *str, const char *pragma_or_attr)
+aarch64_handle_attr_isa_flags (char *str)
 {
   enum aarch64_parse_opt_result parse_res;
   unsigned long isa_flags = aarch64_isa_flags;
@@ -9712,13 +9705,11 @@ aarch64_handle_attr_isa_flags (char *str, const char *pragma_or_attr)
   switch (parse_res)
     {
       case AARCH64_PARSE_MISSING_ARG:
-	error ("missing feature modifier in target %s %qs",
-	       pragma_or_attr, str);
+	error ("missing value in %<target()%> pragma or attribute");
 	break;
 
       case AARCH64_PARSE_INVALID_FEATURE:
-	error ("invalid feature modifier in target %s %qs",
-	       pragma_or_attr, str);
+	error ("invalid value (\"%s\") in %<target()%> pragma or attribute", str);
 	break;
 
       default:
@@ -9756,12 +9747,10 @@ static const struct aarch64_attribute_info aarch64_attributes[] =
 };
 
 /* Parse ARG_STR which contains the definition of one target attribute.
-   Show appropriate errors if any or return true if the attribute is valid.
-   PRAGMA_OR_ATTR holds the string to use in error messages about whether
-   we're processing a target attribute or pragma.  */
+   Show appropriate errors if any or return true if the attribute is valid.  */
 
 static bool
-aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
+aarch64_process_one_target_attr (char *arg_str)
 {
   bool invert = false;
 
@@ -9769,7 +9758,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 
   if (len == 0)
     {
-      error ("malformed target %s", pragma_or_attr);
+      error ("malformed %<target()%> pragma or attribute");
       return false;
     }
 
@@ -9785,7 +9774,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
      through the machinery for the rest of the target attributes in this
      function.  */
   if (*str_to_check == '+')
-    return aarch64_handle_attr_isa_flags (str_to_check, pragma_or_attr);
+    return aarch64_handle_attr_isa_flags (str_to_check);
 
   if (len > 3 && strncmp (str_to_check, "no-", 3) == 0)
     {
@@ -9817,8 +9806,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 
       if (attr_need_arg_p ^ (arg != NULL))
 	{
-	  error ("target %s %qs does not accept an argument",
-		  pragma_or_attr, str_to_check);
+	  error ("pragma or attribute %<target(\"%s\")%> does not accept an argument", str_to_check);
 	  return false;
 	}
 
@@ -9826,8 +9814,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 	 then we can't match.  */
       if (invert && !p_attr->allow_neg)
 	{
-	  error ("target %s %qs does not allow a negated form",
-		  pragma_or_attr, str_to_check);
+	  error ("pragma or attribute %<target(\"%s\")%> does not allow a negated form", str_to_check);
 	  return false;
 	}
 
@@ -9837,7 +9824,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 	   For example, cpu=, arch=, tune=.  */
 	  case aarch64_attr_custom:
 	    gcc_assert (p_attr->handler);
-	    if (!p_attr->handler (arg, pragma_or_attr))
+	    if (!p_attr->handler (arg))
 	      return false;
 	    break;
 
@@ -9881,8 +9868,7 @@ aarch64_process_one_target_attr (char *arg_str, const char* pragma_or_attr)
 		}
 	      else
 		{
-		  error ("target %s %s=%s is not valid",
-			 pragma_or_attr, str_to_check, arg);
+		  error ("pragma or attribute %<target(\"%s=%s\")%> is not valid", str_to_check, arg);
 		}
 	      break;
 	    }
@@ -9916,12 +9902,10 @@ num_occurences_in_str (char c, char *str)
 }
 
 /* Parse the tree in ARGS that contains the target attribute information
-   and update the global target options space.  PRAGMA_OR_ATTR is a string
-   to be used in error messages, specifying whether this is processing
-   a target attribute or a target pragma.  */
+   and update the global target options space.  */
 
 bool
-aarch64_process_target_attr (tree args, const char* pragma_or_attr)
+aarch64_process_target_attr (tree args)
 {
   if (TREE_CODE (args) == TREE_LIST)
     {
@@ -9930,7 +9914,7 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
 	  tree head = TREE_VALUE (args);
 	  if (head)
 	    {
-	      if (!aarch64_process_target_attr (head, pragma_or_attr))
+	      if (!aarch64_process_target_attr (head))
 		return false;
 	    }
 	  args = TREE_CHAIN (args);
@@ -9951,7 +9935,7 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
 
   if (len == 0)
     {
-      error ("malformed target %s value", pragma_or_attr);
+      error ("malformed %<target()%> pragma or attribute");
       return false;
     }
 
@@ -9966,9 +9950,9 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
   while (token)
     {
       num_attrs++;
-      if (!aarch64_process_one_target_attr (token, pragma_or_attr))
+      if (!aarch64_process_one_target_attr (token))
 	{
-	  error ("target %s %qs is invalid", pragma_or_attr, token);
+	  error ("pragma or attribute %<target(\"%s\")%> is not valid", token);
 	  return false;
 	}
 
@@ -9977,8 +9961,7 @@ aarch64_process_target_attr (tree args, const char* pragma_or_attr)
 
   if (num_attrs != num_commas + 1)
     {
-      error ("malformed target %s list %qs",
-	      pragma_or_attr, TREE_STRING_POINTER (args));
+      error ("malformed %<target(\"%s\")%> pragma or attribute", TREE_STRING_POINTER (args));
       return false;
     }
 
@@ -10037,8 +10020,7 @@ aarch64_option_valid_attribute_p (tree fndecl, tree, tree args, int)
     cl_target_option_restore (&global_options,
 			TREE_TARGET_OPTION (target_option_current_node));
 
-
-  ret = aarch64_process_target_attr (args, "attribute");
+  ret = aarch64_process_target_attr (args);
 
   /* Set up any additional state.  */
   if (ret)

[-- Attachment #3: pr79868-tests.patch --]
[-- Type: text/x-patch, Size: 4388 bytes --]

diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
index ccfe417..f57e0c5 100644
--- a/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_1.c
@@ -4,6 +4,6 @@ __attribute__((target ("arch=armv8-a-typo"))) void
 foo ()
 {
   /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'armv8-a'?"  "" { target *-*-* } .-1 } */
-  /* { dg-error "unknown value 'armv8-a-typo' for 'arch' target attribute"  "" { target *-*-* } .-2 } */
-  /* { dg-error "target attribute 'arch=armv8-a-typo' is invalid"  "" { target *-*-* } .-3 } */
+  /* { dg-error "invalid name \\(\"armv8-a-typo\"\\) in 'target\\(\"arch=\"\\)' pragma or attribute"  "" { target *-*-* } .-2 } */
+  /* { dg-error "pragma or attribute 'target\\(\"arch=armv8-a-typo\"\\)' is not valid"  "" { target *-*-* } .-3 } */
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
index 42ba51a..70096f8 100644
--- a/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_2.c
@@ -3,7 +3,7 @@
 __attribute__((target ("cpu=cortex-a57-typo"))) void
 foo ()
 {
-  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57?"  "" { target *-*-* } .-1 } */
-  /* { dg-error "unknown value 'cortex-a57-typo' for 'cpu' target attribute"  "" { target *-*-* } .-2 } */
-  /* { dg-error "target attribute 'cpu=cortex-a57-typo' is invalid"  "" { target *-*-* } .-3 } */
+  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57'?"  "" { target *-*-* } .-1 } */
+  /* { dg-error "invalid name \\(\"cortex-a57-typo\"\\) in 'target\\(\"cpu=\"\\)' pragma or attribute"  "" { target *-*-* } .-2 } */
+  /* { dg-error "pragma or attribute 'target\\(\"cpu=cortex-a57-typo\"\\)' is not valid"  "" { target *-*-* } .-3 } */
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
index 03d2bbf..20dff2b 100644
--- a/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_3.c
@@ -3,7 +3,7 @@
 __attribute__((target ("tune=cortex-a57-typo"))) void
 foo ()
 {
-  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57?"  "" { target *-*-* } .-1 } */
-  /* { dg-error "unknown value 'cortex-a57-typo' for 'tune' target attribute"  "" { target *-*-* } .-2 } */
-  /* { dg-error "target attribute 'tune=cortex-a57-typo' is invalid"  "" { target *-*-* } .-3 } */
+  /* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57'?"  "" { target *-*-* } .-1 } */
+  /* { dg-error "invalid name \\(\"cortex-a57-typo\"\\) in 'target\\(\"tune=\"\\)' pragma or attribute"  "" { target *-*-* } .-2 } */
+  /* { dg-error "pragma or attribute 'target\\(\"tune=cortex-a57-typo\"\\)' is not valid"  "" { target *-*-* } .-3 } */
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_11.c b/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
index 7cfb826..a3df438 100644
--- a/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
+++ b/gcc/testsuite/gcc.target/aarch64/target_attr_11.c
@@ -10,4 +10,4 @@ foo (int a)
 }
 
 /* { dg-error "does not allow a negated form" "" { target *-*-* } 0 } */
-/* { dg-error "is invalid" "" { target *-*-* } 0 } */
+/* { dg-error "is not valid" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_12.c b/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
index 39cb996..8a3a25b 100644
--- a/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
+++ b/gcc/testsuite/gcc.target/aarch64/target_attr_12.c
@@ -10,4 +10,4 @@ foo (int a)
 }
 
 /* { dg-error "does not accept an argument" "" { target *-*-* } 0 } */
-/* { dg-error "is invalid" "" { target *-*-* } 0 } */
+/* { dg-error "is not valid" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_17.c b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
index 483cc6d..2a7a751 100644
--- a/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
+++ b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c
@@ -5,4 +5,4 @@ foo (int a)
   return a + 5;
 }
 
-/* { dg-error "target attribute.*is invalid" "" { target *-*-* } 0 } */
\ No newline at end of file
+/* { dg-error "attribute 'target\\(\"invalid-attr-string\"\\)' is not valid" "" { target *-*-* } 0 } */

Re: [PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Richard Earnshaw (lists)]

On 30/10/17 20:50, Steve Ellcey wrote:
> On Thu, 2017-10-26 at 13:56 +0100, Richard Earnshaw (lists) wrote:
>>  
>> I can't help feeling that all this logic is somewhat excessive and
>> changing the wording of each message to include "pragma or attribute"
>> would solve it equally well.  With the new context highlighting it's
>> trivial to tell which subcase of usage is being referred to.
>>
>> R.
> 
> I have no problem with that.  Here is a version that uses "pragma or
> attribute".
> 
> Tested on ToT with no regressions.  Ok to checkin?
> 
> Steve Ellcey
> sellcey@cavium.com
> 
> 
> 
> ChangeLog:
> 
> 2017-10-30  Steve Ellcey  <sellcey@cavium.com>
> 
> 	PR target/79868
> 	* config/aarch64/aarch64-c.c (aarch64_pragma_target_parse):
> 	Remove second argument from aarch64_process_target_attr call.
> 	* config/aarch64/aarch64-protos.h (aarch64_process_target_attr):
> 	Ditto.
> 	* config/aarch64/aarch64.c (aarch64_attribute_info): Change
> 	field type.
> 	(aarch64_handle_attr_arch): Remove second argument.
> 	(aarch64_handle_attr_cpu): Ditto.
> 	(aarch64_handle_attr_tune): Ditto.
> 	(aarch64_handle_attr_isa_flags): Ditto.
> 	(aarch64_process_one_target_attr): Ditto.
> 	(aarch64_process_target_attr): Ditto.
> 	(aarch64_option_valid_attribute_p): Remove second argument.
> 	on aarch64_process_target_attr call.
> 
> 
> Testsuite ChangeLog:
> 
> 2017-10-30  Steve Ellcey  <sellcey@cavium.com>
> 
> 	PR target/79868
> 	* gcc.target/aarch64/spellcheck_1.c: Update dg-error string to match
> 	new format.
> 	* gcc.target/aarch64/spellcheck_2.c: Ditto.
> 	* gcc.target/aarch64/spellcheck_3.c: Ditto.
> 	* gcc.target/aarch64/target_attr_11.c: Ditto.
> 	* gcc.target/aarch64/target_attr_12.c: Ditto.
> 	* gcc.target/aarch64/target_attr_17.c: Ditto.
> 

This is looking better...

I may have missed some discussion on this topic, but what's the
reasoning behind changing the quoting around the 'str' parameter value in

-	error ("unknown value %qs for 'cpu' target %s", str, pragma_or_attr);
+	error ("invalid name (\"%s\") in %<target(\"cpu=\")%> pragma or
attribute", str);

And also with the new generic message does the %<target(\"cpu=\")%>
still make sense?  My feeling is that the original text here is perhaps
more appropriate.  Similarly for other messages.

R.

Re: [PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Steve Ellcey]

On Tue, 2017-10-31 at 09:57 +0000, Richard Earnshaw (lists) wrote:
> 
> This is looking better...
> 
> I may have missed some discussion on this topic, but what's the
> reasoning behind changing the quoting around the 'str' parameter
> value in
> 
> -	error ("unknown value %qs for 'cpu' target %s", str,
> pragma_or_attr);
> +	error ("invalid name (\"%s\") in %<target(\"cpu=\")%> pragma
> or
> attribute", str);
> 
> And also with the new generic message does the %<target(\"cpu=\")%>
> still make sense?  My feeling is that the original text here is perhaps
> more appropriate.  Similarly for other messages.
> 
> R.


%qs uses single quotes vs. double quotes, changing that was suggested
by Martin Sebor in this comment:

https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01569.html

using '%<target(\"cpu=\")%>' was also suggested by Martin in that
same thread at:

https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01277.html
https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01469.html

as being more consistent with other usage (mainly config/i386/i386.c).

Steve Ellcey
sellcey@cavium.com

Re: [PATCH PR79868 ][aarch64] Fix error calls in aarch64 code so they can be translated (version 2)

[Richard Earnshaw (lists)]

On 31/10/17 16:53, Steve Ellcey wrote:
> On Tue, 2017-10-31 at 09:57 +0000, Richard Earnshaw (lists) wrote:
>>  
>> This is looking better...
>>
>> I may have missed some discussion on this topic, but what's the
>> reasoning behind changing the quoting around the 'str' parameter
>> value in
>>
>> -	error ("unknown value %qs for 'cpu' target %s", str,
>> pragma_or_attr);
>> +	error ("invalid name (\"%s\") in %<target(\"cpu=\")%> pragma
>> or
>> attribute", str);
>>
>> And also with the new generic message does the %<target(\"cpu=\")%>
>> still make sense?  My feeling is that the original text here is perhaps
>> more appropriate.  Similarly for other messages.
>>
>> R.
> 
> 
> %qs uses single quotes vs. double quotes, changing that was suggested
> by Martin Sebor in this comment:
> 
> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01569.html
> 
> using '%<target(\"cpu=\")%>' was also suggested by Martin in that
> same thread at:
> 
> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01277.html
> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01469.html
> 
> as being more consistent with other usage (mainly config/i386/i386.c).
> 
> Steve Ellcey
> sellcey@cavium.com
> 

Thanks.

On that basis, this patch is OK.

R.