This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] New attribute to create target clones
- From: Evgeny Stupachenko <evstupac at gmail dot com>
- To: Jan Hubicka <hubicka at ucw dot cz>
- Cc: Jeff Law <law at redhat dot com>, Bernd Schmidt <bschmidt at redhat dot com>, Bernd Schmidt <bernds_cb1 at t-online dot de>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 15 Oct 2015 00:32:12 +0300
- Subject: Re: [PATCH] New attribute to create target clones
- Authentication-results: sourceware.org; auth=none
- References: <5601C375 dot 5050706 at redhat dot com> <CAOvf_xwUK1k=zx=f7eAWs4xs2J4KBCZGzeom_a5SfdkfF_gqSQ at mail dot gmail dot com> <CAOvf_xy6Kq_AGArurgxYC65K=LMNt6_gXHK15VFsoCd64F+TTA at mail dot gmail dot com> <5616BD37 dot 2000307 at redhat dot com> <20151008192349 dot GB90964 at kam dot mff dot cuni dot cz> <5616C9CA dot 2010702 at redhat dot com> <20151008213644 dot GD5527 at kam dot mff dot cuni dot cz> <5617FD1D dot 9010406 at redhat dot com> <20151009182753 dot GA7750 at kam dot mff dot cuni dot cz> <CAOvf_xyO5_s6ZF56SErj24N8ARbSHEH_nHAKfKFa1uWX+T9xiQ at mail dot gmail dot com> <20151009200422 dot GA23615 at kam dot mff dot cuni dot cz> <CAOvf_xzpEV=q7Mi6=xEr=cSV8D0NOcpeYR8mJgAgkz5kocuYQg at mail dot gmail dot com> <CAOvf_xxv3uuGwTtmy40qKxUoi+oBCRaN1KU7tZD=hd5VRMAZng at mail dot gmail dot com>
Bootstrap and make check for x86 passed. No new fails.
Please ignore an empty line added to omp-low.c in the patch, the
misprint will be removed prior to a commit.
Thanks,
Evgeny
On Tue, Oct 13, 2015 at 2:35 AM, Evgeny Stupachenko <evstupac@gmail.com> wrote:
> 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