In D53157#1291978, @cameron.mcinally wrote:

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.

The Standard allows for this (to some degree). E.g. FENV_ACCESS can be toggled between nested compound statements.

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

Rather than having separate methods to create the constrained versions of the intrinsics, why not have a way to set the constrained state in the IR builder and have the regular CreateFAdd et. al. functions use that to automatically create the constrained intrinsic versions? This would correspond to how fast math flags are handled.

In D53157#1290248, @cameron.mcinally wrote:

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

I don't see the benefit of this change. The intrinsics are good enough to be functional right now (minus a few that are in progress), so I'm not sure we need IRBuilder functions. Am I missing something?

In D53157#1290044, @spatel wrote:

Should the builder's FastMathFlags setting apply to these calls? Is it worth adding an optional FMF parameter to these now, so we don't end up with duplicated API like we have for the regular FP binops?

Update to have tests in this patch.

Minor test changes.

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

In D43142#1285789, @cameron.mcinally wrote:

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

The only thing we need to worry about is making sure that FP operations don't migrate across calls or other FP operations that would break these semantics. Using the intrinsics after inlining takes care of that.

I don't agree with this. This is changing the semantics if we choose to inline a function by converting some operations into constrained intrinsics. In other words, an executable will have different behavior if we choose to inline or not. That's not ok.

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#1285602, @cameron.mcinally wrote:

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.

I'll add that this is a ton of work. A binary Instruction can't currently have two Constant operands. So, ConstantFolding is baked into the Instruction implementation right now. If I'm mistaken, someone please correct me.

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

In D43142#1284800, @hfinkel wrote:

In D43142#1284271, @cameron.mcinally wrote:

In D43142#1284190, @kpn wrote:

...

It's unclear to me how LTO (or other cross file inlining) would work here. I haven't given it much though until now. My knee-jerk reaction is that we shouldn't be inlining from a FENV_ACCESS=OFF to FENV_ACCESS=ON location.

I'm pretty sure that we decided that we couldn't (because once you inline the regular FP ops, there's no way to restrict their movement relative to the constrained intrinsics at the IR level).

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.

In D43142#1285069, @cameron.mcinally wrote:

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.

I'm realizing that FENV_ACCESS is poorly designed.

Let's take this small example:

#include <fenv.h>
#include <stdio.h>

void bar();

#pragma STDC FENV_ACCESS ON
int main() {
  feenableexcept(FE_DIVBYZERO);
  bar();
  float x = 1.0/0.0;
  printf("main x=%lf\n", x);
}

#pragma STDC FENV_ACCESS OFF
void bar() {
  float x = 1.0/0.0;
  printf("bar x=%lf\n", x);
}

The FDiv in bar() would access the FP environment since main() enabled exceptions. But, since bar() is preceded with FENV_ACCESS=OFF, the Standard says that accessing the FP environment in bar() is undefined behavior. Since it's undefined behavior, the compiler can optimize as it wishes, including inlining bar() without modifying the FDiv.

In D43142#1284271, @cameron.mcinally wrote:

In D43142#1284190, @kpn wrote:

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

I don't think that would work in all cases. E.g. a binary operator with two constant operands.

int main() {
  float z = 5.0/0.0;
  return z;
}

Well, not without a considerable effort to rework the IRBuilder at least. I'm fairly sure we can't produce an Instruction with two Constant operands right now.

Oh, of course FNeg will need to be emitted by the front end. And couldn't we turn off constant folding in the IRBuilder when necessary?

Isn't this pass needed since we don't have a way to prevent code movement between blocks using the pragma FENV_ACCESS ON and blocks that aren't? I suggest having it check for use of constrained intrinsics before making any changes.

Address review comments.

FWIW, I like it.

Personally, I'd like to see something where developer-only debug options show up in -help-hidden or similar for the correct executable: opt vs llc. Right now I'm pretty sure they don't. I don't have any opinion on how to make that happen, though.

In D53411#1269102, @uweigand wrote:

While you're at it, shouldn't we also add ROUND and TRUNC to complete the set?

Upload the correct diff this time. Sorry about the confusion!

Address review comments.

I forgot to address the template question. I've spend the past 18 years doing SystemZ backend work on multiple {,JIT} compilers. I'm not really in a position to answer your question about how to handle getting these FP bits down into C++ templates. I hope that won't hold up getting FENV_ACCESS working for C compilations.

The test case for this is in D52839, now.

Address review comments.

Update based on feedback to D52839: add missing AST (de)serialization support.

Ping

Correct obvious error with powi. Fix test and test both C and (partial) C++.

Split out changes as requested. This diff is just the four new intrinsics. The fptoui pass and the change to frem will be later.

In D43515#1229127, @craig.topper wrote:

Adding new constrained instrinsics and adding the pass should be separate patches I think. Changing the syntax of frem should be another patch.

Rebase. Ping.

In D51255#1214427, @jdoerfert wrote:

In D51255#1214424, @asl wrote:

I think we need to preserve the protocol. E.g. if the page is loaded via http we need to load css via http as well.

That sounds reasonable and is (afaik) implied by removing the hostname.
As mentioned before, I like the solution but I just don't know if that might cause problems somewhere else.

Is there a good reason to have the hostname in the URL? Why not just link to "/Users.html" instead of "https://llvm.org/Users.html", for example? Using the hostname is only needed when linking to a page on a different server.

In D50913#1212622, @cameron.mcinally wrote:

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

I haven't thought about this before today, but some of the flags we would want. E.g. reassoc/contract/afn.

In a pure trap-safe mode, contract/afn should be turned off. But, generally speaking, our users would prefer to see optimized code that misses some right-at-the-edge cases.

I just got done talking to one of our floating point math guys here.

In D50913#1205990, @cameron.mcinally wrote:

If the fsub that led to the FNEG node was not constrained in the first place then this seems like a valid transform.

Bah, sorry. My comment was ambiguous...

I meant that the FNEG->FSUB xform is "bad" in the context Ulrich proposed. For example:

The user writes a unary minus, -x, which is a bitwise operation and will never trap.

In D50913#1205893, @cameron.mcinally wrote:

I'm looking into the FNEG handling and found one bad xform in llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:

// Expand Y = FNEG(X) -> Y = SUB -0.0, X

That's not necessarily a show-stopper though. We could change this code to do a bitwise operation instead. That may even be faster than a FSUB, depending on the target of course.

In D50913#1205242, @uweigand wrote:

In D50913#1204810, @cameron.mcinally wrote:

I do see that you wrote -0-x. Is there something special about the -0?

I understand this is to map -0 to +0 and vice versa.

In D50913#1204826, @cameron.mcinally wrote:

User writes 0-x: Frontend maps to constrained FSUB
User writes -x: Frontend maps to FSUB

Yes, exactly.

Rebase. Ping.

This includes the change to pass the annotation token when FENV_ACCESS is turned off.

Replace accidentally lost comment.

Thanks for the fast turnaround! I don't have commit access, can I get you to commit it for me? And can I get you to put my email address <kevin.neal@sas.com> in the commit message?

Updated based on review.

Delete a line accidentally left in.

Missed one place in the documentation that needed updating.

This new diff adds changing the default lowering of STRICT_FP_TO_UINT to not allow speculative execution. I've also eliminated the rounding metadata from instructions when it wasn't needed.

At the request of my employer's legal department:
Copyright © 2018 SAS Institute Inc., Cary, NC, USA. All Rights Reserved.