GCC target_clone support

GCC target_clone support

[Michael Meissner]

I'm in the middle of adding support for the target_clone attribute to the
PowerPC.  At the moment, the x86_64/i386 port is the only port that supports
target_clone.  I added the arm/s390 port maintainers to this query, because
those ports might also consider supporting target_clones in the future.

In doing the work, I noticed that some of the functions called by the target
hooks that were fairly generic, and we might want to move these functions into
common code?  Would people object if I put out a patch to move these functions
to common code?

I also have a question on the functionality of target_clone that I will address
in a separate message.

So far, the list of generic code that could be moved to common code to allow
other ports to add target_clone support include:

make_resolver_func:

	Make the resolver function decl to dispatch the versions of a
	multi-versioned function, DEFAULT_DECL.  Create an empty basic block in
	the resolver and store the pointer in EMPTY_BB.  Return the decl of the
	resolver function.

make_dispatcher_decl:

	Make a dispatcher declaration for the multi-versioned function DECL.
	Calls to DECL function will be replaced with calls to the dispatcher by
	the front-end.  Return the decl created.

is_function_default_version:

	Returns true if decl is multi-versioned and DECL is the default
	function, that is it is not tagged with target specific optimization.

attr_strcmp:

	Comparator function to be used in qsort routine to sort attribute
	specification strings to "target".

sorted_attr_string:

	ARGLIST is the argument to target attribute.  This function tokenizes
	the comma separated arguments, sorts them and returns a string which is
	a unique identifier for the comma separated arguments.  It also
	replaces non-identifier characters "=,-" with "_".

<xxx>_function_versions:

	This function returns true if FN1 and FN2 are versions of the same
	function, that is, the target strings of the function decls are
	different.  This assumes that FN1 and FN2 have the same signature.
	This is the TARGET_OPTION_FUNCTION_VERSIONS target hook, and it is the
	same between the x86 and ppc.

-- 
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

Re: GCC target_clone support (functionality question)

[Michael Meissner]

This message is separated from the question about moving code, as it is a
questions about the functionality of target_clone support.

Right now it looks like target_clone only generates the ifunc handler if there
is a call to the function in the object file.  It does not generate the ifunc
handler if there is no call.

For the default function, it generates the normal name.  This means that any
function that calls the function from a different object module will only get
the standard function.  From a library and user perspective, I think this is
wrong.  Instead the default function should be generated with a different name,
and the ifunc function should list the standard name.  Then you don't have to
change all of the other calls in the object file, the normal ifunc handling
will handle it.  It also means you can more easily put this function in a
library and automatically call the appropriate version.

Do people agree with this assessment, and should I change the code to do this
(for both x86 nd ppc targets)?  If not, what is the reason for the objection?

Consider mvc5.c from the testsuite:

	/* { dg-do compile } */
	/* { dg-require-ifunc "" } */
	/* { dg-options "-fno-inline" } */
	/* { dg-final { scan-assembler-times "foo.ifunc" 6 } } */

	__attribute__((target_clones("default","avx","avx2")))
	int
	foo ()
	{
	  return 10;
	}

	__attribute__((target_clones("default","avx","avx2")))
	int
	bar ()
	{
	  return -foo ();
	}

It generates:

		.file	"mvc5.c"
		.text
		.p2align 4,,15
		.globl	foo
		.type	foo, @function
	foo:
		movl	$10, %eax
		ret

		.type	foo.avx.2, @function
	foo.avx.2:
		movl	$10, %eax
		ret

		.type	foo.avx2.3, @function
	foo.avx2.3:
		movl	$10, %eax
		ret

		.weak	foo.resolver
		.type	foo.resolver, @function
	foo.resolver:
		subq	$8, %rsp
		call	__cpu_indicator_init
		movl	__cpu_model+12(%rip), %eax
		testb	$4, %ah
		je	.L8
		movl	$foo.avx2.3, %eax
		addq	$8, %rsp
		ret
	.L8:
		testb	$2, %ah
		movl	$foo.avx.2, %edx
		movl	$foo, %eax
		cmovne	%rdx, %rax
		addq	$8, %rsp
		ret

		.type	foo.ifunc, @gnu_indirect_function
		.set	foo.ifunc,foo.resolver

	// Note these functions are not referenced

		.type	bar.avx2.1, @function
	bar.avx2.1:
		subq	$8, %rsp
		xorl	%eax, %eax
		call	foo.ifunc
		addq	$8, %rsp
		negl	%eax
		ret

		.type	bar.avx.0, @function
	bar.avx.0:
		subq	$8, %rsp
		xorl	%eax, %eax
		call	foo.ifunc
		addq	$8, %rsp
		negl	%eax
		ret

		.type	bar, @function

	// Note how it calls foo.ifunc instead of foo.

	bar:
		subq	$8, %rsp
		xorl	%eax, %eax
		call	foo.ifunc
		addq	$8, %rsp
		negl	%eax
		ret


Now, if I remove the bar call, and just leave foo it generates:

		.type	foo, @function
	foo:
		movl	$10, %eax
		ret

		.type	foo.avx.0, @function
	foo.avx.0:
		movl	$10, %eax
		ret

		.type	foo.avx2.1, @function
	foo.avx2.1:
		movl	$10, %eax
		ret

Note, it does not generate the resolver at all.

-- 
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

Re: GCC target_clone support (functionality question)

[Michael Meissner]

On Fri, May 05, 2017 at 12:32:03PM -0700, Evgeny Stupachenko wrote:
> Hi Michael,
> 
> On Fri, May 5, 2017 at 11:45 AM, Michael Meissner
> <meissner@linux.vnet.ibm.com> wrote:
> > This message is separated from the question about moving code, as it is a
> > questions about the functionality of target_clone support.
> >
> > Right now it looks like target_clone only generates the ifunc handler if there
> > is a call to the function in the object file.  It does not generate the ifunc
> > handler if there is no call.
> >
> > For the default function, it generates the normal name.  This means that any
> > function that calls the function from a different object module will only get
> > the standard function.  From a library and user perspective, I think this is
> > wrong.  Instead the default function should be generated with a different name,
> > and the ifunc function should list the standard name.  Then you don't have to
> > change all of the other calls in the object file, the normal ifunc handling
> > will handle it.  It also means you can more easily put this function in a
> > library and automatically call the appropriate version.
> 
> I think library issue could be resolved using special headers. You can
> look into GLIBC, there should be similar technique.
> When you call resolver from initial (default) function, you make an
> assumption that ifunc is supported.

Yes, ifunc should be required before you even consider using target_clone.

What I'm trying to do is make it easier for people to add target clones to
their code, but not having to go through using special headers or other gunk.
I want them to be able to add just the target clone line, and everything will
work.  That means in the current case, the default case should be renamed to
<function>.default, and what is <function>.ifunc should become <function>.

-- 
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

Re: GCC target_clone support (functionality question)

[Michael Meissner]

On Fri, May 05, 2017 at 01:38:03PM -0700, Evgeny Stupachenko wrote:
> On Fri, May 5, 2017 at 12:48 PM, Michael Meissner
> <meissner@linux.vnet.ibm.com> wrote:
> > On Fri, May 05, 2017 at 12:32:03PM -0700, Evgeny Stupachenko wrote:
> >> Hi Michael,
> >>
> >> On Fri, May 5, 2017 at 11:45 AM, Michael Meissner
> >> <meissner@linux.vnet.ibm.com> wrote:
> >> > This message is separated from the question about moving code, as it is a
> >> > questions about the functionality of target_clone support.
> >> >
> >> > Right now it looks like target_clone only generates the ifunc handler if there
> >> > is a call to the function in the object file.  It does not generate the ifunc
> >> > handler if there is no call.
> >> >
> >> > For the default function, it generates the normal name.  This means that any
> >> > function that calls the function from a different object module will only get
> >> > the standard function.  From a library and user perspective, I think this is
> >> > wrong.  Instead the default function should be generated with a different name,
> >> > and the ifunc function should list the standard name.  Then you don't have to
> >> > change all of the other calls in the object file, the normal ifunc handling
> >> > will handle it.  It also means you can more easily put this function in a
> >> > library and automatically call the appropriate version.
> >>
> >> I think library issue could be resolved using special headers. You can
> >> look into GLIBC, there should be similar technique.
> >> When you call resolver from initial (default) function, you make an
> >> assumption that ifunc is supported.
> >
> > Yes, ifunc should be required before you even consider using target_clone.
> 
> If function foo() called using regular name it usually means that
> there is no information about ifunc support.

The whole point of ifunc is that the caller should not need to know whether a
function is a normal function or an ifunc function.  The linker handles it all
making it go through the PLT like it does for shared library functions.

I don't understand what you mean by this:

> Another object file which
> is built with target_clones will have foo() definition with
> foo.resolver inside.

And for users outside of GLIBC, it is more cumbersome to have to have the whole
infrastructure to build separate versions of the file, and then write separate
ifunc declaration.

-- 
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

Re: GCC target_clone support (functionality question)

[Jeff Law]

On 05/06/2017 12:44 AM, Richard Sandiford wrote:
> Michael Meissner <meissner@linux.vnet.ibm.com> writes:
>> This message is separated from the question about moving code, as it is a
>> questions about the functionality of target_clone support.
>>
>> Right now it looks like target_clone only generates the ifunc handler if there
>> is a call to the function in the object file.  It does not generate the ifunc
>> handler if there is no call.
>>
>> For the default function, it generates the normal name.  This means that any
>> function that calls the function from a different object module will only get
>> the standard function.  From a library and user perspective, I think this is
>> wrong.  Instead the default function should be generated with a different name,
>> and the ifunc function should list the standard name.  Then you don't have to
>> change all of the other calls in the object file, the normal ifunc handling
>> will handle it.  It also means you can more easily put this function in a
>> library and automatically call the appropriate version.
> 
> Yeah, that sounds much more useful.  One thing on my wish list is to
> support target_clone for Advanced SIMD vs. SVE on AArch64 (although
> it's unlikely that'll happen in the GCC 8 timeframe).  I'd naively
> assumed it would already work in the way you suggested.
Likewise.
jeff

Re: GCC target_clone support

[Michael Meissner]

A minor feature that also should be considered is if you have two clone
functions, one that calls the other, we should optimize the call to avoid using
the indirect call setup by ifunc.

I.e.

	extern __attribute__((target_clones("default","avx","avx2"))) int caller ();
	extern __attribute__((target_clones("default","avx","avx2"))) int callee ();

	__attribute__((target_clones("default","avx","avx2")))
	int caller (void)
	{
	  return -callee ();
	}

	__attribute__((target_clones("default","avx","avx2")))
	int callee (void)
	{
	  return 10;
	}

I.e. caller.avx should call callee.avx, not callee (or callee.ifunc), and
caller.avx2 should call callee.avx2.  Do people think this is useful?

-- 
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

Re: GCC target_clone support

[Jeff Law]

On 05/05/2017 12:18 PM, Michael Meissner wrote:
> I'm in the middle of adding support for the target_clone attribute to the
> PowerPC.  At the moment, the x86_64/i386 port is the only port that supports
> target_clone.  I added the arm/s390 port maintainers to this query, because
> those ports might also consider supporting target_clones in the future.
> 
> In doing the work, I noticed that some of the functions called by the target
> hooks that were fairly generic, and we might want to move these functions into
> common code?  Would people object if I put out a patch to move these functions
> to common code?
Sounds wise to me -- as you know its not unusual to find commonality 
when a feature is enabled on a new target.


> 
> I also have a question on the functionality of target_clone that I will address
> in a separate message.
> 
> So far, the list of generic code that could be moved to common code to allow
> other ports to add target_clone support include:Please consider this class of changes pre-approved.  There's no reason 
to wait for review as you move this stuff around and generalize it slightly.

jeff