diff mbox

avx runtime check

Message ID 42629FC8-9372-410B-B9C0-7C6341B3D812@comcast.net
State New
Headers show

Commit Message

Mike Stump May 16, 2014, 2:47 a.m. UTC 
This reorders the avx checks and gates on a target triplet check before compiling any code.

Ok?

Comments

Richard Biener May 16, 2014, 6:52 a.m. UTC | #1 
On May 16, 2014 4:47:11 AM CEST, Mike Stump <mikestump@comcast.net> wrote:
>This reorders the avx checks and gates on a target triplet check before
>compiling any code.

Can you explain why?

>Ok?
>
>diff --git a/gcc/testsuite/lib/target-supports.exp
>b/gcc/testsuite/lib/target-supports.exp
>index 40b5414..103a28a 100644
>--- a/gcc/testsuite/lib/target-supports.exp
>+++ b/gcc/testsuite/lib/target-supports.exp
>@@ -1353,8 +1353,8 @@ proc check_effective_target_sse2_runtime { } {
># Return 1 if the target supports running AVX executables, 0 otherwise.
> 
> proc check_effective_target_avx_runtime { } {
>-    if { [check_effective_target_avx]
>-        && [check_avx_hw_available]
>+    if { [check_avx_hw_available]
>+        && [check_effective_target_avx]
>         && [check_avx_os_support_available] } {
>        return 1
>     }
Mike Stump May 16, 2014, 2:20 p.m. UTC | #2 
On May 15, 2014, at 11:52 PM, Richard Biener <richard.guenther@gmail.com> wrote:
> On May 16, 2014 4:47:11 AM CEST, Mike Stump <mikestump@comcast.net> wrote:
>> This reorders the avx checks and gates on a target triplet check before
>> compiling any code.
> 
> Can you explain why?

Sure, because check_avx_hw_available runs [istarget x86_64-*-*] || [istarget i?86-*-*] before doing anything, like compiling:

       typedef double __m512d __attribute__ ((__vector_size__ (64)));

        __m512d _mm512_add (__m512d a)
        {
          return __builtin_ia32_addpd512_mask (a, a, a, 1, 4);
        }

with -mavx512f, which my target doesn’t have, but even running a compilation of that seems wrong.  The other possibility would be to add in a:

        # If this is not the right target then we can skip the test.                                                                                                                  
        if { !([istarget x86_64-*-*] || [istarget i?86-*-*]) } {
            expr 0
        } else {

into the test for check_effective_target_avx512f.


proc check_effective_target_avx512f { } {
    return [check_no_compiler_messages avx512f object {
        typedef double __m512d __attribute__ ((__vector_size__ (64)));

        __m512d _mm512_add (__m512d a)
        {
          return __builtin_ia32_addpd512_mask (a, a, a, 1, 4);
        }
    } "-O2 -mavx512f" ]
}

proc check_avx_hw_available { } {
    return [check_cached_effective_target avx_hw_available {
        # If this is not the right target then we can skip the test.                                                                                                                  
        if { !([istarget x86_64-*-*] || [istarget i?86-*-*]) } {
            expr 0
        } else {
            check_runtime_nocache avx_hw_available {
                #include "cpuid.h"                                                                                                                                                    
                int main ()
                {
	          unsigned int eax, ebx, ecx, edx;
                  if (__get_cpuid (1, &eax, &ebx, &ecx, &edx))
	            return ((ecx & (bit_AVX | bit_OSXSAVE))
                            != (bit_AVX | bit_OSXSAVE));
                  return 1;
                }
            } ""
        }
    }]
}


>> diff --git a/gcc/testsuite/lib/target-supports.exp
>> b/gcc/testsuite/lib/target-supports.exp
>> index 40b5414..103a28a 100644
>> --- a/gcc/testsuite/lib/target-supports.exp
>> +++ b/gcc/testsuite/lib/target-supports.exp
>> @@ -1353,8 +1353,8 @@ proc check_effective_target_sse2_runtime { } {
>> # Return 1 if the target supports running AVX executables, 0 otherwise.
>> 
>> proc check_effective_target_avx_runtime { } {
>> -    if { [check_effective_target_avx]
>> -        && [check_avx_hw_available]
>> +    if { [check_avx_hw_available]
>> +        && [check_effective_target_avx]
>>        && [check_avx_os_support_available] } {
>>       return 1
>>    }
Richard Biener May 19, 2014, 8:47 a.m. UTC | #3 
On Fri, May 16, 2014 at 4:20 PM, Mike Stump <mikestump@comcast.net> wrote:
> On May 15, 2014, at 11:52 PM, Richard Biener <richard.guenther@gmail.com> wrote:
>> On May 16, 2014 4:47:11 AM CEST, Mike Stump <mikestump@comcast.net> wrote:
>>> This reorders the avx checks and gates on a target triplet check before
>>> compiling any code.
>>
>> Can you explain why?
>
> Sure, because check_avx_hw_available runs [istarget x86_64-*-*] || [istarget i?86-*-*] before doing anything, like compiling:
>
>        typedef double __m512d __attribute__ ((__vector_size__ (64)));
>
>         __m512d _mm512_add (__m512d a)
>         {
>           return __builtin_ia32_addpd512_mask (a, a, a, 1, 4);
>         }
>
> with -mavx512f, which my target doesn’t have, but even running a compilation of that seems wrong.  The other possibility would be to add in a:
>
>         # If this is not the right target then we can skip the test.
>         if { !([istarget x86_64-*-*] || [istarget i?86-*-*]) } {
>             expr 0
>         } else {
>
> into the test for check_effective_target_avx512f.

Yes, that looks like a better fix.

Ok with that change.

Thanks,
Richard.

>
> proc check_effective_target_avx512f { } {
>     return [check_no_compiler_messages avx512f object {
>         typedef double __m512d __attribute__ ((__vector_size__ (64)));
>
>         __m512d _mm512_add (__m512d a)
>         {
>           return __builtin_ia32_addpd512_mask (a, a, a, 1, 4);
>         }
>     } "-O2 -mavx512f" ]
> }
>
> proc check_avx_hw_available { } {
>     return [check_cached_effective_target avx_hw_available {
>         # If this is not the right target then we can skip the test.
>         if { !([istarget x86_64-*-*] || [istarget i?86-*-*]) } {
>             expr 0
>         } else {
>             check_runtime_nocache avx_hw_available {
>                 #include "cpuid.h"
>                 int main ()
>                 {
>                   unsigned int eax, ebx, ecx, edx;
>                   if (__get_cpuid (1, &eax, &ebx, &ecx, &edx))
>                     return ((ecx & (bit_AVX | bit_OSXSAVE))
>                             != (bit_AVX | bit_OSXSAVE));
>                   return 1;
>                 }
>             } ""
>         }
>     }]
> }
>
>
>>> diff --git a/gcc/testsuite/lib/target-supports.exp
>>> b/gcc/testsuite/lib/target-supports.exp
>>> index 40b5414..103a28a 100644
>>> --- a/gcc/testsuite/lib/target-supports.exp
>>> +++ b/gcc/testsuite/lib/target-supports.exp
>>> @@ -1353,8 +1353,8 @@ proc check_effective_target_sse2_runtime { } {
>>> # Return 1 if the target supports running AVX executables, 0 otherwise.
>>>
>>> proc check_effective_target_avx_runtime { } {
>>> -    if { [check_effective_target_avx]
>>> -        && [check_avx_hw_available]
>>> +    if { [check_avx_hw_available]
>>> +        && [check_effective_target_avx]
>>>        && [check_avx_os_support_available] } {
>>>       return 1
>>>    }
diff mbox

Patch

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 40b5414..103a28a 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -1353,8 +1353,8 @@  proc check_effective_target_sse2_runtime { } {
 # Return 1 if the target supports running AVX executables, 0 otherwise.
 
 proc check_effective_target_avx_runtime { } {
-    if { [check_effective_target_avx]
-        && [check_avx_hw_available]
+    if { [check_avx_hw_available]
+        && [check_effective_target_avx]
         && [check_avx_os_support_available] } {
        return 1
     }