[Mesa-dev] [PATCH 2/3] vc4: fix arm64 build with Neon

[Emil Velikov]

On 30 January 2017 at 22:54, Rob Herring <robh at kernel.org> wrote:
> The addition of Neon assembly breaks on arm64 builds because the assembly
> syntax is different. For now, restrict Neon to ARMv7 builds.
>
> Signed-off-by: Rob Herring <robh at kernel.org>
> ---
>  src/gallium/drivers/vc4/vc4_tiling.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/vc4/vc4_tiling.h b/src/gallium/drivers/vc4/vc4_tiling.h
> index 218130b2007c..ba1ad6fb3f7d 100644
> --- a/src/gallium/drivers/vc4/vc4_tiling.h
> +++ b/src/gallium/drivers/vc4/vc4_tiling.h
> @@ -87,7 +87,7 @@ void vc4_store_tiled_image(void *dst, uint32_t dst_stride,
>   * should extend this to have some runtime detection of being built for ARMv6
>   * on a Pi 2+.
>   */
> -#if defined(__ARM_ARCH) && __ARM_ARCH >= 7
> +#if defined(__ARM_ARCH) && __ARM_ARCH == 7
>  #define NEON_SUFFIX(x) x ## _neon
>  #else
>  #define NEON_SUFFIX(x) x ## _base
Things seem quite inconsistent in the area. Not sure if they are
left-overs from dev stage or I'm missing something subtle.

The functions are declared via NEON_TAG macro and used via
NEON_SUFFIX. Both macros seem identical, but... former is guarded by
defined(VC4_BUILD_NEON) alone, and the latter with defined(__ARM_ARCH)
&& __ARM_ARCH == 7. Additionally we have some assembly which is
guarded by defined(VC4_BUILD_NEON) && defined(__ARM_ARCH).
There's a comment about the latter, which is to keep the x86 simulator running.

Wouldn't it be better to have single heuristic used throughout,
something like the following may be reasonable.
#if defined(__ARM_ARCH) && __ARM_ARCH == 7
#define VC4_BUILD_NEON
#endif

One could even have a _base + _neon parts of vc4_load_utile and
vc4_store_utile (not 100% sure here) and then use a single NEON_SUFFIX
(maybe as below) to tag things throughout.

#ifdef VC4_BUILD_NEON
#define NEON_SUFFIX(x) x ## _neon
#else
#define NEON_SUFFIX(x) x ## _base
#endif


As a nice side effect you can then get rid of the libvc4_neon.la static library.

Thanks
Emil
P.S. Did you have some ideas to have separate winsys (similar to
virgl) - one for HW and another for SIM ?
Is there any progress in the area ?