ipa: Improve error handling for target_clone single value

Message ID cd471307-ce05-ce70-9e34-3d6d1fc63bc8@suse.cz
State New
Headers
Series ipa: Improve error handling for target_clone single value |

Commit Message

Martin Liška Feb. 28, 2022, 1:27 p.m. UTC
  The patch moves attribute checking to handle_target_clones_attribute where
we drop the attribute if it contains only a single value.

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

Ready to be installed?
Thanks,
Martin

	PR ipa/104533

gcc/c-family/ChangeLog:

	* c-attribs.cc (handle_target_clones_attribute): Use
	get_target_clone_attr_len and report warning soon.

gcc/ChangeLog:

	* multiple_target.cc (get_attr_len): Move to tree.c.
	(expand_target_clones): Remove single value checking.
	* tree.cc (get_target_clone_attr_len): New fn.
	* tree.h (get_target_clone_attr_len): Likewise.

gcc/testsuite/ChangeLog:

	* g++.target/i386/pr104533.C: New test.
---
  gcc/c-family/c-attribs.cc                |  6 ++++++
  gcc/multiple_target.cc                   | 26 +-----------------------
  gcc/testsuite/g++.target/i386/pr104533.C | 11 ++++++++++
  gcc/tree.cc                              | 24 ++++++++++++++++++++++
  gcc/tree.h                               |  2 ++
  5 files changed, 44 insertions(+), 25 deletions(-)
  create mode 100644 gcc/testsuite/g++.target/i386/pr104533.C
  

Comments

Richard Biener March 1, 2022, 8:51 a.m. UTC | #1
On Mon, Feb 28, 2022 at 2:27 PM Martin Liška <mliska@suse.cz> wrote:
>
> The patch moves attribute checking to handle_target_clones_attribute where
> we drop the attribute if it contains only a single value.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?

OK.

Thanks,
Richard.

> Thanks,
> Martin
>
>         PR ipa/104533
>
> gcc/c-family/ChangeLog:
>
>         * c-attribs.cc (handle_target_clones_attribute): Use
>         get_target_clone_attr_len and report warning soon.
>
> gcc/ChangeLog:
>
>         * multiple_target.cc (get_attr_len): Move to tree.c.
>         (expand_target_clones): Remove single value checking.
>         * tree.cc (get_target_clone_attr_len): New fn.
>         * tree.h (get_target_clone_attr_len): Likewise.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.target/i386/pr104533.C: New test.
> ---
>   gcc/c-family/c-attribs.cc                |  6 ++++++
>   gcc/multiple_target.cc                   | 26 +-----------------------
>   gcc/testsuite/g++.target/i386/pr104533.C | 11 ++++++++++
>   gcc/tree.cc                              | 24 ++++++++++++++++++++++
>   gcc/tree.h                               |  2 ++
>   5 files changed, 44 insertions(+), 25 deletions(-)
>   create mode 100644 gcc/testsuite/g++.target/i386/pr104533.C
>
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index 3849dba90b2..d394ea9d57e 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -5486,6 +5486,12 @@ handle_target_clones_attribute (tree *node, tree name, tree ARG_UNUSED (args),
>                    "with %qs attribute", name, "target");
>           *no_add_attrs = true;
>         }
> +      else if (get_target_clone_attr_len (args) == -1)
> +       {
> +         warning (OPT_Wattributes,
> +                  "single %<target_clones%> attribute is ignored");
> +         *no_add_attrs = true;
> +       }
>         else
>         /* Do not inline functions with multiple clone targets.  */
>         DECL_UNINLINABLE (*node) = 1;
> diff --git a/gcc/multiple_target.cc b/gcc/multiple_target.cc
> index 5a5a75f0c38..7fe02fb55c8 100644
> --- a/gcc/multiple_target.cc
> +++ b/gcc/multiple_target.cc
> @@ -185,30 +185,6 @@ create_dispatcher_calls (struct cgraph_node *node)
>       }
>   }
>
> -/* Return length of attribute names string,
> -   if arglist chain > 1, -1 otherwise.  */
> -
> -static int
> -get_attr_len (tree arglist)
> -{
> -  tree arg;
> -  int str_len_sum = 0;
> -  int argnum = 0;
> -
> -  for (arg = arglist; arg; arg = TREE_CHAIN (arg))
> -    {
> -      const char *str = TREE_STRING_POINTER (TREE_VALUE (arg));
> -      size_t len = strlen (str);
> -      str_len_sum += len + 1;
> -      for (const char *p = strchr (str, ','); p; p = strchr (p + 1, ','))
> -       argnum++;
> -      argnum++;
> -    }
> -  if (argnum <= 1)
> -    return -1;
> -  return str_len_sum;
> -}
> -
>   /* Create string with attributes separated by comma.
>      Return number of attributes.  */
>
> @@ -342,7 +318,7 @@ expand_target_clones (struct cgraph_node *node, bool definition)
>       return false;
>
>     tree arglist = TREE_VALUE (attr_target);
> -  int attr_len = get_attr_len (arglist);
> +  int attr_len = get_target_clone_attr_len (arglist);
>
>     /* No need to clone for 1 target attribute.  */
>     if (attr_len == -1)
> diff --git a/gcc/testsuite/g++.target/i386/pr104533.C b/gcc/testsuite/g++.target/i386/pr104533.C
> new file mode 100644
> index 00000000000..6a1d8def097
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/i386/pr104533.C
> @@ -0,0 +1,11 @@
> +/* PR ipa/104533 */
> +/* { dg-do compile } */
> +/* { dg-additional-options "-std=c++11 -fPIC -Ofast -fno-semantic-interposition" } */
> +/* { dg-require-ifunc "" } */
> +
> +struct B
> +{
> +  virtual ~B();
> +};
> +__attribute__((target_clones("avx")))
> +B::~B() = default; /* { dg-warning "single .target_clones. attribute is ignored" } */
> diff --git a/gcc/tree.cc b/gcc/tree.cc
> index 2bbef2d6b75..4522d90c4d9 100644
> --- a/gcc/tree.cc
> +++ b/gcc/tree.cc
> @@ -14553,6 +14553,30 @@ get_attr_nonstring_decl (tree expr, tree *ref)
>     return NULL_TREE;
>   }
>
> +/* Return length of attribute names string,
> +   if arglist chain > 1, -1 otherwise.  */
> +
> +int
> +get_target_clone_attr_len (tree arglist)
> +{
> +  tree arg;
> +  int str_len_sum = 0;
> +  int argnum = 0;
> +
> +  for (arg = arglist; arg; arg = TREE_CHAIN (arg))
> +    {
> +      const char *str = TREE_STRING_POINTER (TREE_VALUE (arg));
> +      size_t len = strlen (str);
> +      str_len_sum += len + 1;
> +      for (const char *p = strchr (str, ','); p; p = strchr (p + 1, ','))
> +       argnum++;
> +      argnum++;
> +    }
> +  if (argnum <= 1)
> +    return -1;
> +  return str_len_sum;
> +}
> +
>   #if CHECKING_P
>
>   namespace selftest {
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 95334b077da..36ceed57064 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -6579,4 +6579,6 @@ extern unsigned fndecl_dealloc_argno (tree);
>      object or pointer.  Otherwise return null.  */
>   extern tree get_attr_nonstring_decl (tree, tree * = NULL);
>
> +extern int get_target_clone_attr_len (tree);
> +
>   #endif  /* GCC_TREE_H  */
> --
> 2.35.1
>
  

Patch

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 3849dba90b2..d394ea9d57e 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -5486,6 +5486,12 @@  handle_target_clones_attribute (tree *node, tree name, tree ARG_UNUSED (args),
  		   "with %qs attribute", name, "target");
  	  *no_add_attrs = true;
  	}
+      else if (get_target_clone_attr_len (args) == -1)
+	{
+	  warning (OPT_Wattributes,
+		   "single %<target_clones%> attribute is ignored");
+	  *no_add_attrs = true;
+	}
        else
        /* Do not inline functions with multiple clone targets.  */
  	DECL_UNINLINABLE (*node) = 1;
diff --git a/gcc/multiple_target.cc b/gcc/multiple_target.cc
index 5a5a75f0c38..7fe02fb55c8 100644
--- a/gcc/multiple_target.cc
+++ b/gcc/multiple_target.cc
@@ -185,30 +185,6 @@  create_dispatcher_calls (struct cgraph_node *node)
      }
  }
  
-/* Return length of attribute names string,
-   if arglist chain > 1, -1 otherwise.  */
-
-static int
-get_attr_len (tree arglist)
-{
-  tree arg;
-  int str_len_sum = 0;
-  int argnum = 0;
-
-  for (arg = arglist; arg; arg = TREE_CHAIN (arg))
-    {
-      const char *str = TREE_STRING_POINTER (TREE_VALUE (arg));
-      size_t len = strlen (str);
-      str_len_sum += len + 1;
-      for (const char *p = strchr (str, ','); p; p = strchr (p + 1, ','))
-	argnum++;
-      argnum++;
-    }
-  if (argnum <= 1)
-    return -1;
-  return str_len_sum;
-}
-
  /* Create string with attributes separated by comma.
     Return number of attributes.  */
  
@@ -342,7 +318,7 @@  expand_target_clones (struct cgraph_node *node, bool definition)
      return false;
  
    tree arglist = TREE_VALUE (attr_target);
-  int attr_len = get_attr_len (arglist);
+  int attr_len = get_target_clone_attr_len (arglist);
  
    /* No need to clone for 1 target attribute.  */
    if (attr_len == -1)
diff --git a/gcc/testsuite/g++.target/i386/pr104533.C b/gcc/testsuite/g++.target/i386/pr104533.C
new file mode 100644
index 00000000000..6a1d8def097
--- /dev/null
+++ b/gcc/testsuite/g++.target/i386/pr104533.C
@@ -0,0 +1,11 @@ 
+/* PR ipa/104533 */
+/* { dg-do compile } */
+/* { dg-additional-options "-std=c++11 -fPIC -Ofast -fno-semantic-interposition" } */
+/* { dg-require-ifunc "" } */
+
+struct B
+{
+  virtual ~B();
+};
+__attribute__((target_clones("avx")))
+B::~B() = default; /* { dg-warning "single .target_clones. attribute is ignored" } */
diff --git a/gcc/tree.cc b/gcc/tree.cc
index 2bbef2d6b75..4522d90c4d9 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -14553,6 +14553,30 @@  get_attr_nonstring_decl (tree expr, tree *ref)
    return NULL_TREE;
  }
  
+/* Return length of attribute names string,
+   if arglist chain > 1, -1 otherwise.  */
+
+int
+get_target_clone_attr_len (tree arglist)
+{
+  tree arg;
+  int str_len_sum = 0;
+  int argnum = 0;
+
+  for (arg = arglist; arg; arg = TREE_CHAIN (arg))
+    {
+      const char *str = TREE_STRING_POINTER (TREE_VALUE (arg));
+      size_t len = strlen (str);
+      str_len_sum += len + 1;
+      for (const char *p = strchr (str, ','); p; p = strchr (p + 1, ','))
+	argnum++;
+      argnum++;
+    }
+  if (argnum <= 1)
+    return -1;
+  return str_len_sum;
+}
+
  #if CHECKING_P
  
  namespace selftest {
diff --git a/gcc/tree.h b/gcc/tree.h
index 95334b077da..36ceed57064 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -6579,4 +6579,6 @@  extern unsigned fndecl_dealloc_argno (tree);
     object or pointer.  Otherwise return null.  */
  extern tree get_attr_nonstring_decl (tree, tree * = NULL);
  
+extern int get_target_clone_attr_len (tree);
+
  #endif  /* GCC_TREE_H  */