From owner-svn-src-all@FreeBSD.ORG Sun Apr 20 18:25:50 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 0349ADB2; Sun, 20 Apr 2014 18:25:50 +0000 (UTC) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id A19B2173B; Sun, 20 Apr 2014 18:25:49 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id 879AA782094; Mon, 21 Apr 2014 04:02:36 +1000 (EST) Date: Mon, 21 Apr 2014 04:02:35 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andrew Turner Subject: Re: svn commit: r264696 - head/lib/libc/arm/gen In-Reply-To: <201404201458.s3KEwFQx079814@svn.freebsd.org> Message-ID: <20140421011410.E8569@besplex.bde.org> References: <201404201458.s3KEwFQx079814@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=QIpRGG7L c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=_0BtgO3-ezQA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=FYdb16HlwN49KzZiXeYA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 20 Apr 2014 18:25:50 -0000 On Sun, 20 Apr 2014, Andrew Turner wrote: > Log: > Add the deprecated fp{get,set}* functions, a few ports use them. > > Added: > head/lib/libc/arm/gen/fpgetmask.c (contents, props changed) > head/lib/libc/arm/gen/fpgetround.c (contents, props changed) > head/lib/libc/arm/gen/fpgetsticky.c (contents, props changed) > head/lib/libc/arm/gen/fpsetmask.c (contents, props changed) > head/lib/libc/arm/gen/fpsetround.c (contents, props changed) > head/lib/libc/arm/gen/fpsetsticky.c (contents, props changed) These obsolete i386 APIs are hard to kill. They keep coming back for new arches. I don't see how most of them can even be used on those of the new arches including arm where they are not declared in any header. They used to be declared in an opt-out way in , but preparation for killing them in ~2008 removed the declarations there. The declarations are now opt-in in . Many arches are now broken by having a different set of declarations than what they implement. Perhaps it is not broken to have no declarations, but this is unclear, and in amd64 there is a declaration for an unimplemented function. These functions in libc but not declared in any header are supposed to be for binary compatibility only. Soft-float already has these mistakes, but binary compatibility of hard-float with mistakes in soft-float shouldn't be needed. > Added: head/lib/libc/arm/gen/fpsetsticky.c > +#ifdef __weak_alias > +__weak_alias(fpsetsticky,_fpsetsticky) This seems to be backwards. It aliases the non-polluting name to the polluting name. There is no need for a non-polluting name if the polluting one is unavoidable. All libc code for fp* aliases has the same bug (mips/hardfloat, softfloat). The pollution is smaller for some private (sic) public symbols in shared libraries, but this public symbol is exported. Even private public symbols need underscores for the static case. > +#endif > + > +#define FP_X_MASK (FP_X_INV | FP_X_DZ | FP_X_OFL | FP_X_UFL | FP_X_IMP) Style bug (space insted of tab). > ... > +fp_except > +fpsetsticky(fp_except except) > +{ > + fp_except old, new; > + > + __asm __volatile("vmrs %0, fpscr" : "=&r"(old)); > + new = old & ~(FP_X_MASK); Style bug (bogus parentheses). > + new &= ~except; > + __asm __volatile("vmsr fpscr, %0" : : "r"(new)); > + Style bug (extra blank line). > + return (old & except); > +} fpsetsticky() and fpresetsticky() are especially bogus and not needed for new arches. There is considerable confusion about their name, and some old arches have either never had them or have removed 1 or both of them, even in object files. fpresetsticky() seems to be the correct name, and fpsetsticky() just a FreeBSD mistake. fpsetsticky() was never documented in FreeBSD's man page for such things (fpgetround.3), but I think it was the most popular spelling. It was removed for i386 in 2008. Googling it then and now shows almost no hits for it outside of FreeBSD, and mostly old hits for FreeBSD. fpresetsticky() was once fairly standard (see google). It was obsoleted by femumble() in C99. i386 still has fpresetsticky() for API compatibility. On i386, the fp* functions were always static inlines. This keeps them out of libraries, so they don't have any ABI to be compatible with. But on many new arches, they get into libc for compatibility and then cause even more ABI compatibility problems than they avoided. fpresetsticky() is still the only one documented. It is documented for all arches, only old arches should have it. This is not even wrong for new arches. New arches tend to have the the never-documented fpsetsticky() instead :-(. Arch-dependent ifdefs would be needed to select the wrong function. On i386, the situation is as described above (inline functions with only fpsetsticky() removed and fpresetsticky() fully available as an inline function but not as an extern function). On amd64: from amd64/include/ieeefp.h: % #if !defined(__IEEEFP_NOINLINES__) && defined(__GNUCLIKE_ASM) % % #define fpgetmask() __fpgetmask() % #define fpgetprec() __fpgetprec() % #define fpgetround() __fpgetround() % #define fpgetsticky() __fpgetsticky() % #define fpsetmask(m) __fpsetmask(m) % #define fpsetprec(m) __fpsetprec(m) % #define fpsetround(m) __fpsetround(m) These are the internal declarations (in terms of inline functions), with no style bugs and both setsticky functions intentionally left out. One reason that they were left out is that the confusion between their names makes it unclude whether they set or clear bits (there was a PR or 2 about this). Another is just to see what breaks when they are killed. Apparently nothing. % % #else /* !(!__IEEEFP_NOINLINES__ && __GNUCLIKE_ASM) */ % % /* Augment the userland declarations. */ % __BEGIN_DECLS % extern fp_rnd_t fpgetround(void); % extern fp_rnd_t fpsetround(fp_rnd_t); % extern fp_except_t fpgetmask(void); % extern fp_except_t fpsetmask(fp_except_t); % extern fp_except_t fpgetsticky(void); % extern fp_except_t fpsetsticky(fp_except_t); The above mound of style bugs (with unsorting, redundant externs and corrupt tabs) was copied here and perhaps to some other MD ieeefp.h's from the MI ieeefp.h. Similarly for some other arches. % fp_prec_t fpgetprec(void); % fp_prec_t fpsetprec(fp_prec_t); These prototypes are here because these were the only APIs that were not declared in the MI ieeefp.h. They have no style bugs since the file was originally more carefully written. Note that all of these prototypes are very rarely used. I think their only practical use is to quieten -Wmumble when this file is used to implement the public functions in libc. I don't know why amd64 has regressed relative to i386 by making this obsolete API more available (i386 only has it as inlines via macros). Other arches only ever had the public functions, so they always had a binary compatibility problem if anyone actually used these functions. % __END_DECLS % % #endif /* !__IEEEFP_NOINLINES__ && __GNUCLIKE_ASM */ arm ieeefp.h doesn't declare any of the functions (unless you changed it recently). arm libc now implements 6 (?) of the functions. arm ieeefp.h is completely unusable in applications even for testing the exception flags, since it doesn't declare the standard type fp_except_t, but only fp_except. fp_except_t is documented as being #define'd as int. This is broken on all arches except amd64 and i386 -- it is a typedef on the others, except on arm where it doesn't exist. Many other style bugs (mostly formatting and punctuation) can be fixed by copying from i386. ia64 ieeefp.h declares the 4 functions that libc implements. It has fewer style bugs than arm ieeefp.h, but it has gross namespace pollution (nested include of ). ia64 doesn't declare or implement either get or set of sticky. mips ieeefp.h doesn't declare any of the functions, but mips libc implements 6 in hardfloat and softfloat, including both get and set of sticky (hopefully set sticky is actually reset sticky = clear). I think softfloat applies to arm too. A different naming scheme (with a hardfloat subdir) is used for mips hardfloat. mips ieeefp.h also has fp_except but is not missing fp_except_t. It also has fp_rnd alongside fp_rnd_t. Perhaps the _t suffixes were another old i386 mistake. powerpc ieeefp.h declares 5 functions. powerpc libc implements all 5 (not et of sticky). spardc64 is like powerpc. The following arches have a public fpsetsticky: mips/hardfloat, softfloat, and now arm(hardfloat?). 8 functions are documented, but most arches don't have runtime-modifiable precision, so there are normally 6 functions. Omitting reset of sticky gives 5, and omitting get of sticky gives 4. Bruce