Re: [FFmpeg-devel] [PATCH] lavu/riscv: Fallback to getauxval() for cpu flags

["Rémi Denis-Courmont" <remi@remlab.net>]

Le tiistaina 30. heinäkuuta 2024, 17.07.52 EEST Nathan E. Egge a écrit :
> Signed-off-by: Nathan E. Egge <unlord@xiph.org>
> ---
>  libavutil/riscv/cpu.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/libavutil/riscv/cpu.c b/libavutil/riscv/cpu.c
> index e035f4b024..13b939b9d9 100644
> --- a/libavutil/riscv/cpu.c
> +++ b/libavutil/riscv/cpu.c
> @@ -90,7 +90,8 @@ int ff_get_cpu_flags_riscv(void)
>              default:
>          }
>      }
> -#elif HAVE_GETAUXVAL
> +#endif
> +#if HAVE_GETAUXVAL

I don't see the point in doing that. If <sys/hwprobe.h> and/or <asm/hwprobe.h> 
exist, the target is the upstream GNU/Linux ABI. That being the case, falling 
back to the auxillary vector is pointless, as noted in previous commits.

The Canaan (5.10) and SpacemiT (6.1) vendor forks of the Linux kernel do not 
have those two headers and their SDK should take the HAVE_GETAUXVAL branch as 
things stand already.

The `elif` there is meant to drop nominally useless code whilst also not 
mistaking the XTheadVector extension for proper RVV on T-Head vendor kernels. 
This patch reintroduces the problem.

>      {
>          const unsigned long hwcap = getauxval(AT_HWCAP);
> 
> @@ -100,14 +101,16 @@ int ff_get_cpu_flags_riscv(void)
>              ret |= AV_CPU_FLAG_RVF;
>          if (hwcap & HWCAP_RV('D'))
>              ret |= AV_CPU_FLAG_RVD;
> -        if (hwcap & HWCAP_RV('B'))
> -            ret |= AV_CPU_FLAG_RVB_ADDR | AV_CPU_FLAG_RVB_BASIC |
> -                   AV_CPU_FLAG_RVB;

This would break B detection on the presumptive OpenBSD and FreeBSD ABIs.

>          /* The V extension implies all Zve* functional subsets */
>          if (hwcap & HWCAP_RV('V'))
>               ret |= AV_CPU_FLAG_RVV_I32 | AV_CPU_FLAG_RVV_I64
> 
>                    | AV_CPU_FLAG_RVV_F32 | AV_CPU_FLAG_RVV_F64;
> 
> +
> +        /* The V extension implies Zb* on all existing hardware */
> +        if (hwcap & (HWCAP_RV('B') | HWCAP_RV('V')))
> +            ret |= AV_CPU_FLAG_RVB_ADDR | AV_CPU_FLAG_RVB_BASIC |
> +                   AV_CPU_FLAG_RVB;

Normally I would say that this belongs in a separate commit. But this 
contradicts the ISA specifications as well as the semantics Linux interfaces 
and, again, the presumptive BSD interfaces.

This does work on all of the two bits of *contemporary* commercially available 
*silicon*. This does breaks some correct emulator and simulator 
implementations though, and we have no warranties that this won't break some 
real hardware in the future. And indeed, this would break pure RVA22 hardware.

>      }
>  #endif

-- 
レミ・デニ-クールモン
http://www.remlab.net/



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".