[PATCH][stage1] Enhance target and target_clone error messages.

[PATCH][stage1] Enhance target and target_clone error messages.

[Martin Liška]

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

Hi.

The patch distinguishes among target and target_clone attribute when
reporting for an error. I've also reworded the affected error messages
a bit.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed after stage1 opens?
Thanks,
Martin

---
 gcc/cgraphclones.c                         |  2 +-
 gcc/config/i386/i386-c.c                   |  5 +-
 gcc/config/i386/i386-protos.h              |  4 +-
 gcc/config/i386/i386.c                     | 54 +++++++++++++---------
 gcc/testsuite/gcc.target/i386/funcspec-4.c |  2 +-
 5 files changed, 39 insertions(+), 28 deletions(-)



[-- Attachment #2: 0001-Enhance-target-and-target_clone-error-messages.patch --]
[-- Type: text/x-patch, Size: 9176 bytes --]

diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index 15f7e119d18..fd867ecac91 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -1056,7 +1056,7 @@ cgraph_node::create_version_clone_with_body
       location_t saved_loc = input_location;
       tree v = TREE_VALUE (target_attributes);
       input_location = DECL_SOURCE_LOCATION (new_decl);
-      bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 0);
+      bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 1);
       input_location = saved_loc;
       if (!r)
 	return NULL;
diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c
index 5e7e46fcebe..50cac3b1a9f 100644
--- a/gcc/config/i386/i386-c.c
+++ b/gcc/config/i386/i386-c.c
@@ -586,8 +586,9 @@ ix86_pragma_target_parse (tree args, tree pop_target)
     }
   else
     {
-      cur_tree = ix86_valid_target_attribute_tree (args, &global_options,
-						   &global_options_set);
+      cur_tree = ix86_valid_target_attribute_tree (NULL_TREE, args,
+						   &global_options,
+						   &global_options_set, 0);
       if (!cur_tree || cur_tree == error_mark_node)
        {
          cl_target_option_restore (&global_options,
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 83645e89a81..cfc4783205e 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -215,9 +215,9 @@ extern unsigned int ix86_minimum_alignment (tree, machine_mode,
 extern tree ix86_handle_shared_attribute (tree *, tree, tree, int, bool *);
 extern tree ix86_handle_selectany_attribute (tree *, tree, tree, int, bool *);
 extern int x86_field_alignment (tree, int);
-extern tree ix86_valid_target_attribute_tree (tree,
+extern tree ix86_valid_target_attribute_tree (tree, tree,
 					      struct gcc_options *,
-					      struct gcc_options *);
+					      struct gcc_options *, bool);
 extern unsigned int ix86_get_callcvt (const_tree);
 
 #endif
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ed9a9c2e3f7..aa8adc7b2aa 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -847,10 +847,11 @@ static void ix86_function_specific_post_stream_in (struct cl_target_option *);
 static void ix86_function_specific_print (FILE *, int,
 					  struct cl_target_option *);
 static bool ix86_valid_target_attribute_p (tree, tree, tree, int);
-static bool ix86_valid_target_attribute_inner_p (tree, char *[],
+static bool ix86_valid_target_attribute_inner_p (tree, tree, char *[],
 						 struct gcc_options *,
 						 struct gcc_options *,
-						 struct gcc_options *);
+						 struct gcc_options *,
+						 bool);
 static bool ix86_can_inline_p (tree, tree);
 static void ix86_set_current_function (tree);
 static unsigned int ix86_minimum_incoming_stack_boundary (bool);
@@ -5149,10 +5150,11 @@ ix86_function_specific_print (FILE *file, int indent,
    over the list.  */
 
 static bool
-ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
+ix86_valid_target_attribute_inner_p (tree fndecl, tree args, char *p_strings[],
 				     struct gcc_options *opts,
 				     struct gcc_options *opts_set,
-				     struct gcc_options *enum_opts_set)
+				     struct gcc_options *enum_opts_set,
+				     bool target_clone_attr)
 {
   char *next_optstr;
   bool ret = true;
@@ -5296,9 +5298,12 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
     IX86_ATTR_YES ("recip",
 		   OPT_mrecip,
 		   MASK_RECIP),
-
   };
 
+  location_t loc
+    = fndecl == NULL ? UNKNOWN_LOCATION : DECL_SOURCE_LOCATION (fndecl);
+  const char *attr_name = target_clone_attr ? "target_clone" : "target";
+
   /* If this is a list, recurse to get the options.  */
   if (TREE_CODE (args) == TREE_LIST)
     {
@@ -5306,9 +5311,10 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 
       for (; args; args = TREE_CHAIN (args))
 	if (TREE_VALUE (args)
-	    && !ix86_valid_target_attribute_inner_p (TREE_VALUE (args),
+	    && !ix86_valid_target_attribute_inner_p (fndecl, TREE_VALUE (args),
 						     p_strings, opts, opts_set,
-						     enum_opts_set))
+						     enum_opts_set,
+						     target_clone_attr))
 	  ret = false;
 
       return ret;
@@ -5316,7 +5322,7 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 
   else if (TREE_CODE (args) != STRING_CST)
     {
-      error ("attribute %<target%> argument not a string");
+      error_at (loc, "attribute %qs argument not a string", attr_name);
       return false;
     }
 
@@ -5328,7 +5334,6 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
       char *p = next_optstr;
       char *orig_p = p;
       char *comma = strchr (next_optstr, ',');
-      const char *opt_string;
       size_t len, opt_len;
       int opt;
       bool opt_set_p;
@@ -5374,7 +5379,6 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 	    {
 	      opt = attrs[i].opt;
 	      mask = attrs[i].mask;
-	      opt_string = attrs[i].string;
 	      break;
 	    }
 	}
@@ -5382,7 +5386,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
       /* Process the option.  */
       if (opt == N_OPTS)
 	{
-	  error ("attribute(target(\"%s\")) is unknown", orig_p);
+	  error_at (loc, "attribute value %qs is unknown in %qs attribute",
+		    orig_p, attr_name);
 	  ret = false;
 	}
 
@@ -5410,7 +5415,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 	{
 	  if (p_strings[opt])
 	    {
-	      error ("option(\"%s\") was already specified", opt_string);
+	      error_at (loc, "attribute value %qs was already specified "
+			"in %qs attribute", orig_p, attr_name);
 	      ret = false;
 	    }
 	  else
@@ -5429,7 +5435,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 			global_dc);
 	  else
 	    {
-	      error ("attribute(target(\"%s\")) is unknown", orig_p);
+	      error_at (loc, "attribute value %qs is unknown in %qs attribute",
+			orig_p, attr_name);
 	      ret = false;
 	    }
 	}
@@ -5453,9 +5460,10 @@ release_options_strings (char **option_strings)
 /* Return a TARGET_OPTION_NODE tree of the target options listed or NULL.  */
 
 tree
-ix86_valid_target_attribute_tree (tree args,
+ix86_valid_target_attribute_tree (tree fndecl, tree args,
 				  struct gcc_options *opts,
-				  struct gcc_options *opts_set)
+				  struct gcc_options *opts_set,
+				  bool target_clone_attr)
 {
   const char *orig_arch_string = opts->x_ix86_arch_string;
   const char *orig_tune_string = opts->x_ix86_tune_string;
@@ -5471,8 +5479,9 @@ ix86_valid_target_attribute_tree (tree args,
   memset (&enum_opts_set, 0, sizeof (enum_opts_set));
 
   /* Process each of the options on the chain.  */
-  if (! ix86_valid_target_attribute_inner_p (args, option_strings, opts,
-					     opts_set, &enum_opts_set))
+  if (!ix86_valid_target_attribute_inner_p (fndecl, args, option_strings, opts,
+					    opts_set, &enum_opts_set,
+					    target_clone_attr))
     return error_mark_node;
 
   /* If the changed options are different from the default, rerun
@@ -5545,7 +5554,7 @@ static bool
 ix86_valid_target_attribute_p (tree fndecl,
 			       tree ARG_UNUSED (name),
 			       tree args,
-			       int ARG_UNUSED (flags))
+			       int flags)
 {
   struct gcc_options func_options;
   tree new_target, new_optimize;
@@ -5580,8 +5589,9 @@ ix86_valid_target_attribute_p (tree fndecl,
   cl_target_option_restore (&func_options,
 			    TREE_TARGET_OPTION (target_option_default_node));
 
-  new_target = ix86_valid_target_attribute_tree (args, &func_options,
-						 &global_options_set);
+  /* FLAGS == 1 is used for target_clones attribute.  */
+  new_target = ix86_valid_target_attribute_tree (fndecl, args, &func_options,
+						 &global_options_set, flags == 1);
 
   new_optimize = build_optimization_node (&func_options);
 
@@ -31967,8 +31977,8 @@ get_builtin_code_for_version (tree decl, tree *predicate_list)
   if (strstr (attrs_str, "arch=") != NULL)
     {
       cl_target_option_save (&cur_target, &global_options);
-      target_node = ix86_valid_target_attribute_tree (attrs, &global_options,
-						      &global_options_set);
+      target_node = ix86_valid_target_attribute_tree (decl, attrs, &global_options,
+						      &global_options_set, 0);
     
       gcc_assert (target_node);
       if (target_node == error_mark_node)
diff --git a/gcc/testsuite/gcc.target/i386/funcspec-4.c b/gcc/testsuite/gcc.target/i386/funcspec-4.c
index 025b97dff8e..e345acdef17 100644
--- a/gcc/testsuite/gcc.target/i386/funcspec-4.c
+++ b/gcc/testsuite/gcc.target/i386/funcspec-4.c
@@ -5,7 +5,7 @@
 extern void error1 (void) __attribute__((__target__("fma400"))); /* { dg-error "unknown" } */
 
 /* Multiple arch switches */
-extern void error2 (void) __attribute__((__target__("arch=core2,arch=k8"))); /* { dg-error "already specified" } */
+extern void error2 (void) __attribute__((__target__("arch=core2,arch=k8"))); /* { dg-error "attribute value 'arch=k8' was already specified in 'target' attribute" } */
 
 /* Unknown tune target */
 extern void error3 (void) __attribute__((__target__("tune=foobar"))); /* { dg-error "bad value" } */

Re: [PATCH][stage1] Enhance target and target_clone error messages.

[Martin Liška]

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

On 4/18/19 1:09 PM, Martin Liška wrote:
> Hi.
> 
> The patch distinguishes among target and target_clone attribute when
> reporting for an error. I've also reworded the affected error messages
> a bit.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed after stage1 opens?
> Thanks,
> Martin
> 
> ---
>  gcc/cgraphclones.c                         |  2 +-
>  gcc/config/i386/i386-c.c                   |  5 +-
>  gcc/config/i386/i386-protos.h              |  4 +-
>  gcc/config/i386/i386.c                     | 54 +++++++++++++---------
>  gcc/testsuite/gcc.target/i386/funcspec-4.c |  2 +-
>  5 files changed, 39 insertions(+), 28 deletions(-)
> 
> 

I forgot to attach ChangeLog.

Martin

[-- Attachment #2: 0001-Enhance-target-and-target_clone-error-messages.patch --]
[-- Type: text/x-patch, Size: 10502 bytes --]

From 8330ad8d25463ea2f596a617f3e133d2050b9ac8 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 17 Apr 2019 13:59:14 +0200
Subject: [PATCH] Enhance target and target_clone error messages.

gcc/ChangeLog:

2019-04-18  Martin Liska  <mliska@suse.cz>

	* cgraphclones.c: Call valid_attribute_p with 1 for
	target_clone.
	* config/i386/i386-c.c (ix86_pragma_target_parse): Use 0 as
	it's for target attribute.
	* config/i386/i386-protos.h (ix86_valid_target_attribute_tree):
	Add new boolean argument.
	* config/i386/i386.c (ix86_valid_target_attribute_inner_p):
	Likewise.
	(ix86_valid_target_attribute_tree): Pass target_clone_attr
	to ix86_valid_target_attribute_inner_p.
	(ix86_valid_target_attribute_p): Pass flags argument to
	ix86_valid_target_attribute_inner_p.
	(get_builtin_code_for_version): Use 0 as it's target attribute.

gcc/testsuite/ChangeLog:

2019-04-18  Martin Liska  <mliska@suse.cz>

	* gcc.target/i386/funcspec-4.c: Update scanned pattern.
---
 gcc/cgraphclones.c                         |  2 +-
 gcc/config/i386/i386-c.c                   |  5 +-
 gcc/config/i386/i386-protos.h              |  4 +-
 gcc/config/i386/i386.c                     | 56 +++++++++++++---------
 gcc/testsuite/gcc.target/i386/funcspec-4.c |  2 +-
 5 files changed, 41 insertions(+), 28 deletions(-)

diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index 15f7e119d18..fd867ecac91 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -1056,7 +1056,7 @@ cgraph_node::create_version_clone_with_body
       location_t saved_loc = input_location;
       tree v = TREE_VALUE (target_attributes);
       input_location = DECL_SOURCE_LOCATION (new_decl);
-      bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 0);
+      bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 1);
       input_location = saved_loc;
       if (!r)
 	return NULL;
diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c
index 5e7e46fcebe..50cac3b1a9f 100644
--- a/gcc/config/i386/i386-c.c
+++ b/gcc/config/i386/i386-c.c
@@ -586,8 +586,9 @@ ix86_pragma_target_parse (tree args, tree pop_target)
     }
   else
     {
-      cur_tree = ix86_valid_target_attribute_tree (args, &global_options,
-						   &global_options_set);
+      cur_tree = ix86_valid_target_attribute_tree (NULL_TREE, args,
+						   &global_options,
+						   &global_options_set, 0);
       if (!cur_tree || cur_tree == error_mark_node)
        {
          cl_target_option_restore (&global_options,
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 83645e89a81..cfc4783205e 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -215,9 +215,9 @@ extern unsigned int ix86_minimum_alignment (tree, machine_mode,
 extern tree ix86_handle_shared_attribute (tree *, tree, tree, int, bool *);
 extern tree ix86_handle_selectany_attribute (tree *, tree, tree, int, bool *);
 extern int x86_field_alignment (tree, int);
-extern tree ix86_valid_target_attribute_tree (tree,
+extern tree ix86_valid_target_attribute_tree (tree, tree,
 					      struct gcc_options *,
-					      struct gcc_options *);
+					      struct gcc_options *, bool);
 extern unsigned int ix86_get_callcvt (const_tree);
 
 #endif
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ed9a9c2e3f7..eff22215e99 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -847,10 +847,11 @@ static void ix86_function_specific_post_stream_in (struct cl_target_option *);
 static void ix86_function_specific_print (FILE *, int,
 					  struct cl_target_option *);
 static bool ix86_valid_target_attribute_p (tree, tree, tree, int);
-static bool ix86_valid_target_attribute_inner_p (tree, char *[],
+static bool ix86_valid_target_attribute_inner_p (tree, tree, char *[],
 						 struct gcc_options *,
 						 struct gcc_options *,
-						 struct gcc_options *);
+						 struct gcc_options *,
+						 bool);
 static bool ix86_can_inline_p (tree, tree);
 static void ix86_set_current_function (tree);
 static unsigned int ix86_minimum_incoming_stack_boundary (bool);
@@ -5149,10 +5150,11 @@ ix86_function_specific_print (FILE *file, int indent,
    over the list.  */
 
 static bool
-ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
+ix86_valid_target_attribute_inner_p (tree fndecl, tree args, char *p_strings[],
 				     struct gcc_options *opts,
 				     struct gcc_options *opts_set,
-				     struct gcc_options *enum_opts_set)
+				     struct gcc_options *enum_opts_set,
+				     bool target_clone_attr)
 {
   char *next_optstr;
   bool ret = true;
@@ -5296,9 +5298,12 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
     IX86_ATTR_YES ("recip",
 		   OPT_mrecip,
 		   MASK_RECIP),
-
   };
 
+  location_t loc
+    = fndecl == NULL ? UNKNOWN_LOCATION : DECL_SOURCE_LOCATION (fndecl);
+  const char *attr_name = target_clone_attr ? "target_clone" : "target";
+
   /* If this is a list, recurse to get the options.  */
   if (TREE_CODE (args) == TREE_LIST)
     {
@@ -5306,9 +5311,10 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 
       for (; args; args = TREE_CHAIN (args))
 	if (TREE_VALUE (args)
-	    && !ix86_valid_target_attribute_inner_p (TREE_VALUE (args),
+	    && !ix86_valid_target_attribute_inner_p (fndecl, TREE_VALUE (args),
 						     p_strings, opts, opts_set,
-						     enum_opts_set))
+						     enum_opts_set,
+						     target_clone_attr))
 	  ret = false;
 
       return ret;
@@ -5316,7 +5322,7 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 
   else if (TREE_CODE (args) != STRING_CST)
     {
-      error ("attribute %<target%> argument not a string");
+      error_at (loc, "attribute %qs argument not a string", attr_name);
       return false;
     }
 
@@ -5328,7 +5334,6 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
       char *p = next_optstr;
       char *orig_p = p;
       char *comma = strchr (next_optstr, ',');
-      const char *opt_string;
       size_t len, opt_len;
       int opt;
       bool opt_set_p;
@@ -5374,7 +5379,6 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 	    {
 	      opt = attrs[i].opt;
 	      mask = attrs[i].mask;
-	      opt_string = attrs[i].string;
 	      break;
 	    }
 	}
@@ -5382,7 +5386,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
       /* Process the option.  */
       if (opt == N_OPTS)
 	{
-	  error ("attribute(target(\"%s\")) is unknown", orig_p);
+	  error_at (loc, "attribute value %qs is unknown in %qs attribute",
+		    orig_p, attr_name);
 	  ret = false;
 	}
 
@@ -5410,7 +5415,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 	{
 	  if (p_strings[opt])
 	    {
-	      error ("option(\"%s\") was already specified", opt_string);
+	      error_at (loc, "attribute value %qs was already specified "
+			"in %qs attribute", orig_p, attr_name);
 	      ret = false;
 	    }
 	  else
@@ -5429,7 +5435,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 			global_dc);
 	  else
 	    {
-	      error ("attribute(target(\"%s\")) is unknown", orig_p);
+	      error_at (loc, "attribute value %qs is unknown in %qs attribute",
+			orig_p, attr_name);
 	      ret = false;
 	    }
 	}
@@ -5453,9 +5460,10 @@ release_options_strings (char **option_strings)
 /* Return a TARGET_OPTION_NODE tree of the target options listed or NULL.  */
 
 tree
-ix86_valid_target_attribute_tree (tree args,
+ix86_valid_target_attribute_tree (tree fndecl, tree args,
 				  struct gcc_options *opts,
-				  struct gcc_options *opts_set)
+				  struct gcc_options *opts_set,
+				  bool target_clone_attr)
 {
   const char *orig_arch_string = opts->x_ix86_arch_string;
   const char *orig_tune_string = opts->x_ix86_tune_string;
@@ -5471,8 +5479,9 @@ ix86_valid_target_attribute_tree (tree args,
   memset (&enum_opts_set, 0, sizeof (enum_opts_set));
 
   /* Process each of the options on the chain.  */
-  if (! ix86_valid_target_attribute_inner_p (args, option_strings, opts,
-					     opts_set, &enum_opts_set))
+  if (!ix86_valid_target_attribute_inner_p (fndecl, args, option_strings, opts,
+					    opts_set, &enum_opts_set,
+					    target_clone_attr))
     return error_mark_node;
 
   /* If the changed options are different from the default, rerun
@@ -5545,7 +5554,7 @@ static bool
 ix86_valid_target_attribute_p (tree fndecl,
 			       tree ARG_UNUSED (name),
 			       tree args,
-			       int ARG_UNUSED (flags))
+			       int flags)
 {
   struct gcc_options func_options;
   tree new_target, new_optimize;
@@ -5580,8 +5589,10 @@ ix86_valid_target_attribute_p (tree fndecl,
   cl_target_option_restore (&func_options,
 			    TREE_TARGET_OPTION (target_option_default_node));
 
-  new_target = ix86_valid_target_attribute_tree (args, &func_options,
-						 &global_options_set);
+  /* FLAGS == 1 is used for target_clones attribute.  */
+  new_target
+    = ix86_valid_target_attribute_tree (fndecl, args, &func_options,
+					&global_options_set, flags == 1);
 
   new_optimize = build_optimization_node (&func_options);
 
@@ -31967,8 +31978,9 @@ get_builtin_code_for_version (tree decl, tree *predicate_list)
   if (strstr (attrs_str, "arch=") != NULL)
     {
       cl_target_option_save (&cur_target, &global_options);
-      target_node = ix86_valid_target_attribute_tree (attrs, &global_options,
-						      &global_options_set);
+      target_node
+	= ix86_valid_target_attribute_tree (decl, attrs, &global_options,
+					    &global_options_set, 0);
     
       gcc_assert (target_node);
       if (target_node == error_mark_node)
diff --git a/gcc/testsuite/gcc.target/i386/funcspec-4.c b/gcc/testsuite/gcc.target/i386/funcspec-4.c
index 025b97dff8e..e345acdef17 100644
--- a/gcc/testsuite/gcc.target/i386/funcspec-4.c
+++ b/gcc/testsuite/gcc.target/i386/funcspec-4.c
@@ -5,7 +5,7 @@
 extern void error1 (void) __attribute__((__target__("fma400"))); /* { dg-error "unknown" } */
 
 /* Multiple arch switches */
-extern void error2 (void) __attribute__((__target__("arch=core2,arch=k8"))); /* { dg-error "already specified" } */
+extern void error2 (void) __attribute__((__target__("arch=core2,arch=k8"))); /* { dg-error "attribute value 'arch=k8' was already specified in 'target' attribute" } */
 
 /* Unknown tune target */
 extern void error3 (void) __attribute__((__target__("tune=foobar"))); /* { dg-error "bad value" } */
-- 
2.21.0

Re: [PATCH][stage1] Enhance target and target_clone error messages.

[Martin Sebor]

On 4/18/19 5:09 AM, Martin Liška wrote:
> Hi.
> 
> The patch distinguishes among target and target_clone attribute when
> reporting for an error. I've also reworded the affected error messages
> a bit.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed after stage1 opens?
> Thanks,
> Martin
> 
> ---
>   gcc/cgraphclones.c                         |  2 +-
>   gcc/config/i386/i386-c.c                   |  5 +-
>   gcc/config/i386/i386-protos.h              |  4 +-
>   gcc/config/i386/i386.c                     | 54 +++++++++++++---------
>   gcc/testsuite/gcc.target/i386/funcspec-4.c |  2 +-
>   5 files changed, 39 insertions(+), 28 deletions(-)

Just a suggestion to also consider making the phrasing in the messages
consistent by adding "is" below

@@ -5316,7 +5322,7 @@ ix86_valid_target_attribute_inner_p (tree args, 
char *p_strings[],

    else if (TREE_CODE (args) != STRING_CST)
      {
-      error ("attribute %<target%> argument not a string");
+      error_at (loc, "attribute %qs argument not a string", attr_name);
        return false;

i.e., "is not a string" and changing the below

@@ -5382,7 +5386,8 @@ ix86_valid_target_attribute_inner_p (tree args, 
char *p_strings[],
        /* Process the option.  */
        if (opt == N_OPTS)
  	{
-	  error ("attribute(target(\"%s\")) is unknown", orig_p);
+	  error_at (loc, "attribute value %qs is unknown in %qs attribute",
+		    orig_p, attr_name);

to

   error_at (loc, "attribute %qs argument %qs is unknown",
             attr_name, orig_p);

Martin

Re: [PATCH][stage1] Enhance target and target_clone error messages.

[Martin Liška]

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

On 4/19/19 2:28 AM, Martin Sebor wrote:
> On 4/18/19 5:09 AM, Martin Liška wrote:
>> Hi.
>>
>> The patch distinguishes among target and target_clone attribute when
>> reporting for an error. I've also reworded the affected error messages
>> a bit.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed after stage1 opens?
>> Thanks,
>> Martin
>>
>> ---
>>   gcc/cgraphclones.c                         |  2 +-
>>   gcc/config/i386/i386-c.c                   |  5 +-
>>   gcc/config/i386/i386-protos.h              |  4 +-
>>   gcc/config/i386/i386.c                     | 54 +++++++++++++---------
>>   gcc/testsuite/gcc.target/i386/funcspec-4.c |  2 +-
>>   5 files changed, 39 insertions(+), 28 deletions(-)
> 
> Just a suggestion to also consider making the phrasing in the messages
> consistent by adding "is" below
> 
> @@ -5316,7 +5322,7 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
> 
>    else if (TREE_CODE (args) != STRING_CST)
>      {
> -      error ("attribute %<target%> argument not a string");
> +      error_at (loc, "attribute %qs argument not a string", attr_name);
>        return false;
> 
> i.e., "is not a string" and changing the below
> 
> @@ -5382,7 +5386,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
>        /* Process the option.  */
>        if (opt == N_OPTS)
>      {
> -      error ("attribute(target(\"%s\")) is unknown", orig_p);
> +      error_at (loc, "attribute value %qs is unknown in %qs attribute",
> +            orig_p, attr_name);
> 
> to
> 
>   error_at (loc, "attribute %qs argument %qs is unknown",
>             attr_name, orig_p);
> 
> Martin

Thank you Martin. I'm sending updated patch.

Martin

[-- Attachment #2: 0001-Enhance-target-and-target_clone-error-messages.patch --]
[-- Type: text/x-patch, Size: 11173 bytes --]

From 2778f9ab4f1cd091780694e8cc1335d6125d95ec Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 17 Apr 2019 13:59:14 +0200
Subject: [PATCH] Enhance target and target_clone error messages.

gcc/ChangeLog:

2019-04-18  Martin Liska  <mliska@suse.cz>

	* cgraphclones.c: Call valid_attribute_p with 1 for
	target_clone.
	* config/i386/i386-c.c (ix86_pragma_target_parse): Use 0 as
	it's for target attribute.
	* config/i386/i386-protos.h (ix86_valid_target_attribute_tree):
	Add new boolean argument.
	* config/i386/i386.c (ix86_valid_target_attribute_inner_p):
	Likewise.
	(ix86_valid_target_attribute_tree): Pass target_clone_attr
	to ix86_valid_target_attribute_inner_p.
	(ix86_valid_target_attribute_p): Pass flags argument to
	ix86_valid_target_attribute_inner_p.
	(get_builtin_code_for_version): Use 0 as it's target attribute.

gcc/testsuite/ChangeLog:

2019-04-18  Martin Liska  <mliska@suse.cz>

	* gcc.target/i386/funcspec-4.c: Update scanned pattern.
	* g++.target/i386/pr57362.C: Likewise.
---
 gcc/cgraphclones.c                         |  2 +-
 gcc/config/i386/i386-c.c                   |  5 +-
 gcc/config/i386/i386-protos.h              |  4 +-
 gcc/config/i386/i386.c                     | 56 +++++++++++++---------
 gcc/testsuite/g++.target/i386/pr57362.C    |  2 +-
 gcc/testsuite/gcc.target/i386/funcspec-4.c |  2 +-
 6 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index 15f7e119d18..fd867ecac91 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -1056,7 +1056,7 @@ cgraph_node::create_version_clone_with_body
       location_t saved_loc = input_location;
       tree v = TREE_VALUE (target_attributes);
       input_location = DECL_SOURCE_LOCATION (new_decl);
-      bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 0);
+      bool r = targetm.target_option.valid_attribute_p (new_decl, NULL, v, 1);
       input_location = saved_loc;
       if (!r)
 	return NULL;
diff --git a/gcc/config/i386/i386-c.c b/gcc/config/i386/i386-c.c
index 5e7e46fcebe..50cac3b1a9f 100644
--- a/gcc/config/i386/i386-c.c
+++ b/gcc/config/i386/i386-c.c
@@ -586,8 +586,9 @@ ix86_pragma_target_parse (tree args, tree pop_target)
     }
   else
     {
-      cur_tree = ix86_valid_target_attribute_tree (args, &global_options,
-						   &global_options_set);
+      cur_tree = ix86_valid_target_attribute_tree (NULL_TREE, args,
+						   &global_options,
+						   &global_options_set, 0);
       if (!cur_tree || cur_tree == error_mark_node)
        {
          cl_target_option_restore (&global_options,
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 83645e89a81..cfc4783205e 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -215,9 +215,9 @@ extern unsigned int ix86_minimum_alignment (tree, machine_mode,
 extern tree ix86_handle_shared_attribute (tree *, tree, tree, int, bool *);
 extern tree ix86_handle_selectany_attribute (tree *, tree, tree, int, bool *);
 extern int x86_field_alignment (tree, int);
-extern tree ix86_valid_target_attribute_tree (tree,
+extern tree ix86_valid_target_attribute_tree (tree, tree,
 					      struct gcc_options *,
-					      struct gcc_options *);
+					      struct gcc_options *, bool);
 extern unsigned int ix86_get_callcvt (const_tree);
 
 #endif
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 198af816f74..49f48e6041c 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -847,10 +847,11 @@ static void ix86_function_specific_post_stream_in (struct cl_target_option *);
 static void ix86_function_specific_print (FILE *, int,
 					  struct cl_target_option *);
 static bool ix86_valid_target_attribute_p (tree, tree, tree, int);
-static bool ix86_valid_target_attribute_inner_p (tree, char *[],
+static bool ix86_valid_target_attribute_inner_p (tree, tree, char *[],
 						 struct gcc_options *,
 						 struct gcc_options *,
-						 struct gcc_options *);
+						 struct gcc_options *,
+						 bool);
 static bool ix86_can_inline_p (tree, tree);
 static void ix86_set_current_function (tree);
 static unsigned int ix86_minimum_incoming_stack_boundary (bool);
@@ -5149,10 +5150,11 @@ ix86_function_specific_print (FILE *file, int indent,
    over the list.  */
 
 static bool
-ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
+ix86_valid_target_attribute_inner_p (tree fndecl, tree args, char *p_strings[],
 				     struct gcc_options *opts,
 				     struct gcc_options *opts_set,
-				     struct gcc_options *enum_opts_set)
+				     struct gcc_options *enum_opts_set,
+				     bool target_clone_attr)
 {
   char *next_optstr;
   bool ret = true;
@@ -5296,9 +5298,12 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
     IX86_ATTR_YES ("recip",
 		   OPT_mrecip,
 		   MASK_RECIP),
-
   };
 
+  location_t loc
+    = fndecl == NULL ? UNKNOWN_LOCATION : DECL_SOURCE_LOCATION (fndecl);
+  const char *attr_name = target_clone_attr ? "target_clone" : "target";
+
   /* If this is a list, recurse to get the options.  */
   if (TREE_CODE (args) == TREE_LIST)
     {
@@ -5306,9 +5311,10 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 
       for (; args; args = TREE_CHAIN (args))
 	if (TREE_VALUE (args)
-	    && !ix86_valid_target_attribute_inner_p (TREE_VALUE (args),
+	    && !ix86_valid_target_attribute_inner_p (fndecl, TREE_VALUE (args),
 						     p_strings, opts, opts_set,
-						     enum_opts_set))
+						     enum_opts_set,
+						     target_clone_attr))
 	  ret = false;
 
       return ret;
@@ -5316,7 +5322,7 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 
   else if (TREE_CODE (args) != STRING_CST)
     {
-      error ("attribute %<target%> argument not a string");
+      error_at (loc, "attribute %qs argument is not a string", attr_name);
       return false;
     }
 
@@ -5328,7 +5334,6 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
       char *p = next_optstr;
       char *orig_p = p;
       char *comma = strchr (next_optstr, ',');
-      const char *opt_string;
       size_t len, opt_len;
       int opt;
       bool opt_set_p;
@@ -5374,7 +5379,6 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 	    {
 	      opt = attrs[i].opt;
 	      mask = attrs[i].mask;
-	      opt_string = attrs[i].string;
 	      break;
 	    }
 	}
@@ -5382,7 +5386,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
       /* Process the option.  */
       if (opt == N_OPTS)
 	{
-	  error ("attribute(target(\"%s\")) is unknown", orig_p);
+	  error_at (loc, "attribute %qs argument %qs is unknown",
+		    orig_p, attr_name);
 	  ret = false;
 	}
 
@@ -5410,7 +5415,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 	{
 	  if (p_strings[opt])
 	    {
-	      error ("option(\"%s\") was already specified", opt_string);
+	      error_at (loc, "attribute value %qs was already specified "
+			"in %qs attribute", orig_p, attr_name);
 	      ret = false;
 	    }
 	  else
@@ -5429,7 +5435,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
 			global_dc);
 	  else
 	    {
-	      error ("attribute(target(\"%s\")) is unknown", orig_p);
+	      error_at (loc, "attribute value %qs is unknown in %qs attribute",
+			orig_p, attr_name);
 	      ret = false;
 	    }
 	}
@@ -5453,9 +5460,10 @@ release_options_strings (char **option_strings)
 /* Return a TARGET_OPTION_NODE tree of the target options listed or NULL.  */
 
 tree
-ix86_valid_target_attribute_tree (tree args,
+ix86_valid_target_attribute_tree (tree fndecl, tree args,
 				  struct gcc_options *opts,
-				  struct gcc_options *opts_set)
+				  struct gcc_options *opts_set,
+				  bool target_clone_attr)
 {
   const char *orig_arch_string = opts->x_ix86_arch_string;
   const char *orig_tune_string = opts->x_ix86_tune_string;
@@ -5471,8 +5479,9 @@ ix86_valid_target_attribute_tree (tree args,
   memset (&enum_opts_set, 0, sizeof (enum_opts_set));
 
   /* Process each of the options on the chain.  */
-  if (! ix86_valid_target_attribute_inner_p (args, option_strings, opts,
-					     opts_set, &enum_opts_set))
+  if (!ix86_valid_target_attribute_inner_p (fndecl, args, option_strings, opts,
+					    opts_set, &enum_opts_set,
+					    target_clone_attr))
     return error_mark_node;
 
   /* If the changed options are different from the default, rerun
@@ -5545,7 +5554,7 @@ static bool
 ix86_valid_target_attribute_p (tree fndecl,
 			       tree ARG_UNUSED (name),
 			       tree args,
-			       int ARG_UNUSED (flags))
+			       int flags)
 {
   struct gcc_options func_options;
   tree new_target, new_optimize;
@@ -5580,8 +5589,10 @@ ix86_valid_target_attribute_p (tree fndecl,
   cl_target_option_restore (&func_options,
 			    TREE_TARGET_OPTION (target_option_default_node));
 
-  new_target = ix86_valid_target_attribute_tree (args, &func_options,
-						 &global_options_set);
+  /* FLAGS == 1 is used for target_clones attribute.  */
+  new_target
+    = ix86_valid_target_attribute_tree (fndecl, args, &func_options,
+					&global_options_set, flags == 1);
 
   new_optimize = build_optimization_node (&func_options);
 
@@ -32103,8 +32114,9 @@ get_builtin_code_for_version (tree decl, tree *predicate_list)
   if (strstr (attrs_str, "arch=") != NULL)
     {
       cl_target_option_save (&cur_target, &global_options);
-      target_node = ix86_valid_target_attribute_tree (attrs, &global_options,
-						      &global_options_set);
+      target_node
+	= ix86_valid_target_attribute_tree (decl, attrs, &global_options,
+					    &global_options_set, 0);
     
       gcc_assert (target_node);
       if (target_node == error_mark_node)
diff --git a/gcc/testsuite/g++.target/i386/pr57362.C b/gcc/testsuite/g++.target/i386/pr57362.C
index 5e612130357..c76e5206c32 100644
--- a/gcc/testsuite/g++.target/i386/pr57362.C
+++ b/gcc/testsuite/g++.target/i386/pr57362.C
@@ -199,4 +199,4 @@ int foo(void) { return 1; }
 /* { dg-prune-output "attribute.* is unknown" } */
 /* { dg-prune-output "missing 'target' attribute*" } */
 /* { dg-prune-output "redefinition of 'int foo" } */
-/* { dg-prune-output "no dispatcher found for" } */
+/* { dg-prune-output "ISA '.*' is not supported in 'target' attribute, use 'arch=' syntax" } */
diff --git a/gcc/testsuite/gcc.target/i386/funcspec-4.c b/gcc/testsuite/gcc.target/i386/funcspec-4.c
index 025b97dff8e..e345acdef17 100644
--- a/gcc/testsuite/gcc.target/i386/funcspec-4.c
+++ b/gcc/testsuite/gcc.target/i386/funcspec-4.c
@@ -5,7 +5,7 @@
 extern void error1 (void) __attribute__((__target__("fma400"))); /* { dg-error "unknown" } */
 
 /* Multiple arch switches */
-extern void error2 (void) __attribute__((__target__("arch=core2,arch=k8"))); /* { dg-error "already specified" } */
+extern void error2 (void) __attribute__((__target__("arch=core2,arch=k8"))); /* { dg-error "attribute value 'arch=k8' was already specified in 'target' attribute" } */
 
 /* Unknown tune target */
 extern void error3 (void) __attribute__((__target__("tune=foobar"))); /* { dg-error "bad value" } */
-- 
2.21.0

Re: [PATCH][stage1] Enhance target and target_clone error messages.

[Jeff Law]

On 4/23/19 4:06 AM, Martin Liška wrote:
> On 4/19/19 2:28 AM, Martin Sebor wrote:
>> On 4/18/19 5:09 AM, Martin Liška wrote:
>>> Hi.
>>>
>>> The patch distinguishes among target and target_clone attribute when
>>> reporting for an error. I've also reworded the affected error messages
>>> a bit.
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>
>>> Ready to be installed after stage1 opens?
>>> Thanks,
>>> Martin
>>>
>>> ---
>>>   gcc/cgraphclones.c                         |  2 +-
>>>   gcc/config/i386/i386-c.c                   |  5 +-
>>>   gcc/config/i386/i386-protos.h              |  4 +-
>>>   gcc/config/i386/i386.c                     | 54 +++++++++++++---------
>>>   gcc/testsuite/gcc.target/i386/funcspec-4.c |  2 +-
>>>   5 files changed, 39 insertions(+), 28 deletions(-)
>> Just a suggestion to also consider making the phrasing in the messages
>> consistent by adding "is" below
>>
>> @@ -5316,7 +5322,7 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
>>
>>    else if (TREE_CODE (args) != STRING_CST)
>>      {
>> -      error ("attribute %<target%> argument not a string");
>> +      error_at (loc, "attribute %qs argument not a string", attr_name);
>>        return false;
>>
>> i.e., "is not a string" and changing the below
>>
>> @@ -5382,7 +5386,8 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[],
>>        /* Process the option.  */
>>        if (opt == N_OPTS)
>>      {
>> -      error ("attribute(target(\"%s\")) is unknown", orig_p);
>> +      error_at (loc, "attribute value %qs is unknown in %qs attribute",
>> +            orig_p, attr_name);
>>
>> to
>>
>>   error_at (loc, "attribute %qs argument %qs is unknown",
>>             attr_name, orig_p);
>>
>> Martin
> Thank you Martin. I'm sending updated patch.
> 
> Martin
> 
> 
> 0001-Enhance-target-and-target_clone-error-messages.patch
> 
> From 2778f9ab4f1cd091780694e8cc1335d6125d95ec Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Wed, 17 Apr 2019 13:59:14 +0200
> Subject: [PATCH] Enhance target and target_clone error messages.
> 
> gcc/ChangeLog:
> 
> 2019-04-18  Martin Liska  <mliska@suse.cz>
> 
> 	* cgraphclones.c: Call valid_attribute_p with 1 for
> 	target_clone.
> 	* config/i386/i386-c.c (ix86_pragma_target_parse): Use 0 as
> 	it's for target attribute.
> 	* config/i386/i386-protos.h (ix86_valid_target_attribute_tree):
> 	Add new boolean argument.
> 	* config/i386/i386.c (ix86_valid_target_attribute_inner_p):
> 	Likewise.
> 	(ix86_valid_target_attribute_tree): Pass target_clone_attr
> 	to ix86_valid_target_attribute_inner_p.
> 	(ix86_valid_target_attribute_p): Pass flags argument to
> 	ix86_valid_target_attribute_inner_p.
> 	(get_builtin_code_for_version): Use 0 as it's target attribute.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-04-18  Martin Liska  <mliska@suse.cz>
> 
> 	* gcc.target/i386/funcspec-4.c: Update scanned pattern.
> 	* g++.target/i386/pr57362.C: Likewise.
OK
Jeff