Re: [PATCH] amdgcn: Enable SIMD vectorization of math functions

[Andrew Stubbs <ams@codesourcery.com>]

On 01/03/2023 10:52, Andre Vieira (lists) wrote:
> 
> 
> On 01/03/2023 10:01, Andrew Stubbs wrote:
>  > On 28/02/2023 23:01, Kwok Cheung Yeung wrote:
>  >> Hello
>  >>
>  >> This patch implements the TARGET_VECTORIZE_BUILTIN_VECTORIZED_FUNCTION
>  >> target hook for the AMD GCN architecture, such that when vectorized,
>  >> calls to builtin standard math functions such as asinf, exp, pow etc.
>  >> are converted to calls to the recently added vectorized math functions
>  >> for GCN in Newlib. The -fno-math-errno flag is required in addition to
>  >> the usual vectorization optimization flags for this to occur, and some
>  >> of the math functions (the larger double-precision ones) require a
>  >> large stack size to function properly.
>  >>
>  >> This patch requires the GCN vector math functions in Newlib to
>  >> function - these were included in the recent 4.3.0.20230120 snapshot.
>  >> As this was a minimum requirement starting from the patch 'amdgcn,
>  >> libgomp: Manually allocated stacks', this should not be a problem.
>  >>
>  >> I have added new testcases in the testsuite that compare the output of
>  >> the vectorized math functions against the scalar, passing if they are
>  >> sufficiently close. With the testcase for standalone GCN (without
>  >> libgomp) in gcc.target/gcn/, there is a problem since gcn-run
>  >> currently cannot set the stack size correctly in DejaGnu testing, so I
>  >> have made it a compile test for now - it is still useful to check that
>  >> calls to the correct functions are being made. The runtime correctness
>  >> is still covered by the libgomp test.
>  >>
>  >> Okay for trunk?
>  >
>  > The main part of the patch is OK, with the small changes below.
>  >
>  > Others have pointed out that "omp declare simd" exists, but you and I
>  > have been all through that verbally, long ago, and as Tobias says the
>  > offload compiler cannot rely on markup in the host compiler's header
>  > files to solve this problem.
> 
> For what it's worth, I am currently working on enabling "omp declare 
> simd" for SVE and more importantly teaching GCC to use "omp declare 
> variant"'s with simd construct's as simdclones during autovect. This 
> gives a bit more control on what simdclones you advertise as available. 
> I hope to have some RFC's on here soon. I obviously am not familiar with 
> your constraints but just wanted to let you know.

We can use "omp declare target sim" (or whatever the exact form is) to 
create SIMD clones of user functions, but this doesn't work for libm 
functions (or any library) unless the header file for both host and 
offload device have matching markup. Given that the x86_64 Linux host is 
using Glibc and the offload device compiler is using Newlib this is not 
likely to be the case.

I suppose the variants thing could something about this, but with this 
patch we don't need it to, for now.

Andrew