- User Since
- Jun 10 2014, 9:32 AM (275 w, 5 d)
Thu, Sep 5
"pow(x, ±0) returns 1 for any x, even a NaN"
Wed, Sep 4
I believe that the code can still be simplified somewhat, but that it's correct as-is for float, double, and long double. I'll take an AI to follow-up on future improvements, and let's get this in.
Wed, Aug 28
Eric showed me this link https://godbolt.org/z/AjBHYqv
I would tend to write this function in the following form:
// set up lower bound and upper bound if (r > upperBound) r = upperBound; if (!(r >= lowerBound)) r = lowerBound; // NaN is mapped to l.b. return static_cast<IntType>(r);
I prefer to avoid the explicit trunc call, since that's the defined behavior of the static_cast once the value is in-range, anyway.
Aug 8 2019
It's not that NaN is rare in normal programs, or that it indicates a bug in the code. It's that testing for NaN is usually an indication that you're testing for an exceptional case, and it makes sense to move those off the hot path (i.e. NaN is actually pretty common, but the likelihood of handling it on the normal-value path through code is small).
Aug 6 2019
(Ideally we would just call them e.g. builtin_floor, but that would be source-breaking. builtin_tgmath_floor seems like a good compromise.)
Strongly agree with what @rjmccall said. If we can make these generic builtins instead of ending up with O(100) variants of each math operation, that would make life immensely nicer.
Jul 26 2019
Jul 24 2019
Jul 22 2019
Reviewers: what do we need to get this across the finish line?
Jul 16 2019
LGTM. Please get at least one additional reviewer's approval before merging, as this is a corner of clang that I don't work on often.
Jul 15 2019
May 30 2019
Mar 11 2019
Ah, now I see what you're talking about. And in fact, because of the way divide works out, there's a little gap of results that are even possible to achieve just below each binade boundary, so the code you have here will work out fine. We *should* add a comment to clarify this somewhat, but I'm happy to do that in a separate commit. LGTM.
These results *are* tiny in the before rounding sense.
In the parlance of IEEE 754, there are two ways to "detect tininess": "before rounding" and "after rounding". The standard doesn't define how to flush subnormal results, but in practice most HW flushes results that are "tiny". The existing code flushes as though tininess is detected before rounding. This proposed update flushes as though tininess were detected after rounding.
Jan 25 2019
do we want to support _Float16 anywhere else?
ARM is the only in-tree target with a defined ABI that I'm aware of.
Jan 11 2019
Dec 17 2018
Can you also add a check for .double infinity? It looks like that's likely missing too.
Dec 7 2018
Oct 9 2018
Are these documented anywhere? I haven't seen it in any of the patches so far. What do they return for NaN inputs?
Oct 3 2018
Sep 24 2018
I suspect that we'd rather use ilogb in the long term, but as a like-for-like replacement this looks OK.
Jul 19 2018
Jul 18 2018
May 22 2018
IIRC the optimization of divide-by-power-of-two --> multiply-by-inverse does not occur at -O0, so it would be better to multiply by 2^(-fbits) instead.
May 10 2018
May 9 2018
One more question: the caller is responsible for closing the file when they're done, right?
Two quick notes as someone who has never used these functions:
- Name is really the *path* of the file to open, right?
- What happens to ResultFD if the function fails?
May 8 2018
May 4 2018
Apr 26 2018
I like Chandler's wording. Something like:
Apr 25 2018
@gottesmm can you take a look at this? You're more familiar with the APFloat API than I am.
Apr 20 2018
Tangential question: Do we have an intrinsic floating -> integer conversion with defined semantics for out-of-range values?
Apr 6 2018
I also wonder whether requiring fast-math to allow tree reductions is overkill. Tree reductions can be implemented reasonably efficiently in many architectures, while linearly ordered reduction appear to me to be more of a niche.
Mar 26 2018
Two questions, to which I do not know the answer:
This is the class of optimizations that I would call "formally allowed by the standard, but extremely likely to break things and surprise people." Which isn't to say that we shouldn't do it, just ... be prepared.
Mar 20 2018
I'm OK with this.
Mar 16 2018
Mar 5 2018
IIRC Intrinsic::sqrt is undef for negative inputs (unlike the sqrt libcall), so we don't need FMF.noNaNs to license this transformation.
Mar 2 2018
Works for me.
Mar 1 2018
We should be able to go farther and just do a fptrunc if SrcSize > DstSize.
Feb 7 2018
Feb 6 2018
Underflow or overflow doesn't change sign, so 0 < C < inf && X >= 0 --> C/X >= 0.
Jan 25 2018
(After checking the architecture manual)
My recollection is that on 386 and later unnormals with a zero significand are treated as an invalid operand; should these print as 0 or as NaN?
Dec 4 2017
Yup, there's likely something more general that we could match, but it's also worth taking this as is.
Nov 27 2017
IEEE 754 rules are that everything canonicalizes except bitwise operations (copy, abs, negate, copysign) and decimal re-encoding operations (which you don't care about).
Oct 30 2017
mod is not bound to the IEEE 754 remainder operation. It binds the C fmod operation. You're looking for the remainder operation.
Sep 13 2017
Sep 12 2017
Sep 6 2017
LGTM. Sorry that I didn't see this earlier.
Aug 21 2017
... except, please add another test-case where the other component is not an integer as well.
This looks fine.
Aug 4 2017
Mar 31 2017
(x-y) == 0 --> x == y does not require nsz (zeros of any sign compare equal), nor does it require nnan (if x or y is NaN, both comparisons are false). It *does* require ninf (because inf-inf = NaN), and it also requires that subnormals are not flushed by the CPU. There's been some churn around flushing recently, Hal may have thoughts (+Hal).
This addresses my concerns. Gentle ping to the other reviewers.
Mar 21 2017
The assumption is that an integer add is more canonical and/or cheaper than an FP add. Is that universally true?
Mar 8 2017
What's the rationale for using rationals here instead of a (simpler) fixed-point representation, if we want to get rid of float?
Feb 7 2017
Great, thanks Simon! This looks fine numerically. @gottesmm, can you double-check for style, etc?
Feb 1 2017
Numerically this is sound, and something that I've wanted to fix for a while but haven't had time to deal with. Thanks for doing it! Please add some test cases that would detect the old error (@gottesmm can likely point you in the right direction).
Jan 19 2017
These tests should either be exact, or should have a tolerance that's mathematically sound. +/-1ulp is not sound; the allowed error should be proportional to the magnitude of the larger of the real and imaginary components of the result -- e.g. if one component is very small compared to the other, the smaller one may have *no* "correct" bits and still be acceptable.
Jan 6 2017
OK, I'm satisfied with that.
Jan 4 2017
Dec 23 2016
I don't particularly care about the shift, since the code is completely equivalent either way, though it would be nice to clean up. However, please replace "mantissa" with "significand" before committing.
Dec 22 2016
Please s/mantissa/significand/. I know that "mantissa" is used in a number of places in llvm sources, but it's incorrect terminology. Significand is the IEEE-754 nomenclature, which any new work should follow.