From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6479 invoked by alias); 19 Nov 2012 19:30:23 -0000 Received: (qmail 6453 invoked by uid 22791); 19 Nov 2012 19:30:21 -0000 X-SWARE-Spam-Status: No, hits=-7.1 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,TW_FN X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 19 Nov 2012 19:29:49 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 28873A329E; Mon, 19 Nov 2012 20:29:48 +0100 (CET) Message-ID: <50AA88A8.3000102@suse.com> Date: Mon, 19 Nov 2012 19:30:00 -0000 From: Andreas Jaeger User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121025 Thunderbird/16.0.2 MIME-Version: 1.0 To: Marcus Shawcroft Cc: libc-ports@sourceware.org Subject: Re: [PATCH] AArch64 optimized maths functions. References: In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org X-SW-Source: 2012-11/txt/msg00066.txt.bz2 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