Re: [PATCH] New attribute to create target clones

[Evgeny Stupachenko <evstupac@gmail.com>]

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