diff mbox series

[FFmpeg-devel,v2,5/5] aarch64: Add Windows runtime detection of the dotprod instructions

Message ID 20230530123043.52940-5-martin@martin.st
State New
Headers show
Series [FFmpeg-devel,v2,1/5] configure: aarch64: Support assembling the dotprod and i8mm arch extensions | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Martin Storsjö May 30, 2023, 12:30 p.m. UTC
For Windows, there's no publicly defined constant for checking for
the i8mm extension yet.
---
 libavutil/aarch64/cpu.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Martin Storsjö June 3, 2023, 8:51 p.m. UTC | #1
On Tue, 30 May 2023, Martin Storsjö wrote:

> For Windows, there's no publicly defined constant for checking for
> the i8mm extension yet.
> ---
> libavutil/aarch64/cpu.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)

If there's no objections or further comments on this patchset, I'll push 
it early next week (with the unnecessary checks in patch 3/5 removed and a 
comment amended).

// Martin
James Zern June 5, 2023, 5:36 p.m. UTC | #2
On Tue, May 30, 2023 at 5:31 AM Martin Storsjö <martin@martin.st> wrote:
>
> For Windows, there's no publicly defined constant for checking for
> the i8mm extension yet.
> ---
>  libavutil/aarch64/cpu.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/libavutil/aarch64/cpu.c b/libavutil/aarch64/cpu.c
> index ffb00f6dd2..4b97530240 100644
> --- a/libavutil/aarch64/cpu.c
> +++ b/libavutil/aarch64/cpu.c
> @@ -94,6 +94,16 @@ static int detect_flags(void)
>      return flags;
>  }
>
> +#elif defined(_WIN32)
> +#include <windows.h>
> +
> +static int detect_flags(void)
> +{
> +    int flags = 0;
> +    if (IsProcessorFeaturePresent(PF_ARM_V82_DP_INSTRUCTIONS_AVAILABLE))

I think this requires a recent Windows SDK, is compatibility a concern?
lgtm otherwise.
Martin Storsjö June 6, 2023, 9:32 a.m. UTC | #3
On Mon, 5 Jun 2023, James Zern wrote:

> On Tue, May 30, 2023 at 5:31 AM Martin Storsjö <martin@martin.st> wrote:
>>
>> For Windows, there's no publicly defined constant for checking for
>> the i8mm extension yet.
>> ---
>>  libavutil/aarch64/cpu.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/libavutil/aarch64/cpu.c b/libavutil/aarch64/cpu.c
>> index ffb00f6dd2..4b97530240 100644
>> --- a/libavutil/aarch64/cpu.c
>> +++ b/libavutil/aarch64/cpu.c
>> @@ -94,6 +94,16 @@ static int detect_flags(void)
>>      return flags;
>>  }
>>
>> +#elif defined(_WIN32)
>> +#include <windows.h>
>> +
>> +static int detect_flags(void)
>> +{
>> +    int flags = 0;
>> +    if (IsProcessorFeaturePresent(PF_ARM_V82_DP_INSTRUCTIONS_AVAILABLE))
>
> I think this requires a recent Windows SDK, is compatibility a concern?

Good catch, thanks! Yes, this requires a fairly recent SDK; it's been 
added to mingw-w64 in 2021, and in the MS WinSDK since 10.0.22000.0. So 
this would indeed have failed in my own fate config with an older MSVC...

It's easy to wrap this up in an #ifdef though, since the identifier is a 
define (both in mingw-w64 and WinSDK).

Other than that, this code is only used for aarch64, so the target 
audience is much smaller than generic windows code, but nevertheless, 
let's not break older toolchains if we don't need to.

I'll amend the patch locally with an ifdef before pushing.

// Martin
diff mbox series

Patch

diff --git a/libavutil/aarch64/cpu.c b/libavutil/aarch64/cpu.c
index ffb00f6dd2..4b97530240 100644
--- a/libavutil/aarch64/cpu.c
+++ b/libavutil/aarch64/cpu.c
@@ -94,6 +94,16 @@  static int detect_flags(void)
     return flags;
 }
 
+#elif defined(_WIN32)
+#include <windows.h>
+
+static int detect_flags(void)
+{
+    int flags = 0;
+    if (IsProcessorFeaturePresent(PF_ARM_V82_DP_INSTRUCTIONS_AVAILABLE))
+        flags |= AV_CPU_FLAG_DOTPROD;
+    return flags;
+}
 #else
 
 static int detect_flags(void)