Date: Mon, 21 Apr 2014 04:02:35 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Andrew Turner <andrew@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r264696 - head/lib/libc/arm/gen Message-ID: <20140421011410.E8569@besplex.bde.org> In-Reply-To: <201404201458.s3KEwFQx079814@svn.freebsd.org> References: <201404201458.s3KEwFQx079814@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <ieeefp.h>, but
preparation for killing them in ~2008 removed the declarations there.
The declarations are now opt-in in <machine/ieeefp.h>. 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 <machine.fpu.h>). 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140421011410.E8569>