[PATCH], PowerPC target_clones minor support
Michael Meissner
meissner@linux.vnet.ibm.com
Fri Jul 7 18:45:00 GMT 2017
On Fri, Jul 07, 2017 at 07:22:04AM -0500, Segher Boessenkool wrote:
> On Wed, Jun 28, 2017 at 02:28:23PM -0400, Michael Meissner wrote:
> > Some minor changes to the PowerPC target_clones support:
> >
> > 1) I added a warning if target_clones was used and the compiler whas configured
> > with an older glibc where __builtin_cpu_supports always returns 0;
> >
> > 2) I reworked how the ifunc resolver function is generated, and always made it
> > a static function;
> >
> > 3) I added an executable target_clones test, and I made both clone tests
> > dependent on GCC being configured with a new glibc.
>
> > * config/rs6000/rs6000.c
> > (rs6000_get_function_versions_dispatcher): Add warning if the
> > compiler is not configured to use at least GLIBC version 2.23.
>
> Please say what is really tested for here (namely,
> TARGET_LIBC_PROVIDES_HWCAP_IN_TCB).
I've reworded both the warning message and the ChangeLog entry.
> > /* Append the filename to the resolver function if the versions are
> > not externally visible. This is because the resolver function has
> > to be externally visible for the loader to find it. So, appending
> > the filename will prevent conflicts with a resolver function from
> > another module which is based on the same version name. */
> > - char *resolver_name = make_unique_name (default_decl, "resolver", is_uniq);
> > + tree decl_name = clone_function_name (default_decl, "resolver");
> > + const char *resolver_name = IDENTIFIER_POINTER (decl_name);
>
> I think the comment needs some updating now?
Yes.
> > --- gcc/testsuite/gcc.target/powerpc/clone2.c (.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/testsuite/gcc.target/powerpc) (revision 0)
> > +++ gcc/testsuite/gcc.target/powerpc/clone2.c (.../gcc/testsuite/gcc.target/powerpc) (revision 249738)
> > @@ -0,0 +1,31 @@
> > +/* { dg-do run { target { powerpc*-*-linux* } } } */
> > +/* { dg-options "-mvsx -O2" } */
> > +/* { dg-require-effective-target powerpc_p9vector_ok } */
> > +/* { dg-require-effective-target ppc_cpu_supports_hw } */
>
> What a funny name (it reads as "the CPU supports the hardware"). Yes
> I'm easily amused ;-)
>
> The patch is okay for trunk modulo with those things looked at. Sorry
> for the slow review.
Here is the patch I committed:
[gcc]
2017-07-07 Michael Meissner <meissner@linux.vnet.ibm.com>
* config/rs6000/rs6000.c (rs6000_get_function_versions_dispatcher):
Add warning if GCC was not configured to link against a GLIBC that
exports the hardware capability bits.
(make_resolver_func): Make resolver function private and not a
COMDAT function. Create the name with clone_function_name instead
of make_unique_name.
[gcc/testsuite]
2017-07-07 Michael Meissner <meissner@linux.vnet.ibm.com>
* gcc.target/powerpc/clone1.c: Add check to make sure the
__builtin_cpu_supports function is fully supported.
* gcc.target/powerpc/clone2.c: New runtime test for
target_clones.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797
-------------- next part --------------
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 250054)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -37283,6 +37283,12 @@ rs6000_get_function_versions_dispatcher
default_node = default_version_info->this_node;
+#ifndef TARGET_LIBC_PROVIDES_HWCAP_IN_TCB
+ warning_at (DECL_SOURCE_LOCATION (default_node->decl), 0,
+ "target_clone needs GLIBC (2.23 and newer) to export hardware "
+ "capability bits");
+#endif
+
if (targetm.has_ifunc_p ())
{
struct cgraph_function_version_info *it_v = NULL;
@@ -37328,29 +37334,19 @@ make_resolver_func (const tree default_d
const tree dispatch_decl,
basic_block *empty_bb)
{
- /* IFUNC's have to be globally visible. So, if the default_decl is
- not, then the name of the IFUNC should be made unique. */
- bool is_uniq = (TREE_PUBLIC (default_decl) == 0);
-
- /* Append the filename to the resolver function if the versions are
- not externally visible. This is because the resolver function has
- to be externally visible for the loader to find it. So, appending
- the filename will prevent conflicts with a resolver function from
- another module which is based on the same version name. */
- char *resolver_name = make_unique_name (default_decl, "resolver", is_uniq);
-
- /* The resolver function should return a (void *). */
+ /* Make the resolver function static. The resolver function returns
+ void *. */
+ tree decl_name = clone_function_name (default_decl, "resolver");
+ const char *resolver_name = IDENTIFIER_POINTER (decl_name);
tree type = build_function_type_list (ptr_type_node, NULL_TREE);
tree decl = build_fn_decl (resolver_name, type);
- tree decl_name = get_identifier (resolver_name);
SET_DECL_ASSEMBLER_NAME (decl, decl_name);
DECL_NAME (decl) = decl_name;
TREE_USED (decl) = 1;
DECL_ARTIFICIAL (decl) = 1;
DECL_IGNORED_P (decl) = 0;
- /* IFUNC resolvers have to be externally visible. */
- TREE_PUBLIC (decl) = 1;
+ TREE_PUBLIC (decl) = 0;
DECL_UNINLINABLE (decl) = 1;
/* Resolver is not external, body is generated. */
@@ -37361,15 +37357,6 @@ make_resolver_func (const tree default_d
DECL_INITIAL (decl) = make_node (BLOCK);
DECL_STATIC_CONSTRUCTOR (decl) = 0;
- if (DECL_COMDAT_GROUP (default_decl) || TREE_PUBLIC (default_decl))
- {
- /* In this case, each translation unit with a call to this
- versioned function will put out a resolver. Ensure it
- is comdat to keep just one copy. */
- DECL_COMDAT (decl) = 1;
- make_decl_one_only (decl, DECL_ASSEMBLER_NAME (decl));
- }
-
/* Build result decl and add to function_decl. */
tree t = build_decl (UNKNOWN_LOCATION, RESULT_DECL, NULL_TREE, ptr_type_node);
DECL_ARTIFICIAL (t) = 1;
@@ -37391,7 +37378,7 @@ make_resolver_func (const tree default_d
= make_attribute ("ifunc", resolver_name, DECL_ATTRIBUTES (dispatch_decl));
cgraph_node::create_same_body_alias (dispatch_decl, decl);
- XDELETEVEC (resolver_name);
+
return decl;
}
Index: gcc/testsuite/gcc.target/powerpc/clone1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/clone1.c (revision 250054)
+++ gcc/testsuite/gcc.target/powerpc/clone1.c (working copy)
@@ -2,6 +2,7 @@
/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
/* { dg-options "-mcpu=power8 -O2" } */
/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-require-effective-target ppc_cpu_supports_hw } */
/* Power9 (aka, ISA 3.0) has a MODSD instruction to do modulus, while Power8
(aka, ISA 2.07) has to do modulus with divide and multiply. Make sure
Index: gcc/testsuite/gcc.target/powerpc/clone2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/clone2.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/clone2.c (revision 0)
@@ -0,0 +1,31 @@
+/* { dg-do run { target { powerpc*-*-linux* } } } */
+/* { dg-options "-mvsx -O2" } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */
+/* { dg-require-effective-target ppc_cpu_supports_hw } */
+
+#include <stddef.h>
+#include <stdlib.h>
+
+/* Power9 (aka, ISA 3.0) has a MODSD instruction to do modulus, while Power8
+ (aka, ISA 2.07) has to do modulus with divide and multiply. Make sure that
+ the basic support for target_clones runs.
+
+ Restrict ourselves to Linux, since IFUNC might not be supported in other
+ operating systems. */
+
+__attribute__((__target_clones__("cpu=power9,default")))
+long mod_func (long a, long b)
+{
+ return a % b;
+}
+
+#define X 53L
+#define Y 7L
+int
+main (void)
+{
+ if (mod_func (X, Y) != (X % Y))
+ abort ();
+
+ return 0;
+}
More information about the Gcc-patches
mailing list