Re: [PATCH], PowerPC target_clones minor support

[Michael Meissner <meissner at linux dot vnet dot ibm dot com>]

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

Attachment: clone.patch16b
Description: Text document