Re: [PATCH] fmax(), fmaxf(), fmaxl(), fmin(), fminf(), fminl() simplified

[Rich Felker <dalias@libc.org>]

On Wed, Dec 11, 2019 at 02:16:59PM +0100, Szabolcs Nagy wrote:
> * Stefan Kanthak <stefan.kanthak@nexgo.de> [2019-12-11 13:33:44 +0100]:
> > "Szabolcs Nagy" <nsz@port70.net> wrote:
> > >* Stefan Kanthak <stefan.kanthak@nexgo.de> [2019-12-11 10:55:29 +0100]:
> > > these two are not equivalent for snan input, but we dont care
> > > about snan, nor the compiler by default, so the compiler can
> > > optimize one to the other (although musl uses explicit int
> > > arithmetics instead of __builtin_isnan so it's a bit harder).
> > 
> > The latter behaviour was my reason to use (x != x) here: I attempt
> > to replace as many function calls as possible with "normal" code,
> > and also try to avoid transfers to/from FPU/SSE registers to/from
> > integer registers if that does not result in faster/shorter code.
> 
> why not just change the definition of isnan then?
> 
> #if __GNUC__ > xxx
> #define isnan(x) sizeof(x)==sizeof(float) ? __builtin_isnanf(x) : ...

Yes, I think having this conditioned on GNUC would be acceptable,
provided the builtin is supported in all versions of GNUC going back
conceivably far.

> > > you should run any such change through libc-test
> > > git://repo.or.cz/libc-test and look for regressions.
> > 
> > I already told Rich that I neither use an OS nor a compiler/assembler
> > where musl or libc-test can be built.
> 
> it does not matter where you use musl, if you want
> to submit patches you have to test on supported
> targets (since it's not realistic to test on all
> configurations, at least one relevant configuration
> is enough)

I think it's "ok" to submit patches without having run tests, with the
caveat that it's going to impact how willing folks are to review them,
and if so how quickly it gets done. If submitting patches you haven't
tested, text explaining the reasoning for why you think they're
correct (e.g. "nan behavior is still correct because line N produces a
nan and the function is allowed to raise invalid under this
condition") would help a lot since thought processes like that are
hard to reverse-engineer.

Rich