Fair enough. Just for conversation's sake, I was envisioning the FE support a little differently...

Abandon this Revision for D54649...

Oh, right. I missed that. So there should probably be a new STRICT_SETCC node. Thanks.

I'm working on the changes that we discussed, but they're pretty invasive. A prospective patch is coming soon, but I wanted us to start thinking about how we'll handle these intrinsics at the SelectionDAG level. There are no CMP ISD nodes (also, what does legalization look like??), so this will likely be a significant change.

Good catch, @arsenm. Thanks.

In D53877#1297257, @arsenm wrote:

I just realized the LangRef changes are missing

In D53877#1297257, @arsenm wrote:

I just realized the LangRef changes are missing

@majnemer has approved the changes offline. Will commit this patch imminently...

David also gave a LGTM for the call-and-invoke-with-ranges.ll change off-line. If he's okay with the function code number change in LLVMBitCodes.h, I'll go ahead and commit this patch.

Rebase and create new function code number, as suggested by @majnemer.

Thinking aloud...

Here's a first whack at a list of operations needed internally:

Update test/Transforms/MergeFunc/call-and-invoke-with-ranges.ll to account for changes in the Instruction.def enum values.

In D53157#1291964, @kpn wrote:

I don't expect anyone would need to switch between constrained and regular floating point at an instruction level of granularity.

In D54121#1291755, @craig.topper wrote:

The X86 builtin story is weird. There should be 9 builtins. I'm not sure how you found 12.

Wow, that's great. Thanks, Matt!

More insight...

In D54121#1291085, @andrew.w.kaylor wrote:

In D54121#1290989, @cameron.mcinally wrote:

compareSignalingGreaterEqual(a,b) is equivalent to compareSignalingLessUnordered(b, a)

Is it? If a or b is NaN the first one will return false but the second will return true.

That's cool with me. Definitely a ton of work, but it's clean and complete.

Add bitcode tests for fastmath flags, as @arsenm suggested. Thanks for bearing with me, Matt.

Add two test/Bitcode tests and address one llvm-bcanalyzer issue, as suggested by @arsenm.

Ah, I think I see some problems with llvm-bcanalyzer. Will address...

In D53877#1290637, @arsenm wrote:

Bitcode compatibility tests are missing?

Add newlines to lib/AsmParser/LLParser.cpp, as suggested by @arsenm.

Seems fair. I don't know enough about the Clang FE to have an opinion on that.

Adding the usual suspects: @andrew.w.kaylor @craig.topper @uweigand @hfinkel

All good points.

In D43142#1285974, @andrew.w.kaylor wrote:

We still need to control code motion. For instance, if the code checks the status bits before calling the FENV_ACCESS OFF function we can't have FP operations that are inlined hoisted above the status bit check.

In D43142#1285825, @andrew.w.kaylor wrote:

Well, yes and no. In a region defined as FENV_ACCESS off the user has granted the compiler explicit permission to ignore FP status bits and exception behavior. Depending on how we optimize these regions the FP status bits may come out differently and exceptions may or may not be raised, but the user has promised not to unmask exceptions or look at status bits in that region. So if that part of the behavior of the code changes depending on whether or not we inline that's OK because that's what the user signed up for. In that sense the behavior hasn't changed any more than it does between -O0 and -O3.

In D43142#1285708, @andrew.w.kaylor wrote:

The fact that we have chosen to inline the function doesn't change the semantics.

In D43142#1285692, @kpn wrote:

Hmmm. A clang option to turn on FENV_ACCESS for entire compilations would work around this issue. So the behavior you are describing here would work for me.

In D43142#1285565, @kpn wrote:

If all optimizations including constant folding, or at least optimizations on floating point, are delayed until after inlining then there's no problem.

Rebase and remove extra '^' characters from docs/LangRef.rst.

Add a comment about not reordering the llvm-c/Core.h enum values, as @jyknight suggested.

In D43142#1284977, @andrew.w.kaylor wrote:

I thought what we said was that if constrained FP intrinsics were used anywhere in a function then they had to be used everywhere in the function. So if you inline a function with FENV_ACCESS=ON into a function with FENV_ACCESS=OFF then the FP operations in the destination function need to be converted to constrained intrinsics, and if you inline a function with FENV_ACCESS=OFF into a function with FENV_ACCESS=ON then the FP operations in the function being inlined need to be converted. This conversion will have a detrimental effect on optimizations, so it would be nice to factor that into the inling cost, but at this point I'm mostly concerned about generating correct code.

Don't reorder the members in llvm-c/Core.h as suggested by @jyknight.

In D43142#1284296, @kpn wrote:

Then don't we have to deal with this sometime? If #pragma FENV_ACCESS ON is set for the main() example above then is constant folding allowed?

In D43142#1284190, @kpn wrote:

And couldn't we turn off constant folding in the IRBuilder when necessary?

Here's a quick example. You can see in the IR that the ConstantFolder picks up the FDiv as undefined:

I don't think this will work. The ConstantFolder could fold away traps early in the IRBuilder. We also wouldn't catch the FNeg/FSub case either.

Remove extra whitespace from test/Assembler/fast-math-flags.ll.

Add -strict-whitespace option.

Reorder LLVMAddrSpaceCast as @arsenm suggested.

Updating diff to add test/Assembler tests. This includes both fastmath flags and double-round-tripping tests.

In D53877#1280756, @lebedev.ri wrote:

with the intention of adding more Unary operations in the future. E.g. integer Neg (and maybe Abs, Copysign, etc.).

While there are very clear reasons for FNeg to exist, this isn't so true for any of the others listed here..

Ping.

Whoops, forgot to update. Yeah, your vector test looks good now.

Updating diff based on Sanjay's suggestions. One comment inline...

In D53205#1273112, @cameron.mcinally wrote:

@spatel notice the one test change. That seems like a good change to me, i.e. replace a constant pool load with undef. But, perhaps the CP load is a canonicalization that I don't know about....

Rebase, add Sanjay's changes, and replace some more BinaryOperator::isFNeg(...) calls.

LGTM

Ah, yeah. That would work. Thanks, Sanjay.

Just thinking aloud. I really don't have enough experience with this framework to say for sure...

In D53205#1269202, @spatel wrote:

1. There are no users of that "IgnoreZeroSign" optional parameter in trunk - just delete it?
   
   ` -bool BinaryOperator::isFNeg(const Value *V, bool IgnoreZeroSign) { +bool BinaryOperator::isFNeg(const Value *V) {
   
   `

Add support for FROUND and FTRUNC too.

In D53205#1267982, @cameron.mcinally wrote:

Also, it will be ugly to compensate for this shortcoming at the caller.

Ah, yeah, there is a silent regression here. From BinaryOperator::isFNeg(...):

Rebase to pick up Sanjay's changes...

In D53205#1264508, @spatel wrote:

I suspect that none of these changes are actually 'NFC'.

Bah, I forgot the "Differential Revision:" tag. This was committed with:

Move "NSZ" to be a suffix.

In D52934#1258981, @lebedev.ri wrote:

NSW is a flag *on an instruction*. It's not a 'fneg operation of NSZ instruction'.
Suffix variants are cleaner IMO. Would be great to stick to them, and migrate the rest.

Update 'ignore signed zero' acronym to "NSZ".

Small update to remove superfluous parentheses.

There hasn't been any strong objects, but I haven't seen many strong accepts either besides the few main stakeholders. I'm under the assumption that silence is a passive reject in situations like this.

It looks like an explicit FNEG operation isn't going to fly. I propose we drop that pursuit and continue forward with a constrained FNEG intrinsic.

Did we decide that fast math flags can't be applied in the presence of constrained operations?