Re: [PATCH] New attribute to create target clones

[Evgeny Stupachenko <evstupac at gmail dot com>]

Hi All,

Here is a new version of patch (attached).
Bootstrap and make check are in progress (all new tests passed).

New test case g++.dg/ext/mvc4.C fails with ICE, when options lower
than "-mavx" are passed.
However it has the same behavior if "target_clones" attribute is
replaced by 2 corresponding "target" attributes.
I've filed PR67946 on this:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67946

Thanks,
Evgeny

ChangeLog:

2015-10-13  Evgeny Stupachenko  <evstupac@gmail.com>
gcc/
        * Makefile.in (OBJS): Add multiple_target.o.
        * attrib.c (make_attribute): Moved from config/i386/i386.c
        * config/i386/i386.c (make_attribute): Deleted.
        * multiple_target.c (make_attribute): New.
        (create_dispatcher_calls): Ditto.
        (get_attr_len): Ditto.
        (get_attr_str): Ditto.
        (is_valid_asm_symbol): Ditto.
        (create_new_asm_name): Ditto.
        (create_target_clone): Ditto.
        (expand_target_clones): Ditto.
        (ipa_target_clone): Ditto.
        (ipa_dispatcher_calls): Ditto.
        * passes.def (pass_target_clone): Two new ipa passes.
        * tree-pass.h (make_pass_target_clone): Ditto.

gcc/c-family
        * c-common.c (handle_target_clones_attribute): New.
        * (c_common_attribute_table): Add handle_target_clones_attribute.
        * (handle_always_inline_attribute): Add check on target_clones
        attribute.
        * (handle_target_attribute): Ditto.

gcc/testsuite
        * gcc.dg/mvc1.c: New test for multiple targets cloning.
        * gcc.dg/mvc2.c: Ditto.
        * gcc.dg/mvc3.c: Ditto.
        * gcc.dg/mvc4.c: Ditto.
        * gcc.dg/mvc5.c: Ditto.
        * gcc.dg/mvc6.c: Ditto.
        * gcc.dg/mvc7.c: Ditto.
        * g++.dg/ext/mvc1.C: Ditto.
        * g++.dg/ext/mvc2.C: Ditto.
        * g++.dg/ext/mvc3.C: Ditto.
        * g++.dg/ext/mvc4.C: Ditto.

gcc/doc
        * doc/extend.texi (target_clones): New attribute description.

On Sat, Oct 10, 2015 at 12:44 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> On Fri, Oct 9, 2015 at 11:04 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> On Fri, Oct 9, 2015 at 9:27 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> >> >Of course it also depends what you inline into function. You can have
>>> >> >
>>> >> >bar() target(-mavx) {fancy avx code}
>>> >> >foobar() { ...... if (avx) bar();}
>>> >> >foo() ctarget(-mavx,-mno-avx) {....foobar();....}
>>>
>>> "no-" targets are not supported
>>
>> Why not? I suppose I can use -march=x86_64 in a file compiled with -march=core-avx2 or something like that, too.
> Sure, you can. target(arch=x86-64) is ok. I mean exactly target(no-avx) returns:
>
> aaa.cpp: In function '<built-in>':
> aaa.cpp:7:5: error: No dispatcher found for no-avx
>  int bar()
>      ^
>
>>>
>>> >> >
>>> >> >Now if you compile with -mavx and because ctarget takes effect only after inlining,
>>> >> >at inlining time the target attributes will match and we can edn up inline bar->foobar->foo.
>>> >> >After that we multiversion foo and drop AVX flag we will likely get ICE at expansion
>>> >> >time.
>>> >> But isn't that avoided by fixing up the call graph so that all calls
>>> >> to the affected function are going through the dispatcher?  Or is
>>> >> that happening too late?
>>> >
>>> > There is dispatcher only for foo that is the root of the callgarph tree.
>>> > When inlining we compare target attributes for match (in can_inline_edge_p).
>>> > We do not compare ctarget attributes.  Expanding ctarget to target early would
>>> > avoid need for ctarget handling.
>>> Currently inlining is disabled for functions with target_clone attribute:
>>
>> Do you also disable inlining into functions with target_clone?
>> What I am concerned about is early inliner inlining (say) AVX code into ctarget
>> function because at early inlining time the target is not applied, yet.
> Right. Now I've got your point and ICE on the test.
> Yes the solution is to disable inline into target_clones function.
> Or to move the pass creating clones before inline (as you suggested)
> and leave dispatcher creator after inline.
>
> I like you suggestion. It fixes the ICE.
> I'll fix the patch and retest.
>
> Thank you for the review,
> Evgeny.
>
>
>>
>> Honza

Attachment: target_clones.patch
Description: Binary data