Re: [PATCH] AArch64 optimized maths functions.

[Andreas Jaeger <aj@suse.com>]

On 11/19/2012 10:51 AM, Marcus Shawcroft wrote:
> Hi,
>
> This patch adds AArch64 optimized maths functions which were presented
> in the orignal port but subsequently removed in order to get the core
> of the port through the review process.
>
> Does anyone have comments on this patch?

A couple of nits that I noticed. I only commented the first usage, these 
happen more than once.

> diff --git a/ports/sysdeps/aarch64/fpu/s_ceil.c b/ports/sysdeps/aarch64/fpu/s_ceil.c
> new file mode 100644
> index 0000000..087b9b4
> --- /dev/null
> +++ b/ports/sysdeps/aarch64/fpu/s_ceil.c
> @@ -0,0 +1,21 @@
> +/* Copyright (C) 2011, 2012 Free Software Foundation, Inc.

This should be "2011-2012". Also, each new file should have as first 
line a description what it does.

> +#ifndef TYPE
> +#define TYPE double
> +#define REGS "d"
> +#else
> +#ifndef REGS
> +#error REGS not defined
> +#endif
> +#endif
Your indentation is off, we write
#ifndef
# define
#else
# ifndef
#  error
# endif
#endif

 > [...]
> +#define weak_aliasx(a,b) weak_alias(a,b)
> +weak_aliasx (__CONCATX(__,FUNC), FUNC)
> +#define strong_aliasx(a,b) strong_alias(a,b)
> +#ifdef NO_LONG_DOUBLE
> +strong_aliasx (__CONCATX(__,FUNC),  __CONCATX(__,__CONCATX(FUNC,l)))
> +weak_aliasx (__CONCATX(__,FUNC), __CONCATX(FUNC,l))
> +#endif

Why do you need strong_aliasx and weak_aliasx? I don't see any benefit 
in using these macros here.

> diff --git a/ports/sysdeps/aarch64/fpu/s_frint.x b/ports/sysdeps/aarch64/fpu/s_frint.x
> new file mode 100644
> index 0000000..b3e21e1
> --- /dev/null
> +++ b/ports/sysdeps/aarch64/fpu/s_frint.x

s_frint.x is a C file. I don't think introducing a ".x" ending is 
proper, it's used by sunrpc input files. Why not rename the file to 
"s_frintX.c"?

Andreas
-- 
  Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
   SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
    GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
     GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126