Re: [FFmpeg-devel] [PATCH 03/11] avutil: detect when AVX-512 is available

[James Darnley]

On 2017-11-10 03:11, James Almer wrote:
> On 11/9/2017 8:58 AM, James Darnley wrote:
>> @@ -154,6 +155,13 @@ int ff_get_cpu_flags_x86(void)
>>  if (ebx & 0x0100)
>>  rval |= AV_CPU_FLAG_BMI2;
>>  }
>> +#if HAVE_AVX512 /* F, CD, BW, DQ, VL */
> 
> Nit: Maybe move this chunk inside the HAVE_AVX2 block, at the end of it,
> right above the BMI checks.

I am preparing a final patch set and I forgot to ask you this at the
time.  Do you want me to put the new check inside the AVX2 if(){} block
too? Or just in the preprocessor #if #endif?

>> +if ((xcr0_lo & 0xe6) == 0xe6) {
> 
> Nit: The proper check here i think would be
> 
> if ((xcr0_lo & 0xe0) == 0xe0) {
> if ((rval & AV_CPU_FLAG_AVX) && (ebx & 0xd003) == 0xd003)
> rval |= AV_CPU_FLAG_AVX512;
> }
> 
> But it's functionally the same.

Ah.  Make it dependent on previous checks that have already checked for
features.  Did you mean AVX or AVX2 in this "(rval & AV_CPU_FLAG_AVX)"?

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 03/11] avutil: detect when AVX-512 is available

[James Almer]

On 11/9/2017 8:58 AM, James Darnley wrote:
> ---
> I've changed this patch slightly because I discovered that it would cause an
> illegal instruction exception on much older processors (probably all without
> AVX).  I was running xgetbv() almost uncontitionally.  Now it is a little more
> like what is the in x264 patch.
> 
>  libavutil/x86/cpu.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/libavutil/x86/cpu.c b/libavutil/x86/cpu.c
> index f33088c8c7..589fdef7da 100644
> --- a/libavutil/x86/cpu.c
> +++ b/libavutil/x86/cpu.c
> @@ -97,6 +97,7 @@ int ff_get_cpu_flags_x86(void)
>  int max_std_level, max_ext_level, std_caps = 0, ext_caps = 0;
>  int family = 0, model = 0;
>  union { int i[3]; char c[12]; } vendor;
> +int xcr0_lo = 0, xcr0_hi = 0;
>  
>  if (!cpuid_test())
>  return 0; /* CPUID not supported */
> @@ -132,8 +133,8 @@ int ff_get_cpu_flags_x86(void)
>  /* Check OXSAVE and AVX bits */
>  if ((ecx & 0x1800) == 0x1800) {
>  /* Check for OS support */
> -xgetbv(0, eax, edx);
> -if ((eax & 0x6) == 0x6) {
> +xgetbv(0, xcr0_lo, xcr0_hi);
> +if ((xcr0_lo & 0x6) == 0x6) {
>  rval |= AV_CPU_FLAG_AVX;
>  if (ecx & 0x1000)
>  rval |= AV_CPU_FLAG_FMA3;
> @@ -154,6 +155,13 @@ int ff_get_cpu_flags_x86(void)
>  if (ebx & 0x0100)
>  rval |= AV_CPU_FLAG_BMI2;
>  }
> +#if HAVE_AVX512 /* F, CD, BW, DQ, VL */

Nit: Maybe move this chunk inside the HAVE_AVX2 block, at the end of it,
right above the BMI checks.

> +if ((xcr0_lo & 0xe6) == 0xe6) {

Nit: The proper check here i think would be

if ((xcr0_lo & 0xe0) == 0xe0) {
if ((rval & AV_CPU_FLAG_AVX) && (ebx & 0xd003) == 0xd003)
rval |= AV_CPU_FLAG_AVX512;
}

But it's functionally the same.

> +if ((ebx & 0xd003) == 0xd003)
> +rval |= AV_CPU_FLAG_AVX512;
> +
> +}
> +#endif
>  }
>  
>  cpuid(0x8000, max_ext_level, ebx, ecx, edx);
> 

LGTM either way.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 03/11] avutil: detect when AVX-512 is available

[James Darnley]

---
I've changed this patch slightly because I discovered that it would cause an
illegal instruction exception on much older processors (probably all without
AVX).  I was running xgetbv() almost uncontitionally.  Now it is a little more
like what is the in x264 patch.

 libavutil/x86/cpu.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/libavutil/x86/cpu.c b/libavutil/x86/cpu.c
index f33088c8c7..589fdef7da 100644
--- a/libavutil/x86/cpu.c
+++ b/libavutil/x86/cpu.c
@@ -97,6 +97,7 @@ int ff_get_cpu_flags_x86(void)
 int max_std_level, max_ext_level, std_caps = 0, ext_caps = 0;
 int family = 0, model = 0;
 union { int i[3]; char c[12]; } vendor;
+int xcr0_lo = 0, xcr0_hi = 0;
 
 if (!cpuid_test())
 return 0; /* CPUID not supported */
@@ -132,8 +133,8 @@ int ff_get_cpu_flags_x86(void)
 /* Check OXSAVE and AVX bits */
 if ((ecx & 0x1800) == 0x1800) {
 /* Check for OS support */
-xgetbv(0, eax, edx);
-if ((eax & 0x6) == 0x6) {
+xgetbv(0, xcr0_lo, xcr0_hi);
+if ((xcr0_lo & 0x6) == 0x6) {
 rval |= AV_CPU_FLAG_AVX;
 if (ecx & 0x1000)
 rval |= AV_CPU_FLAG_FMA3;
@@ -154,6 +155,13 @@ int ff_get_cpu_flags_x86(void)
 if (ebx & 0x0100)
 rval |= AV_CPU_FLAG_BMI2;
 }
+#if HAVE_AVX512 /* F, CD, BW, DQ, VL */
+if ((xcr0_lo & 0xe6) == 0xe6) {
+if ((ebx & 0xd003) == 0xd003)
+rval |= AV_CPU_FLAG_AVX512;
+
+}
+#endif
 }
 
 cpuid(0x8000, max_ext_level, ebx, ecx, edx);
-- 
2.15.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel