Re: [PATCH] [RFC] Proposal for implementing AArch64 port of libmvec

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 09/02/23 09:43, Joe Ramsay wrote:
> Thanks for the comments. I will attempt a patch that addresses them, in the meantime just a few questions:
> 
> On 08/02/2023 13:11, Adhemerval Zanella Netto wrote:
>>
>>
>> On 07/02/23 08:35, Joe Ramsay via Libc-alpha wrote:
>>> Hi,
>>>
>>> The attached patch is an attempt to enable libmvec on AArch64. The
>>> proposed change is mainly implementing build infrastructure to add the
>>> new routines to ABI, tests and benchmarks. I have demonstrated how
>>> this all fits together by adding implementations for vector cos, in
>>> both single and double precision, targeting both Advanced SIMD and
>>> SVE.
>>>
>>> The implementations of the routines themselves are just loops over the
>>> scalar routine from libm for now, as we are more concerned with
>>> getting the plumbing right at this point. We plan to contribute vector
>>> routines from the Arm Optimized Routines repo that are compliant with
>>> requirements described in the libmvec wiki.
>>>
>>> Any comments/thoughts much appreciated! In particular, the patch
>>> raises the minimum GCC to 10, in order to be able to submit routines
>>> written using ACLE instead of assembly. This is clearly a big jump,
>>> but we have options if this is not acceptable. One option would be to
>>> submit compiler-generated assembly, similar to the equivalent routines
>>> under sysdeps/x86_64. If GCC 9 is an acceptable compromise then this
>>> would only have to be for SVE routines.
>>
>> Using C implementation with intrinsics would be idea, there are more easily
>> maintained and can leverage compiler improvements.  I rather do it instead
>> of the assembly dump Intel did.
>>
>> The minimum GCC 10 is not ideal, however I don't see it as blocker either
>> (it should be up to arch-maintainers).  One option might be check if
>> compiler does not support building libmvec, disable the build and related
>> checks.  It is not ideal either, since the resulting glibc won't have
>> a complete ABI.
>>
>>
> OK, let's see what arch-maintainers make of it.
>>>
>>> Also, are there plans to merge libmvec into libm, or will they be kept
>>> separate?
>>
>> There is none afaik.  The libpthread, librt, etc. merge was done to
>> fix long standing design and maintanance issues that is not really presented
>> with libm and libmvec.  There is still the partial upgrade one, but
>> it is still present with a disjoint ld, libc, libm anyway.
>>
>> However, it is feasible to merge if your willing to work on it.  We will
>> need to keep the x86_64 lib with the sentinel compat symbol (similar to
>> what we did for libpthread).
>>
>> What I would like to avoid is to have different arquitectures using different
>> approaches, for instance aarch64 begin merged while having x86_64 still
>> using a different library.  It add a slight more complexity to the build
>> process and extra arch specific boilerplate code.
>>
> This sounds good - keeping them separate would be our choice too. I have not come across the sentinel compat symbol - is this something we need to do for AArch64 also?

The sentinel compat symbols are required for the case if you want to merge
x86 libmvec on libm, similar to what rt/librt-compat.c and nptl/libpthread-compat.c
does.  They are required so old binaries linked to libmvec does not fail at
loading time, since they will have libmvec as DT_NEEDED.

>>
>> I think these tests should move to configure tests instead, it advertises beforehand
>> the user that it needs to update the compiler instead through a compiler error.
>>
>> The configure check will then check for both advsimd and SVE support, so there is
>> no need for __ADVSIMD_VEC_MATH_SUPPORTED or __SVE_VEC_MATH_SUPPORTED.
>>
> Apologies, I don't quite understand what you mean by this. I put these tests in so that users could compile against the new symbols with math.h as long as they had a sufficiently new compiler, but wouldn't get undefined types in math.h if they were using an old compiler that didn't have e.g. __Float32x4_t. (I think I remarked in the original message that new symbols hadn't been added to math.h, but this was not correct).

Ah right, I missed that this is an *installed header*.  In this case the 
__GNUC_PREREQ does make sense (sorry for the noise).