On Wed, May 2, 2012 at 9:05 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Wed, May 2, 2012 at 8:08 AM, Sriraman Tallam <tmsri...@google.com> wrote:
>> On Wed, May 2, 2012 at 6:42 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>> On Tue, May 1, 2012 at 7:45 PM, Sriraman Tallam <tmsri...@google.com> wrote:
>>>> Hi H.J,
>>>>
>>>> Done now. Patch attached.
>>>>
>>>> Thanks,
>>>> -Sri.
>>>>
>>>> On Tue, May 1, 2012 at 5:08 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>>>> On Tue, May 1, 2012 at 4:51 PM, Sriraman Tallam <tmsri...@google.com>
>>>>> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> New patch attached, updated test case and fixed bugs related to
>>>>>> __PRETTY_FUNCTION_.
>>>>>>
>>>>>> Patch also available for review here:
>>>>>> http://codereview.appspot.com/5752064
>>>>>
>>>>> @@ -0,0 +1,39 @@
>>>>> +/* Simple test case to check if Multiversioning works. */
>>>>> +/* { dg-do run } */
>>>>> +/* { dg-options "-O2 -fPIC" } */
>>>>> +
>>>>> +#include <assert.h>
>>>>> +
>>>>> +int foo ();
>>>>> +int foo () __attribute__ ((target("arch=corei7,sse4.2,popcnt")));
>>>>> +/* The target operands in this declaration and the definition are
>>>>> re-ordered.
>>>>> + This should still work. */
>>>>> +int foo () __attribute__ ((target("ssse3,avx2")));
>>>>> +
>>>>> +int (*p)() = &foo;
>>>>> +int main ()
>>>>> +{
>>>>> + return foo () + (*p)();
>>>>> +}
>>>>> +
>>>>> +int foo ()
>>>>> +{
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int __attribute__ ((target("arch=corei7,sse4.2,popcnt")))
>>>>> +foo ()
>>>>> +{
>>>>> + assert (__builtin_cpu_is ("corei7")
>>>>> + && __builtin_cpu_supports ("sse4.2")
>>>>> + && __builtin_cpu_supports ("popcnt"));
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int __attribute__ ((target("avx2,ssse3")))
>>>>> +foo ()
>>>>> +{
>>>>> + assert (__builtin_cpu_supports ("avx2")
>>>>> + && __builtin_cpu_supports ("ssse3"));
>>>>> + return 0;
>>>>> +}
>>>>>
>>>>> This test will pass if
>>>>>
>>>>> int foo ()
>>>>> {
>>>>> return 0;
>>>>> }
>>>>>
>>>>> is selected on processors with AVX. The run-time test should
>>>>> check that the right function is selected on the target processor,
>>>>> not the selected function matches the target attribute. You can
>>>>> do it by returning different values for each foo and call cpuid
>>>>> to check if the right foo is selected.
>>>>>
>>>>> You should add a testcase for __builtin_cpu_supports to check
>>>>> all valid arguments.
>>>>>
>>>>> --
>>>>> H.J.
>>>
>>> 2 questions:
>>>
>>> 1. Since AVX > SSE4 > SSSE3 > SSE3 > SSE2 > SSE, with
>>> foo for AVX and SSE3, on AVX processors, which foo will be
>>> selected?
>>
>> foo for AVX will get called since that appears ahead.
>>
>> The dispatching is done in the same order in which the functions are
>> specified. If, potentially, two foo versions can be dispatched for an
>> architecture, the first foo will get called. There is no way right
>> now to specify the order in which the dispatching should be done.
>
> This is very fragile. We know ISAs and processors. The source
> order should be irrelevant.
I am not sure it is always possible keep this dispatching unambiguous
to the user. It might be better to let the user specify a priority for
each version to control the order of dispatching.
Still, one way to implement what you said is to assign a significance
number to each ISA, where the number of sse4 > sse, for instance.
Then, the dispatching can be done in the descending order of
significance. What do you think?
I thought about this earlier and I was thinking along the lines of
letting the user specify a priority for each version, when there is
ambiguity.
>
>>
>>> 2. I don't see any tests for __builtin_cpu_supports ("XXX")
>>> nor __builtin_cpu_is ("XXX"). I think you need tests for
>>> them.
>>
>> This is already there as part of the previous CPU detection patch that
>> was submitted. Please see gcc.target/i386/builtin_target.c. Did you
>> want something else?
>
> gcc.target/i386/builtin_target.c doesn't test if __builtin_cpu_supports
> ("XXX")
> and __builtin_cpu_is ("XXX") are implemented correctly.
Oh, you mean like doing a CPUID again in the test case itself and checking, ok.
Thanks,
-Sri.
>
>
> --
> H.J.
