[PATCH] Do not allow target_clones with alias attr (PR lto/90500).

[Martin Liška]

On 5/16/19 1:42 PM, Richard Biener wrote:
> On Thu, May 16, 2019 at 1:38 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 5/16/19 1:24 PM, Richard Biener wrote:
>>> On Thu, May 16, 2019 at 1:18 PM Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>> Hi.
>>>>
>>>> We should not allow target_clones being combined with alias attribute.
>>>>
>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>
>>>> Ready to be installed?
>>>
>>> So that's because an alias cannot be turned into a dispatcher and still
>>> be an alias, correct?  So a way around this would be to turn the
>>> alias into the dispatcher and clone the alias target, leaving the
>>> plain alias target as default variant not going through the dispatcher?
>>
>> We do allow having an alias to a target clone symbol:
>>
>> __attribute__((target_clones("arch=haswell", "default"))) int __tanh() {}
>> __typeof(__tanh) tanhf64 __attribute__((alias("__tanh")));
>>
>> Having that tanhf64 points to the resolver, which I believe is correct.
> 
> In this case yes.  I think the case in the PR wants to have an alias
> to the default variant instead and that's not possible so it tries to
> do the cloning on an alias (basically tell cloning to use an alternate
> symbol name for the resolver, leaving the default in place).  IMHO
> a reasonable feature, not sure if a reasonable way to achieve.

I see. Agree with you that it can be handy. On the other hand, one can use target
attribute:

__attribute__((target("avx","arch=core-avx2")))
int
bar ()
{
  return 2;
}

__attribute__((target("default")))
int
bar ()
{
  return 2;
}

int barrr () __attribute__((alias("_Z3barv")));

Which directly identifies a concrete implementation.

Martin

> 
>> The PR is about an alias that has itself target_clone attribute, which
>> does not make sense.
>>
>>>
>>> Of course in the testcase the body of the alias target isn't available
>>> but that's a general issue, not special to aliases?
>>
>> We do the target_clone expansion just for node->definition which is true
>> for node->alias == true symbols.
> 
> I see.  I guess it should be done for node->analyzed only, but yes,
> w/o considering aliases or thunks all definitions have bodies.
> 
> Richard.
> 
>> Martin
>>
>>>
>>> Richard.
>>>
>>>> Thanks,
>>>> Martin
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2019-05-16  Martin Liska  <mliska@suse.cz>
>>>>
>>>>         PR lto/90500
>>>>         * multiple_target.c (expand_target_clones): Do not allow
>>>>         target_clones being used with a symbol that is an alias.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2019-05-16  Martin Liska  <mliska@suse.cz>
>>>>
>>>>         PR lto/90500
>>>>         * gcc.target/i386/pr90500-1.c: New test.
>>>>         * gcc.target/i386/pr90500-2.c: New test.
>>>> ---
>>>>  gcc/multiple_target.c                     | 5 ++++-
>>>>  gcc/testsuite/gcc.target/i386/pr90500-1.c | 8 ++++++++
>>>>  gcc/testsuite/gcc.target/i386/pr90500-2.c | 7 +++++++
>>>>  3 files changed, 19 insertions(+), 1 deletion(-)
>>>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr90500-1.c
>>>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr90500-2.c
>>>>
>>>>
>>