Message ID | 000c01cf3972$bd2b72f0$378258d0$@com |
---|---|
State | Superseded |
Headers |
Return-Path: <x14307373@homiemail-mx23.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx23.g.dreamhost.com (caibbdcaabja.dreamhost.com [208.113.200.190]) by wilcox.dreamhost.com (Postfix) with ESMTP id 113DA3600B2 for <siddhesh@wilcox.dreamhost.com>; Thu, 6 Mar 2014 11:32:07 -0800 (PST) Received: by homiemail-mx23.g.dreamhost.com (Postfix, from userid 14307373) id 43EF962A9E458; Thu, 6 Mar 2014 11:32:07 -0800 (PST) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx23.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx23.g.dreamhost.com (Postfix) with ESMTPS id 27BB562A9E44B for <glibc@patchwork.siddhesh.in>; Thu, 6 Mar 2014 11:32:07 -0800 (PST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:references:in-reply-to:subject:date :message-id:mime-version:content-type:content-transfer-encoding; q=dns; s=default; b=XOg0DT0v3TtRnvBYocNXz5/QPtM2H/5/LIiUfFi+pZ5 ejuZPnzvsZgJBwK4b3/IlqwxDdEqUUBIaA/L0SGPA6w36+foS4WqiWqlPRacd/w5 f4X2bxBa9A+7osbFezgjpo0FODqZsff6wp7gVEjRHJ+pKjConWUzyBmpdrxqyKh4 = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:references:in-reply-to:subject:date :message-id:mime-version:content-type:content-transfer-encoding; s=default; bh=/GhwjNKKm2wivMjb09/RgIz03cU=; b=LHYBCjIQ0Cv3qzU0u 4ZxWrNgE9zNahsmaOdlF3sxK/JB2XM9/t6cMsSHy2jeS+GqwfHIgfp7ekI4GKvNM a1Na28ceuea7PJS6nYM/mh7shCGy1rTk8dXhR00NORtyzs1f91gd9b/NHZZyXRoP 1CWqKnSXYhHy1VpD41rPCAAFaU= Received: (qmail 8664 invoked by alias); 6 Mar 2014 19:32:05 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-glibc=patchwork.siddhesh.in@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 8651 invoked by uid 89); 6 Mar 2014 19:32:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com From: "Wilco" <wdijkstr@arm.com> To: "'Joseph Myers'" <joseph@codesourcery.com> Cc: <libc-alpha@sourceware.org> References: <000b01cf3949$7e3a0080$7aae0180$@com> <Pine.LNX.4.64.1403061550110.18293@digraph.polyomino.org.uk> In-Reply-To: <Pine.LNX.4.64.1403061550110.18293@digraph.polyomino.org.uk> Subject: RE: [PATCH] [ARM] ] Add support for fenv_private on ARM Date: Thu, 6 Mar 2014 19:31:57 -0000 Message-ID: <000c01cf3972$bd2b72f0$378258d0$@com> MIME-Version: 1.0 X-MC-Unique: 114030619320000301 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-DH-Original-To: glibc@patchwork.siddhesh.in |
Commit Message
Wilco Dijkstra
March 6, 2014, 7:31 p.m. UTC
On Thu, 6 Mar 2014, Joseph Myers wrote: > ARM_HAVE_VFP isn't suitable for use in #if; see the > sysdeps/unix/sysv/linux/arm/arm-features.h definition. You need either a > new macro meaning "VFP is known at compile time to be available", or to > move to checks with "if" inside the functions. Good point - it appears __SOFTFP__ is what I wanted: > (The case of a soft-float build, VFP hardware available at runtime - the > one addressed by "if" conditionals - has other problems with exceptions > and rounding modes, that I think would best be addressed by use of IFUNCs > in libgcc for the relevant RTABI functions; see bug 10064. Anyway, the > present patch is purely an optimization, so there's certainly no need for > it to cover all cases as long as it doesn't break them.) I think the best option is to not change the behaviour of fenv when you use softfp but run on a CPU that supports VFP - especially since softfp doesn't support rounding modes or exceptions. I'm working on a patch to improve fenv with the same optimizations and clean up the code, so can fix all the if (ARM_HAVE_VFP) to use __SOFTFP__. On a related issue, the fenv testing code doesn't check the return code of the fenv functions, which causes failures. It also assumes that fenv functions set errno when this is not part of the C99 spec. Is that on purpose or a bug? Wilco
Comments
On Thu, 6 Mar 2014, Wilco wrote: > > (The case of a soft-float build, VFP hardware available at runtime - the > > one addressed by "if" conditionals - has other problems with exceptions > > and rounding modes, that I think would best be addressed by use of IFUNCs > > in libgcc for the relevant RTABI functions; see bug 10064. Anyway, the > > present patch is purely an optimization, so there's certainly no need for > > it to cover all cases as long as it doesn't break them.) > > I think the best option is to not change the behaviour of fenv when you > use softfp but run on a CPU that supports VFP - especially since softfp > doesn't support rounding modes or exceptions. The best option for the present patch. I'd like such a configuration to work properly (i.e. when the fenv functions succeed they also do affect arithmetic operations), but that needs the IFUNCs. (Having VFP implementations of the RTABI functions in libgcc conditioned at compile time on __SOFTFP__ is a good idea simply for efficiency, if a soft-float object file is linked with libraries built for VFP - as well as making libgcc smaller when built for VFP, as the VFP implementations should be smaller than the soft-float ones (cf. the recent change in libgcc on MIPS to use minimal implementations of various libgcc functions for hard float, rather than the previous soft-float versions also used in hard-float libraries). IFUNCs for libgcc built for soft float is a step beyond that, which would make libgcc consistent with glibc.) > I'm working on a patch to improve fenv with the same optimizations and > clean up the code, so can fix all the if (ARM_HAVE_VFP) to use __SOFTFP__. But that's not correct in the cases where a runtime check is used (and in general, if () conditions are better than #if because they mean both cases still get checked for syntax on every compilation). I suppose logically there are two meaningful soft-float configurations, one with runtime checks for VFP in both libgcc and glibc, and one with no such checks in either place and the present VFP support in glibc disabled. The former makes sense for general-purpose distributions supporting a wide range of hardware (e.g. Debian armel), by producing binaries that adapt automatically to the functionality of the hardware they run on; the latter if the binaries are only intended for specific soft-float hardware. > On a related issue, the fenv testing code doesn't check the return code > of the fenv functions, which causes failures. It also assumes that fenv > functions set errno when this is not part of the C99 spec. Is that on > purpose or a bug? That should just be math/test-fenv. There's a trade-off between: * detecting a failure as meaning the feature is unsupported and so making the test quietly PASS, when actually the failure is not expected on the given platform and so it should have FAILed, and * treating any failure as meaning the test fails, when actually it might be expected on a given platform. I suppose I'd be inclined to say that test-fenv should use macros from math-tests.h to indicate which functions are expected to work on a given platform and which are expected to fail (on ARM you'd need some initialization step to determine whether traps for exceptions are supported). So unless a platform says certain failures are OK, they would make the test as a whole fail. As for errno setting, generally ISO C says little about errno but the norm for POSIX and glibc functions is to set errno on error unless there is some other defined way to indicate what error occurred (e.g. functions defined to return an error number).
On 6 March 2014 22:32, Joseph S. Myers <joseph@codesourcery.com> wrote: > That should just be math/test-fenv. There's a trade-off between: > > * detecting a failure as meaning the feature is unsupported and so making > the test quietly PASS, when actually the failure is not expected on the > given platform and so it should have FAILed, and > > * treating any failure as meaning the test fails, when actually it might > be expected on a given platform. > > I suppose I'd be inclined to say that test-fenv should use macros from > math-tests.h to indicate which functions are expected to work on a given > platform and which are expected to fail (on ARM you'd need some > initialization step to determine whether traps for exceptions are > supported). So unless a platform says certain failures are OK, they would > make the test as a whole fail. > > As for errno setting, generally ISO C says little about errno but the norm > for POSIX and glibc functions is to set errno on error unless there is > some other defined way to indicate what error occurred (e.g. functions > defined to return an error number). IIRC the relevant part of test-fenv.c in the context of this thread is: errno = 0; fesetenv (FE_NOMASK_ENV); status = errno; fesetenv (&saved); if (status == ENOSYS) There is one target in the glibc tree that appears to make use of errno in this manner, sysdeps/powerpc/fpu/fesetenv.c has a path that calls __fe_nomask_env_priv() setting ENOSYS, curiously this fesetenv() implementation always returns 0 even when setting errno, which seems rather odd to me, I wonder if this is by design or simply a hang over from the days before fesetenv() gained a return value? Given Joseph's comments above and the existing use of errno in this context it doesn;t seem unreasonable to me that both arm and aarch64 might also set ENOSYS on platforms that do not support trapping exceptions. /Marcus
On Fri, 7 Mar 2014, Marcus Shawcroft wrote: > IIRC the relevant part of test-fenv.c in the context of this thread is: > > errno = 0; > fesetenv (FE_NOMASK_ENV); > status = errno; > fesetenv (&saved); > if (status == ENOSYS) > > There is one target in the glibc tree that appears to make use of > errno in this manner, sysdeps/powerpc/fpu/fesetenv.c has a path that > calls __fe_nomask_env_priv() setting ENOSYS, curiously this fesetenv() > implementation always returns 0 even when setting errno, which seems > rather odd to me, I wonder if this is by design or simply a hang over > from the days before fesetenv() gained a return value? Given the lack of other cases setting errno in <fenv.h> functions, it might be better here simply to check the return value - and not to check errno at all. But I think allowing failure should be conditional on something from math-tests.h saying the architecture intends that this feature might not be supported.
diff --git a/sysdeps/arm/fenv_private.h b/sysdeps/arm/fenv_private.h index 6c65cfa..0b73816 100644 --- a/sysdeps/arm/fenv_private.h +++ b/sysdeps/arm/fenv_private.h @@ -23,7 +23,7 @@ #include <fpu_control.h> #include <arm-features.h> -#if ARM_HAVE_VFP +#ifndef __SOFTFP__ static __always_inline void libc_feholdexcept_vfp (fenv_t *envp)