Wearing my compiler user hat, I would much rather use additive -mfeature than have to specify these as -march+feature, even when using a build system that nominally handles this stuff, because I frequently want to be able to compile one specific file with "whatever the prevailing options are, but also enable this one feature." Most build systems make this possible somehow (track down the arch variable, append +feature to it, etc), but it's considerably simpler if you can just append -mfeature to the list of flags and call it a day.

In any event, from the standpoint of the C(23) language, these operations do not set inexact, so I believe that it is appropriate to optimize them as if they do not set inexact.

Looking at implementations of these functions, it looks like GNU libm doesn't raise inexact, but the bionic libm does. I think I'm leaning towards marking all of them as "fng" as it's the more cautious of the two.

Hmm, bionic's behavior sounds a bit surprising. In the committed version, I kept them fnc to keep the previous behavior for now. I'll double-check with @scanon and potentially upload a follow-up patch.

The CFP working group and C23 have since clarified this in Annex F:

There's a lot of churn around proposed "solutions" on this and related PR, but not a very clear analysis of what the problem we're trying to solve is.

In D117480#3248644, @fhahn wrote:

I assume ordered here means sequential. The clang builtin specifies a tree-wise reduction order.

Two minor questions, but also LGTM as is.

What's the rationale for making abs undefined on the minimum value? AFAIK every actual simd implementation defines the result and they agree (and even if one didn't, it would be pretty easy to get the "right" result. Introducing UB here just seems like punishing users for no reason.

I'm happy with this now.

Is there a mechanism to instruct the sanitizer to ignore a specific expression or function? From a cursory reading, I am mildly concerned about a deluge of false positives from primitives that compute exact (or approximate) residuals; these are acting to eliminate or precisely control floating-point errors, but tend to show up as "unstable" in a naive analysis that isn't aware of them.

I'm fine with this.

Strictly speaking, fp-contract=fast probably should have been a separate flag entirely (since there's no _expression_ being contracted in fast). Unfortunately, that ship has sailed, and it does constrain our ability to choose an accurate name somewhat.

I do not much like faststd, as there's nothing "standard" about it. I do not, however, have a better suggestion off the top of my head. Let's pause and consider the name a little bit longer, please?

(If you tell GCC to respect the pragma via -std=c17 or similar, then -ffp-contract=fast overrides it just like clang's current behavior: https://godbolt.org/z/5dxxGb)

GCC doesn't respect the pragma, so "what other compilers do" is not a particularly useful metric.

I guess the counterargument here would be that .x does not produce an extvector(1), and there is at least a plausible argument that .x should be the same as .lo for a two-element vector. I'm not really convinced by this, but it's not totally outrageous.

I'm fairly certain that this will cause some breaks internally at Apple, but I'm also pretty sure that it's a step in the right direction, and we should just sign up to fix any issues it causes.

If we can do it without complication, it would be best to preserve signaling-ness, because that's the more faithful interpretation of IEEE 754 (even though it _doesn't_ match what the HW does, because the HW can signal and APFloat can't). A general principle (imperfectly adhered to) of IEEE 754 is that conversions on signaling NaNs should _either_ signal and produce a quiet NaN (if possible), or should produce a signaling NaN if no signal is possible.

Prior to this change contract was never generated in the case of in-statement contraction only, instead clang was emitting llvm.fmuladd to inform the backend that only those were eligible for contraction. From a correctness perspective I think this was perfectly fine.

Currently I don't see any logic to generate "blocking intrinsics" (I guess to define a region around the instructions emitted for the given statement). Until such mechanism is in place, I think that generating the contract fast-math flag also for in-statement contraction is wrong because it breaks the original program semantic.

TS18661-5 is quite vague on what the intended semantics for the pragma are.

(Please get one additional sign off before committing; I'm mainly signing off on the numerics model aspect).

My concerns have been addressed. Thanks for bearing with me, Melanie!

I don't think the C standard is likely to ever bless reassociative FP math with an expression-local restriction. Steve, do you actually think that would be a useful optimization mode?

Backing up what everyone says here: logb doesn't define the sign of NaN results, and 754 explicitly says not to interpret the sign of NaN as having any meaning except in the copySign, absoluteValue, negate, and copy operations. (That's a semantically meaningless statement, since those operations do not exist in a vacuum, which means that you can't actually say anything about the sign of NaN from a formal perspective, but, well, it's the standard we have.)

"pow(x, ±0) returns 1 for any x, even a NaN"

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.

In D66836#1649846, @zoecarver wrote:

Dead link.

Here: https://godbolt.org/z/AjBHYq

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.

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).

(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.

LGTM

Reviewers: what do we need to get this across the finish line?

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.

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.

do we want to support _Float16 anywhere else?

ARM is the only in-tree target with a defined ABI that I'm aware of.

Can you also add a check for .double infinity? It looks like that's likely missing too.

Shrug. OK.

Are these documented anywhere? I haven't seen it in any of the patches so far. What do they return for NaN inputs?

I suspect that we'd rather use ilogb in the long term, but as a like-for-like replacement this looks OK.

In D49524#1168723, @ab wrote:

I wouldn't be surprised if it failed on some target that can't even handle half parameters

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.

LGTM, thanks!

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?

LGTM

In D46042#1088044, @ab wrote:

So, this makes sense to me, but on x86, should we also be worried about the fact that the calling convention is based on which features are available? (>128bit ext_vector_types are passed in AVX/AVX-512 registers, if available). Presumably swift is also affected, no?

I like Chandler's wording. Something like:

@gottesmm can you take a look at this? You're more familiar with the APFloat API than I am.

Tangential question: Do we have an intrinsic floating -> integer conversion with defined semantics for out-of-range values?

In D45336#1059881, @gnzlbg wrote:

it should really be the default in both LLVM and source languages except where there's an explicit constraint that forces linear reduction.

Tree reductions is what Rust portable packed SIMD RFC [0] currently specifies.

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.

In D44909#1048811, @spatel wrote:

In D44909#1048805, @scanon wrote:

Two questions, to which I do not know the answer:

(a) Are the semantics of ISD::FP_TO_[US]INT for out-of-range values specified anywhere? The do not seem to be, but maybe I'm just missing it.

No, I don't see anything either. We just have:

/// FP_TO_[US]INT - Convert a floating point value to a signed or unsigned
/// integer.

I was assuming these nodes follow the same rules as the IR instructions since they are mapped 1-to-1 in SelectionDAGBuilder::visitFPToSI() etc.

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.

I'm OK with this.

IIRC Intrinsic::sqrt is undef for negative inputs (unlike the sqrt libcall), so we don't need FMF.noNaNs to license this transformation.

Works for me.