On Mon, Aug 3, 2015 at 9:43 PM, Jeff Law <l...@redhat.com> wrote:
> On 07/30/2015 04:19 PM, Evgeny Stupachenko wrote:
>>
>> Hi All,
>>
>> The patch enables new attribute 'ctarget',
>> The attribute force compiler to create clones of a function with the
>> attribute.
>>
>> For example:
>> __attribute__((ctarget("avx","arch=slm","arch=core-avx2","default")))
>
> So presumably we're allowing both changing the ISA and the tuning options?
> In fact, it looks like we're able to change any -m option, right?
>
> What about something like -mregparm? I think the docs need to disallow
> clones with different ABI requirements.
-mregparm is not allowed now. The
targetm.target_option.valid_attribute_p hook specify which -m option
is allowed for architecture. Here patch reuses Function
Multiversioning methods.
>
>> Currently default ctarget means that foo() will be optimized with
>> current compiler options and target.
>> The other option is to switch target to target specific minimum (like
>> target x86-64). Is it better?
>
> I could make an argument for either. Do we have anything to guide us from
> other compilers such as ICC that may have a similar capability?
>
Not sure. However ICC has similar to Function Multiversioning:
__declcpec(cpu_specific(... where "default" is ""generic". I think for
"default" we should do the same as Function Multiversioning - keep
compiler options. That way users will be able to create target
specific minimum by passing corresponding options to command line.
>
>>
>> What do you think about attribute name? 'ctarget' is short but not
>> informative. Other variants are 'target_clones', 'targets'...
>
> target_clones seems good.
>
> For multiple_target.c: Can you please trim down the #include set. I can't
> believe you need all that stuff. If you really need backend stuff (tm.h),
> then use "backend.h" instead.
>
> It generally looks reasonable, but Jan knows the IPA code much better than I
> do and I'd like him to chime in. You might also ask Ilya to review the
> cloning and rules around clones since he head to deal with a lot of that
> stuff in his MPX work.
>
> jeff
