RE: [PATCH v2 1/1] mldev: split bfloat16 routines to separate files

[Ruifeng Wang <Ruifeng.Wang@arm.com>]

> -----Original Message-----
> From: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Sent: Monday, March 13, 2023 8:03 PM
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: dev@dpdk.org; sshankarnara@marvell.com; david.marchand@redhat.com
> Subject: [PATCH v2 1/1] mldev: split bfloat16 routines to separate files
> 
> Since bfloat16 intrinsics are not supported on all ARM platforms that support NEON,
> bfloat16 routines are moved to separate files.
> This would enable using scalar implementation for bfloat16 on unsupported ARM platforms.
> 
> Bugzilla ID: 1179
> Fixes: fc54766b1612 ("mldev: add Arm NEON type conversion")
> 
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> ---
> Depends-on: patch-120653 ("mldev: remove weak symbols use in type conversions")
> Depends-on: patch-125035 ("mldev: fix identical code in conditional branches")
> 
>  lib/mldev/meson.build                   |  11 +-
>  lib/mldev/mldev_utils_neon.c            | 142 +------------
>  lib/mldev/mldev_utils_neon_bfloat16.c   | 154 ++++++++++++++
>  lib/mldev/mldev_utils_scalar.c          | 262 +-----------------------
>  lib/mldev/mldev_utils_scalar.h          |  80 ++++++++
>  lib/mldev/mldev_utils_scalar_bfloat16.c | 197 ++++++++++++++++++
>  6 files changed, 445 insertions(+), 401 deletions(-)  create mode 100644
> lib/mldev/mldev_utils_neon_bfloat16.c
>  create mode 100644 lib/mldev/mldev_utils_scalar.h  create mode 100644
> lib/mldev/mldev_utils_scalar_bfloat16.c
> 
> diff --git a/lib/mldev/meson.build b/lib/mldev/meson.build index c9db42257b..5769b0640a
> 100644
> --- a/lib/mldev/meson.build
> +++ b/lib/mldev/meson.build
> @@ -7,12 +7,21 @@ sources = files(
>          'mldev_utils.c',
>  )
> 
> -if dpdk_conf.has('RTE_ARCH_ARM64')
> +if (dpdk_conf.has('RTE_ARCH_ARM64') and
> +    cc.get_define('__ARM_NEON', args: machine_args) != '')

I found in ACLE document that "__ARM_NEON" is always set to 1 for AArch64".
So this line of check is redundant?

>      sources += files('mldev_utils_neon.c')  else
>      sources += files('mldev_utils_scalar.c')  endif
> 
> +if (dpdk_conf.has('RTE_ARCH_ARM64') and
> +    cc.get_define('__ARM_NEON', args: machine_args) != '' and

Same here.

> +    cc.get_define('__ARM_FEATURE_BF16', args: machine_args) != '')
> +    sources += files('mldev_utils_neon_bfloat16.c')
> +else
> +    sources += files('mldev_utils_scalar_bfloat16.c')
> +endif
> +
>  headers = files(
>          'rte_mldev.h',
>  )
<snip>