Bah, the following patch poked holes in this Diff:

Well, mayRaise Exception is purely a MI level flag. I struggle to see where optimizations on the MI level would ever care about rounding modes in the sense you describe: note that currently, MI optimizations don't even know which operation an MI instruction performs -- if you don't even know whether you're dealing with addition or subtraction, why would you care which rounding mode the operation is performed in? MI transformations instead care about what I'd call "structural" properties of the operation: what are the operands, what is input vs. output, which memory may be accessed, which special registers may involved, which other side effects may the operation have. This is the type of knowledge you need for the types of transformations that are done on the MI level: mostly about moving instructions around, arriving at an optimal schedule, de-duplicating identical operations performed multiple times etc. (Even things like simply changing a register operand to a memory operand for the same operation cannot be done solely by common MI optimizations but require per-target support.)

Well, this is because I'm really only using this feature to handle exceptions (b.t.w. just like the MemOperands in the alternate attempt).

Rounding mode is handled completely in the back-end: we simply make all floating-point instructions (strict or not doesn't even matter here) use the FPC control register, and mark all instructions that change the rounding mode as changing FPC. The one missing piece is that we need to mark all function calls to functions marked as within FENV_ACCESS ON regions as also clobbering FPC -- that can be done easily in the target ABI code as soon as the front-end marks such function calls (which we need anyway).

@uweigand, it looks like you submitted some unintended changes with the last Diff update. Just FYI...

A quiet ping on this, since this is my last working day of the year...

I like this implementation a lot. Some targets control the FPEnv through a register, not through memory. Modeling instructions with register side-effects through MachineMemOperand wasn't intuitive.

In D54649#1318762, @cameron.mcinally wrote:

Looking closer, I may have a better solution for the SETCC case. It won't solve the general problem of nodes needing to be Custom lowered though. Will report back soon...

Revert back to mutating a temp before Custom lowering.

In D54649#1318675, @uweigand wrote:

The one problem with your "new" code is that it now forces back-ends to implement something, or else code involving constrained intrinsics will trigger internal compiler errors. It might be preferable to avoid those ...

Old patch:

In D53157#1316860, @uweigand wrote:

In my reading of the standard text, there is no special provision for library code. This means that in general, calling any library function while running with nondefault trap settings is undefined behavior, unless the library was itself compiled with FENV_ACCESS ON. There does not even appear to be any requirement for the C standard library functions to have been compiled with FENV_ACCESS ON, as far as I can see ...

Remove setting of STRICT_FSETCC Custom lowering, as @uweigand suggested.

This issue was handled with the addition of a dedicated FNeg IR Instruction in D53877.

@uweigand, what about something like this patch? The STRICT_FSETCC isn't handled all the way through the target, but it's stubbed out for now.

Update TableGen changes, as suggested by @nhaehnle.

Digressing a bit, but has anyone given thought to how this implementation will play out with libraries? When running with traps enabled, libraries must be compiled for trap-safety. E.g. optimizations on a lib's code could introduce a NaN or cause a trap that does not exist in the code itself.

In D53157#1304900, @andrew.w.kaylor wrote:

I think at this point we're all on the same page in this regard. I just wanted to make sure we also understand why we're on that page. I still believe it was the correct choice.

In D53157#1304880, @hfinkel wrote:

In D53157#1304873, @cameron.mcinally wrote:

I'd like to hear more about this. The fesetexcept(...) function and friends change the FPEnv state, which can change the semantics for the instructions that follow. They definitely have to act as a barrier.

There's no sense in which we can have a code-motion barrier within a function that acts on the regular FP instructions. We can have a barrier for the constrained intrinsics. This is why we need, in this design, for any function that uses FENV_ACCESS=ON for any part of it, to always use the constrained instrinsics.

In D53157#1304347, @uweigand wrote:

OK, let me try to expand on my point 3 above, which appears to have confused everybody :-)

In D53157#1304275, @kpn wrote:

In D53157#1303398, @cameron.mcinally wrote:

If we all agree upon that, then we simply have to treat the functions that modify the FPEnv, e.g. fesetexcept(...), as barriers. That way it does not matter if a FENV_ACCESS=OFF function is translated with constrained intrinsics or not, since nothing can be scheduled around these barriers.

I thought we couldn't do barriers. No barriers means no way to prevent code motion and mixing of constrained with non-constrained FP. That was the reason for having all FP in a function be constrained if any of it was.

In D53157#1302724, @uweigand wrote:

A couple of comments on the previous discussion:

1. Instead of defining a new command line option, I'd prefer to use the existing options -frounding-math and -ftrapping-math to set the default behavior of math operations w.r.t. rounding modes and exception status. (For compatibility with GCC if nothing else.)

In D53157#1301992, @hfinkel wrote:

Just because FENV_ACCESS can be toggled on that granularity doesn't mean we have to represent it that way. We've previously agreed (I think) that if FENV_ACCESS is enabled anywhere in a function we will want to use the constrained intrinsics for all FP operations in the function, not just the ones in the scope where it was specified.

Yes, this is also my understanding. We can't soundly mix the two in the same function because we can't prevent the code motion within the function.

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.