RE: [PATCH] Remove __isinf uses that rely on signed return value

[Joseph Myers <joseph at codesourcery dot com>]

On Wed, 3 Jun 2015, Wilco Dijkstra wrote:

> > > We discussed this a while back (and I thought it was agreed on going forward
> > > like this) - to recap, it is not required by any standard (so no standard
> > 
> > I don't recall any such discussion or agreement.  The discussion in bug
> > 15367 is on the basis of using __builtin_isinf_sign to implement the
> > header macro, not of changing the requirements and sometimes using
> > __builtin_isinf.
> 
> Well it was a while back. However the GCC built-ins need further optimization
> and fixes so just using the builtins is not a good fix either...

I've grepped my libc-alpha mailboxes for the past 15 years without finding 
anything obvious in this area.

> > glibc is built with _GNU_SOURCE and can freely use all GNU features
> > (subject to link-time namespace issues).  I see no need to change the
> > interface to this macro in glibc.  Presumably the aim should be to make
> > the GCC __builtin_isinf / __builtin_isinf_sign at least as efficient as
> > the out-of-line functions, and safe for all inputs, make the isinf macro
> > use __builtin_isinf_sign, and make GCC optimize __builtin_isinf_sign calls
> > so it's as efficient as __builtin_isinf if the sign isn't used, and as
> > efficient as __builtin_isinf + __builtin_signbit if the sign is used.
> 
> In principle GLIBC could continue to use this feature internally, but do 
> you have any objections to removing all internal uses like my patch does?

Yes, in the absence of evidence that the two produce equivalent code or 
the new one produces better code than the old one (at least for current 
GCC).  I'd expect that to most likely be the case once the GCC built-in 
functions have been appropriately optimized, made to use integer 
arithmetic and are in use when building with current GCC.  It might well 
be the case with appropriate inline integer expansions in glibc headers, 
but I think the GCC inline improvements should take priority with any 
glibc inline expansions being added afterwards as fallbacks for older GCC 
versions.

> In terms of interface, why do you think it is useful to continue a 
> non-standard interface indefinitely rather than making it GNUC specific? 
> Other than backwards compatibility there seems to be no compelling 
> reason to keep it.

It's pretty much universally the case in glibc that feature test macros 
only protect the definitions of identifiers, rather than affecting the 
semantics of those identifiers.  The exceptions are cases where extensions 
actually conflict with supported standards (e.g. basename, or scanf %a).  
The glibc extension does not conflict with the supported standards and 
doesn't seem otherwise problematic - and there's a clear route through GCC 
work for making it not pessimize code for programs that only care about 
the boolean return value of isinf.

> Yes it would be possible to expand isinf as:
> 
> (__isinf_ns (x) ? (signbit (x) ? -1 : 1) : 0)
> 
> This should make if (isinf (x)) as efficient as if (__isinf_ns (x)), 
> however if the result is assigned to a variable then it would always 
> have additional overhead. This is obvious if the sign is not used, but 
> if the sign is used it would still not be as efficient as separate isinf 
> and signbit calls.

I'd expect it to be just as efficient, given that the relevant functions 
have the const attribute so calls to them can be optimized out if the 
results are not used.  It's rather less obvious how the efficiency would 
compare with a direct call to __isinf producing both bits of information, 
until everything is expanded inline.

However, I don't think we really want to add __isinf_ns (an internal 
function at present, not exported from any shared library) to the public 
ABI, given the preferred direction involves compiler support for expanding 
all these calls efficiently inline.

-- 
Joseph S. Myers
joseph@codesourcery.com